Skip to content

Pygments-based highlighting#68

Merged
richrd merged 20 commits intorichrd:devfrom
Jimx-:dev
Jul 20, 2015
Merged

Pygments-based highlighting#68
richrd merged 20 commits intorichrd:devfrom
Jimx-:dev

Conversation

@Jimx-
Copy link
Copy Markdown
Contributor

@Jimx- Jimx- commented Jul 19, 2015

Use pygments to tokenize the code rather than split the code into words directly. This looks much better than the previous highlighter. Now we have built-in support for over 300 languages.

@richrd
Copy link
Copy Markdown
Owner

richrd commented Jul 19, 2015

Wow, great! You're fast 👍

Mostly looks good to me, but there's a few unneeded changes to logging. Specifically some logging is done via the self.app, but self.logging is also available. It's better for each module to have it's own logger, so we can see where the log messages are coming from. (All logging is routed to the root logger anyway)
https://github.com/richrd/suplemon/pull/68/files#diff-458d284e5e7285ccd7bfed93d494a0d9L162
https://github.com/richrd/suplemon/pull/68/files#diff-ee4f0105c178246342814092a4e79e4aL65

There are a few others lines too that use self.app.logger. If you could fix those I'll be ready to merge this. Thanks again very much, great work! I really like how the new highlighting looks. :)

@richrd
Copy link
Copy Markdown
Owner

richrd commented Jul 19, 2015

I also forgot that there are some old not needed imports in this PR. Also it's good to avoid from module import *

They could be removed :)

@Jimx-
Copy link
Copy Markdown
Contributor Author

Jimx- commented Jul 20, 2015

Ok, all unnecessary imports are removed and modules use their own logger. I was fixing conflicts and didin't notice the changes in logging...

@richrd
Copy link
Copy Markdown
Owner

richrd commented Jul 20, 2015

Many thanks! Merging this now. :)

richrd added a commit that referenced this pull request Jul 20, 2015
Pygments-based highlighting
@richrd richrd merged commit 5fce3b2 into richrd:dev Jul 20, 2015
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