Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MRG] Add syntax highlighting to code blocks #110

Open
wants to merge 22 commits into
base: master
Choose a base branch
from

Conversation

bkjohnson
Copy link

@bkjohnson bkjohnson commented Sep 4, 2021

Description

This PR adds support for syntax highlighting on code blocks using pygments.

Related issues

Screenshots

Based on this markdown:

## Code blocks

```python
print("This is a test")
```

Foo

```js
const myFunc = (name, num) => {
  return [name, num + 4]
}

console.log(`Data is ${myFunc('John')[0]}`);
```

---

Here is a screenshot of the current output:

image

With asciimatics version 1.13.1.dev38+g1fde70e, the bug relating to parenthesis rendering is fixed:

image

Next steps

Here are the next steps just for this PR. I'm not planning on adding support for changing the styling theme - that can come later.
I'm planning on refactoring and cleaning up, but feel free to offer any other suggestions.

  • Ensure whitespace has background to match rest of block
  • Stop code from being visually truncated
  • Pad the block with a border of dark background squares
  • Make sure multiple blocks can be rendered on same slide
  • Update docs/_static/demo.gif to show the new highlighting feature
  • Update asciimatics version to get bugfix when possible

I think the Effect may make it difficult/impossible to apply syntax
highlighting to static text
The paint() function seems like the simplest way of rendering
highlighted text, and it's only available on Screen or Canvas objects.
This is why I'm not using an Effect for a code block.
We are no longer calling the _code() function, so we don't need to store
the data in the same way. The code element tells us what to render, and
the row will tell us where to place it on the slide.

_code() will be removed later.
From this function we should be able to get the language from the block,
then highlight and paint line by line
Once we get to the restart page, the animation loop keeps running and we
get an error if we try to set `cur_slide` normally.
Copy link
Author

@bkjohnson bkjohnson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll try to refactor this for clarity.

present/slideshow.py Outdated Show resolved Hide resolved
present/slideshow.py Outdated Show resolved Hide resolved
I don't like modifying slide.code_blocks in the get_effects function.
The reason I'm doing it is because that is where we determine what row
the code block will go on.
I don't want to get too carried away with this type of thing for this
PR. I'm doing this to hopefully make it more obvious where code_blocks
is coming from.
This is just a bit of cleanup for organization & clarity.
Copy link
Author

@bkjohnson bkjohnson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding some context for reviewers. Unless changes are requested, the only other thing I'd like to do in this PR is bump the asciimatics version if the AnsiTerminalParser white foreground bug is fixed.

@@ -17,6 +17,7 @@
"Click>=7.0",
"mistune>=2.0.0a4",
"pyfiglet>=0.8.post1",
"Pygments>=2.10.0",
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pygments is the library that adds highlighting to the code. It has different styles/themes, but I'm leaving it on the default.

present/slideshow.py Outdated Show resolved Hide resolved
present/slideshow.py Outdated Show resolved Hide resolved
present/slideshow.py Outdated Show resolved Hide resolved
present/slideshow.py Outdated Show resolved Hide resolved
@bkjohnson bkjohnson marked this pull request as ready for review September 7, 2021 02:30
@bkjohnson bkjohnson changed the title [WIP] Add syntax highlighting to code blocks [MRG] Add syntax highlighting to code blocks Sep 7, 2021
I tested this change by putting a fireworks effect on the slide and
checking to see if I got the ValueError.
@bkjohnson bkjohnson marked this pull request as draft September 7, 2021 03:50
@bkjohnson
Copy link
Author

The PR works as-is, but I'd like to explore adding a custom HighlightedCode renderer and see if I can work the changes into the overall Scene/Effect structure.

This feels more in line with the Scene/Effect structure of the project.
This doesn't change the functionality - I'm just doing this since there
isn't a particular reason to remove them, and I want to make the PR diff
smaller.
@bkjohnson bkjohnson marked this pull request as ready for review September 7, 2021 15:26
@bkjohnson
Copy link
Author

@vinayak-mehta after plenty of back and forth I think this is finally ready for review. I tried to make a new gif, but none of my recording tools can get a smooth framerate for the animations, so I'm not sure if I'll be able to update demo.gif.

A user might use a code block and intentionally leave out the language
for displaying logs or other things.
@galuszkak
Copy link

Hi,

Amazing PR @bkjohnson !

Would you consider merging and releasing this @vinayak-mehta ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants