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

CodeHilite: Support Pygments options #822

Closed
wants to merge 1 commit into from
Closed

CodeHilite: Support Pygments options #822

wants to merge 1 commit into from

Conversation

p-ouellette
Copy link

@p-ouellette p-ouellette commented May 11, 2019

Allows specifying any Pygments option as suggested in #334. This also allows using the Pygments inline option for line numbers (#652).

Example:

In [2]: text = """ 
   ...:     :::php 
   ...:     print "Hello world"; 
   ...: """                                                                                                            

In [3]: markdown(text, 
   ...:          extensions=['codehilite'], 
   ...:          extension_configs={'codehilite': { 
   ...:              'lexer_options': {'startinline': True}, 
   ...:              'formatter_options': {'wrapcode': True, 'linenos': 'inline'}}})                                   
Out[3]: '<div class="codehilite"><pre><span></span><code><span class="lineno">1 </span><span class="k">print</span> <span class="s2">&quot;Hello world&quot;</span><span class="p">;</span>\n</code></pre></div>'

There's some messiness where the Pygments options overlap with the existing options. I marked the ones that apply only to Pygments as deprecated, but some, like linenums and css_class are used when Pygments is disabled.

@mitya57
Copy link
Collaborator

mitya57 commented May 11, 2019

I like this. It does not actually close #334 though, as this code does not allow one to set the options in a document (only via API). But I guess that can be the next step.

Also, can you make it issue some DeprecationWarnings when the deprecated options are used?

@waylan
Copy link
Member

waylan commented May 11, 2019

this code does not allow one to set the options in a document

See #816 which has started the work on that. Interestingly, this makes that more complicated. As this breaks the arguments up into 2 dicts to differentiate between lexer and formatter options. Previously, we didn't care about that distinction which made specifying options in the document easier. I'll have to think about that.

@p-ouellette
Copy link
Author

I wasn't thinking about setting the options in the document. I'll change it to use a single dict if requested. Then any key=value pair specified in the document that isn't a valid codehilite option could be passed to Pygments.

@waylan waylan added extension Related to one or more of the included extensions. feature Feature request. needs-decision A decision needs to be made regarding request. labels Jun 13, 2019
@@ -75,7 +79,8 @@ class CodeHilite(object):

def __init__(self, src=None, linenums=None, guess_lang=True,
css_class="codehilite", lang=None, style='default',
noclasses=False, tab_length=4, hl_lines=None, use_pygments=True):
noclasses=False, tab_length=4, use_pygments=True,
lexer_options={}, formatter_options={}):
Copy link
Member

Choose a reason for hiding this comment

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

I'm thinking we should just do **options here. That does two things:

  1. It avoids the extra level of nesting when defining the configs.
  2. It avoids separating lexer and formatter options.

As it stands now, Pygments uses **options in their lexer and formatter classes, and ignores any unknown option keywords. So as long as there are no naming conflicts between lexer and formatter options, we could pass all options to both the lexer and formatter. This has the added benefit of making it easier to define options from within documents (see #816).

@waylan
Copy link
Member

waylan commented Dec 31, 2019

I'm closing this in favor of #816. However, @pauloue your work was not in vain as it helped me to see how to best approach the matter.

@waylan waylan closed this Dec 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
extension Related to one or more of the included extensions. feature Feature request. needs-decision A decision needs to be made regarding request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants