search

Python-Markdown / markdown

A Python implementation of John Gruber’s Markdown with Extension support.
https://python-markdown.github.io/
BSD 3-Clause "New" or "Revised" License
3.79k stars 863 forks source link

extensions: copy config dict on each highlighted block #1241

Closed gertvdijk closed 2 years ago

gertvdijk commented 2 years ago

This fixes a bug where any subsequent highlighted block with codehilite would result in the omission of the style setting, falling back to default.

Fixes #1240

mitya57 commented 2 years ago

Can you please add a test for multiple highlighted blocks?

gertvdijk commented 2 years ago

@mitya57 Added them to the now amended commit. 😃

gertvdijk commented 2 years ago

CI error appears unrelated to my change. 😕

43 files checked. ERROR: 5 files with dead links found!

waylan commented 2 years ago

The reason why we pop the setting is because we also pass the rest of the config items on to Pygments (see the very next line). As each of Pygmants' lexers/formatters have a different set of options they accept, we just pass all unknown keywords on. However, there is no need to pass the style on as we have already defined that. The only reason we don't get an error is because Pygments just ignores any unknown keywords passed to its lexers and formatters. Still, it would be best to not pass it on to avoid any weird conflicts in the future.

Perhaps we should be making a copy of the config for each code block and then we can pop the setting off of the copy.

As an aside, we have the pygments_style setting for historical reasons (it existed before we accepted all Pygments options). If we were starting fresh today we would not define a special option for it and users could just use style which would get passed it on with the rest of the Pygments' keyword options. Actually, a user could avoid this bug by using style now (without this fix). That said, we should still fix the bug. Although, we might want to deprecate the pygments_style option as it is duplicative and unnecessary.

waylan commented 2 years ago

CI error appears unrelated to my change. 😕

43 files checked. ERROR: 5 files with dead links found!

Yes, I see that. I have opened a separate issue for that in #1243. You can ignore that failure here.

gertvdijk commented 2 years ago

@waylan Thanks for the explanation! 😃 It took a bit longer to get back to this, but I've pushed an amendment.