sirthias / pegdown

A pure-Java Markdown processor based on a parboiled PEG parser supporting a number of extensions
http://pegdown.org
Apache License 2.0
1.29k stars 217 forks source link

A few fixes accumulated from my plugin releases. #195

Closed vsch closed 9 years ago

vsch commented 9 years ago

…the header as part of the anchor name generator text, text accumulated, special chars ignored. Fix #194

fix #190, two or more blank lines between list items would break the list into separate lists fix #191, completed task list items could have capital X not just lowercase. [X] and [x] are equivalent. fix #192, paragraph wrapping for list items would produce an AST different from how blank lines before list items. Now identical. fix #193 change private to protected methods in ToHtmlSerializer so that it can be extended without duplicating code

sirthias commented 9 years ago

Very nice, thank you very much once again, Vladimir! Are we ready for the next release or do you have more stuff coming? :)

vsch commented 9 years ago

You are welcome.

I am going to be implementing a PSI Parser for the plugin but use pegdown for the AST generation and fake the PSI parsing the same way I am doing with the plugin lexer. The lexer implementation is what led me to find the previous issues.

I am sure when I am done with the parser there will be many more fixes as I will be pushing pegdown beyond the call of duty.

Judging by the clamour for a new release I would say go for the release. I will PR my changes once I have the fake PSI parser complete. No one is waiting or probably will use those changes, so there will be no rush to release them.

slovdahl commented 9 years ago

@sirthias, do you have any ETA for a release with all these fixes?

sirthias commented 9 years ago

I just pushed pegdown 1.6.0 to sonatype. Should appear on Maven Central in a few hours. Thank you all again!

slovdahl commented 9 years ago

Awesome, thank you!

Deraen commented 9 years ago

@vsch Is the different output format (<h1><a name="...">header</a></h1> vs <h1><a name="..."></a>header</h1>) intended between anchorlinks and extanchorlinks? The documentation doesn't mention the difference.

vsch commented 9 years ago

Yes, it creates an anchor link for headers that have more than one child by collecting child text nodes. Original code only creates an anchor link if the header has a single child node.

Deraen commented 9 years ago

Yeah but I wasn't referring to difference on how the name property for anchor is generated or when the anchor is generated but the difference that the old extensions wraps the contents inside the a element and new version adds empty a element to the start of h element. I think this should be mentioned in the readme.

Though, I wonder if there is any reason to keep the old version which only works with one element in header. Doesn't sound like useful option.

vsch commented 9 years ago

The extension makes it compatible with GFM generated HTML and wrapping the header text is not necessary because GFM stylesheet displays a link icon on hover over the header.

I added this option because I needed GFM html generation for my idea-multimarkdown plugin to generate GitHub-like previews. There was no way to do it by customizing the ToHtmlSerializer or LinkRenderer because the parser did not generate the anchor link at all if there is more than one child.

I left the old option for backwards compatibility and I don't recommend removing it. I don't see a lot of people using the new option without modifying some of their code to support the feature. If they do, then they can easily add the extra extension flags. However, anyone else would expect the new version to work with their existing implementation. All the new extension were added as optionals for the same reason. They are not even included in the ALL options and must be specified explicitly.

If someone needs different HTML generation or link rendering, they can now extend ToHtmlSerializer or LinkRenderer, easier than writing plugins for it, and get whatever they want. It takes five minutes and everything in ToHtmlSerializer is re-usable. I changed two private methods to protected in the last PR for that reason. No one has to wait for @sirthias if they need customized HTML rendering. Just override the methods for the Nodes that you want to change. Copy original code from ToHtmlSerializer and hack away.

Same goes for anchor link rendering like GitHub's. I did not care for the exact duplication because I just need a preview that works and looks the same and needed these to be generated for every header not just the plain text ones. I did not see the point in duplicating every nuance of GitHub's implementation details for anchor links. However, if someone needs it, now they only have to override a single method in LinkRenderer and they are in business. As they say in textbooks: "the last part is left as an exercise for the reader."