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

pygments 2.7.3 broke pygments-ansi-color's html formatter subclass #1644

Closed
asottile opened this issue Dec 25, 2020 · 7 comments · Fixed by #1655
Closed

pygments 2.7.3 broke pygments-ansi-color's html formatter subclass #1644

asottile opened this issue Dec 25, 2020 · 7 comments · Fixed by #1655

Comments

@asottile
Copy link

The changes in #1569 made changes to _get_css_classes (which now returns a <span ...> instead of css classes like the name suggests)

I understand this is a private api and all bets are off, but I was surprised to see a break in a patch version nonetheless.

See also chriskuehl/pygments-ansi-color#11

@Anteru
Copy link
Collaborator

Anteru commented Dec 28, 2020

This is a private API (it's a private function after all), so we're free to change. However: What was the use case to use that in the first place that broke? Maybe we can do something about that.

@asottile
Copy link
Author

I'm trying to add additional css classes based on the tokens. For example right now, to mark something as bold red the token is named Text.Color.Red.Bold (or Text.Color.Bold.Red but let's ignore that for a second). This means that pygments will generate a css class that looks like -Color-Red -Color-Red-Bold. This is problematic if you want to write css that applies bold to this -- for example .-Color-Bold { font-weight: bold; }.

So there's a special override of _get_css_classes which takes these tokens and explodes them out into all of the individual css classes: -Color-Red -Color-Bold

This has worked nicely until _get_css_classes no longer returns css classes in 2.7.3

@Anteru
Copy link
Collaborator

Anteru commented Dec 28, 2020

@kurtmckee , assuming you're around: Was it important to have _get_css_classes return the <span> as well? It was part of your performance tweak, but looking at the diff it's unclear to me if the direct-wrap-into-span is critical or not.

@kurtmckee
Copy link
Contributor

kurtmckee commented Dec 28, 2020 via email

@Anteru
Copy link
Collaborator

Anteru commented Dec 29, 2020

Thanks Kurt! If it makes anything slower then please keep the change as-is. As to the original issue, I wonder if that should be an option actually, i.e. to turn Text.Color.Red.Bold into the CSS classes text, color, red, bold.

@kurtmckee
Copy link
Contributor

@Anteru, it looks like I can move the span wrapping code to the except KeyError clause in _format_lines(). I anticipate that will restore the original functionality of _get_css_classes() without sacrificing the caching. I'll try to get this resolved asap.

@asottile, sorry for the problems this caused.

to mark something as bold red the token is named Text.Color.Red.Bold
(or Text.Color.Bold.Red but let's ignore that for a second)

That parenthesized bit concerns me, but I don't recognize those tokens. Are you saying that Pygments is returning identical tokens with different names? If so, please open a bug report with any relevant info you can provide, and @ me somewhere in it. I'd be interested in figuring out why that's happening, but I want to investigate that separately from fixing this.

@asottile
Copy link
Author

@asottile, sorry for the problems this caused.

to mark something as bold red the token is named Text.Color.Red.Bold
(or Text.Color.Bold.Red but let's ignore that for a second)

That parenthesized bit concerns me, but I don't recognize those tokens. Are you saying that Pygments is returning identical tokens with different names? If so, please open a bug report with any relevant info you can provide, and @ me somewhere in it. I'd be interested in figuring out why that's happening, but I want to investigate that separately from fixing this.

They come from the lexer linked which parses ansi escape sequences.

For example these (rightfully) will produce two sets of tokens:

\033[1m  bolded \033[31m bolded red \033[22m no longer bold \033[m no longer red
\033[31m  red \033[1m bolded red \033[22m no longer bold \033[m no longer red

the first has Text.Color.Bold then Text.Color.Bold.Red then Text.Color.Red then Text
the second has Text.Color.Red then Text.Color.Red.Bold then Text.Color.Red then Text

kurtmckee added a commit to kurtmckee/pr-pygments that referenced this issue Dec 30, 2020
This should revert the behavior of the function without losing
the overall caching behavior that was intended.

Closes pygments#1644
Anteru pushed a commit that referenced this issue Dec 30, 2020
This should revert the behavior of the function without losing
the overall caching behavior that was intended.

Closes #1644
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 a pull request may close this issue.

3 participants