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

Add feature for emphasizing some lines in a code block. #274

Merged
merged 4 commits into from
Jan 7, 2014
Merged

Add feature for emphasizing some lines in a code block. #274

merged 4 commits into from
Jan 7, 2014

Conversation

ajdavis
Copy link
Contributor

@ajdavis ajdavis commented Jan 3, 2014

A code blocked headed by “:::python{1,3}” now emphasizes the first and third lines. With fences enabled, ```python{1,3} has the same effect.

Pygments has had this feature for a while. Highlighted lines have a yellow background, which is very useful for drawing a reader's attention to specific lines in a long code block. Some other Markdown implementations use this syntax for specifying which lines to highlight, I thought Python-Markdown would benefit from the same feature.

A code blocked headed by “:::python{1,3}” now emphasizes the first and third lines. With fences enabled, ```python{1,3} has the same effect.
@waylan
Copy link
Member

waylan commented Jan 4, 2014

Interesting. Which markdown implementations already support this? This is the first I've seen it and I try to watch the markdown community closely.

Second, how old is this feature in Pygments? In other words, how likely is it that our users might run into a backward incompatibility issue because they have an older version of Pygments? Or, will Pygments just ignore the unknown keyword if the feature is not supported?

@ajdavis
Copy link
Contributor Author

ajdavis commented Jan 4, 2014

On second thought, there's only one Markdown implementation that supports line-highlighting at all, here. I couldn't find any others. So it seems there's only one syntax in the Markdown community so far, and it's not widely used.

There's a similar idea with a different syntax in Wordpress.com's "code" shortcode. It wants syntax like [code highlight="2,3"].

Wordpress, in turn, uses the SyntaxHighlighter Javascript library, which supports syntax like highlight: [1, 3].

I think highlighting lines is very useful, but I recognize it'd cause Python-Markdown's dialect to diverge further from John Gruber's.

hl_lines was introduced in Pygments 0.11 in August 2008, so backwards compatibility isn't a problem.

@waylan
Copy link
Member

waylan commented Jan 5, 2014

@ajdavis you should be warned that I've spent a lot of time thinking about highlighting code over the years. In fact the CodeHilite extension is actually older that Pygments (it originally used Linux command line tools - which ran a separate new process for each code block in a document - uhg). For some more recent thoughts see this post from 2010 and this draft from 2011.

I'm not opposed to the idea of providing a method of highlighting specific lines of a code block. The codehilite extension is already a divergence from standard markdown anyway (in that specifying the language is specific to Python-Markdown). However, I'm not so sure about the proposed syntax for a few reasons.

First, while there is an existing implementation, it appears to be a third party extension which has not been committed upstream. There is no way of knowing how popular it is. That being the case, I see no reason why we can't diverge from it. Especially given my other concerns below.

My second concern is related to the Attribute List Extension. The proposed syntax is very close, but slightly different than the syntax used there which could create some confusion to our users. Given that this is solving a different problem, I'm thinking a completely different syntax may be better.

Third, don't forget about the Fenced Code Extension. Whatever we do needs to work with that extension also. And it identifies the language slightly differently. See this discussion regarding PHP's implementation for a summary of all the concerns there. Whatever we do needs to fit in with that. Also, note that the use of curly brackets is already supported -- so that would rule them out for indicating highlighted lines.

Finally, we might want to explore supporting the use of JavaScript code highlighters in fenced code blocks. Although, as every one uses a slightly different syntax, most authors are probably better off using raw html and setting the attributes manually in the pre and/or code tags. Still, I think whatever syntax we use can be inspired by and/or feel like something that would work the same/similarly.

My suggestion would be to start with the Fenced Code Blocks syntax, and then carry that syntax over to the traditional indented code blocks.

@ajdavis
Copy link
Contributor Author

ajdavis commented Jan 6, 2014

Thanks for all the info @waylan ! PHP-Markdown supports a syntax like this for specifying the class and id attributes:

~~~{.python #example-1}
print "Code here."
~~~~

I propose this similar syntax for fenced code blocks:

~~~~{.python hl_lines="1 3"}
print "Code here."
~~~~

It's like Python-Markdown's current syntax but with a name="value" pair added. Braces would still be optional:

~~~~.python hl_lines="1 3"
print "Code here."
~~~~

GitHub-style would be like this:

````python hl_lines="1 3"
print "Code here."
````

What do you think?

@waylan
Copy link
Member

waylan commented Jan 7, 2014

@ajdavis I think that is a very reasonable proposal. I just typed up a big long response about whether to use a space or comma delimited list, but in the process talked myself out of commas and deleted it. If you update the PR, I'll be happy to review and merge.

@ajdavis
Copy link
Contributor Author

ajdavis commented Jan 7, 2014

Thanks. I've added a commit, we now support the proposed syntax with both ::: style and fenced code blocks.

I'm glad we agree on having the list space-delimited rather than comma-delimited. I chose spaces because it resembles HTML attribute lists like class="class1 class2". Lists like that will show up often in Markdown source, so the hl_lines list should be formatted similarly.

@waylan
Copy link
Member

waylan commented Jan 7, 2014

Looking good. Although I just noticed you only have support for double quotes. As single quotes are supported elsewhere in markdown (link titles for example), I think we should support them here as well. So hl_lines="1 3" and hl_lines='1 3' should both return the same result. I think it is reasonable for authors to try either and they would find it strange that the one is completely ignored.

Oh and it would be nice to document this new feature in each of the extension's doc pages.

@ajdavis
Copy link
Contributor Author

ajdavis commented Jan 7, 2014

Done and done.

waylan pushed a commit that referenced this pull request Jan 7, 2014
Add feature for emphasizing some lines in a code block.
@waylan waylan merged commit 809195f into Python-Markdown:master Jan 7, 2014
@ajdavis ajdavis deleted the highlight-lines branch January 7, 2014 22:57
@ajdavis
Copy link
Contributor Author

ajdavis commented Jan 7, 2014

Lovely, thank you. Is there an estimated date for the next release?

@waylan
Copy link
Member

waylan commented Jan 9, 2014

Releases happen when they make sense. Right now, given the number of open bug reports, a release doesn't make much sense. And I don't have the time to be debugging and patching bugs right now. Of course, help is always welcome.

@ajdavis
Copy link
Contributor Author

ajdavis commented Jan 9, 2014

Understood, thanks.

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.

None yet

2 participants