Improve translation of pygments styles into urwid #1434
Labels
Comments
neiljp
added
help wanted
|
Hi all, I would like to work on this issue. @zulipbot claim |
|
Welcome to Zulip, @jrijul1201! We just sent you an invite to collaborate on this repository at https://github.com/zulip/zulip-terminal/invitations. Please accept this invite in order to claim this issue and begin a fun, rewarding experience contributing to Zulip! Here's some tips to get you off to a good start:
As you work on this issue, you'll also want to refer to the Zulip code contribution guide, as well as the rest of the developer documentation on that site. See you on the other side (that is, the pull request side)! |
jrijul1201
added a commit
to jrijul1201/zulip-terminal
that referenced
this issue
Dec 19, 2023
Pygments 2.16.0 introduced a style to support a combination of bold and italic styling in pygments/pygments#2444. Both of our gruvbox themes and the light native theme gain a 'bold strong' style via pygments as a result, which urwid fails to parse and blocks the application from loading. This work would represent a clean fix for zulip#1431, which was temporarily fixed by pinning pygments at ~=2.15.1 in zulip#1433. Add method `translate_styles` that manually converts the pygments `bold italic` style into the urwid-compatible `bold, italics`. Tests added. Fixes part of zulip#1434.
18 tasks
jrijul1201
added a commit
to jrijul1201/zulip-terminal
that referenced
this issue
Dec 20, 2023
Pygments 2.16.0 introduced a style to support a combination of bold and italic styling in pygments/pygments#2444. Both of our gruvbox themes and the light native theme gain a 'bold strong' style via pygments as a result, which urwid fails to parse and blocks the application from loading. This work would represent a clean fix for zulip#1431, which was temporarily fixed by pinning pygments at ~=2.15.1 in zulip#1433. Add method `translate_styles` that manually converts the pygments `bold italic` style into the urwid-compatible `bold, italics`. Tests added. Fixes part of zulip#1434.
jrijul1201
added a commit
to jrijul1201/zulip-terminal
that referenced
this issue
Dec 20, 2023
Pygments 2.16.0 introduced a style to support a combination of bold and italic styling in pygments/pygments#2444. Both of our gruvbox themes and the light native theme gain a 'bold strong' style via pygments as a result, which urwid fails to parse and blocks the application from loading. This work would represent a clean fix for zulip#1431, which was temporarily fixed by pinning pygments at ~=2.15.1 in zulip#1433. Add method `translate_styles` that manually converts the pygments `bold italic` style into the urwid-compatible `bold, italics`. Tests added. Fixes part of zulip#1434.
jrijul1201
added a commit
to jrijul1201/zulip-terminal
that referenced
this issue
Dec 21, 2023
Pygments 2.16.0 introduced a style to support a combination of bold and italic styling in pygments/pygments#2444. Both of our gruvbox themes and the light native theme gain a 'bold strong' style via pygments as a result, which urwid fails to parse and blocks the application from loading. This was temporarily fixed by pinning pygments at ~=2.15.1 in zulip#1433. Method `translate_styles` manually converts the pygments `bold italic` style into the urwid-compatible `bold, italics` is there. Unpin Pygments from ~=2.15.1 to >=2.14.0. Fixes part of zulip#1434.
jrijul1201
added a commit
to jrijul1201/zulip-terminal
that referenced
this issue
Dec 22, 2023
Pygments 2.16.0 introduced a style to support a combination of bold and italic styling in pygments/pygments#2444. Both of our gruvbox themes and the light native theme gain a 'bold strong' style via pygments as a result, which urwid fails to parse and blocks the application from loading. This was temporarily fixed by pinning pygments at ~=2.15.1 in zulip#1433. Method `translate_styles` manually converts the pygments `bold italic` style into the urwid-compatible `bold, italics` is there. Unpin Pygments from ~=2.15.1 to >=2.14.0. Fixes part of zulip#1434.
jrijul1201
added a commit
to jrijul1201/zulip-terminal
that referenced
this issue
Dec 22, 2023
Pygments 2.16.0 introduced a style to support a combination of bold and italic styling in pygments/pygments#2444. Both of our gruvbox themes and the light native theme gain a 'bold strong' style via pygments as a result, which urwid fails to parse and blocks the application from loading. This work would represent a clean fix for zulip#1431, which was temporarily fixed by pinning pygments at ~=2.15.1 in zulip#1433. Add method `translate_styles` that manually converts the pygments `bold italic` style into the urwid-compatible `bold, italics`. Tests added. Fixes part of zulip#1434.
jrijul1201
added a commit
to jrijul1201/zulip-terminal
that referenced
this issue
Dec 22, 2023
Pygments 2.16.0 introduced a style to support a combination of bold and italic styling in pygments/pygments#2444. Both of our gruvbox themes and the light native theme gain a 'bold strong' style via pygments as a result, which urwid fails to parse and blocks the application from loading. This was temporarily fixed by pinning pygments at ~=2.15.1 in zulip#1433. Method `translate_styles` manually converts the pygments `bold italic` style into the urwid-compatible `bold, italics` is there. Unpin Pygments from ~=2.15.1 to >=2.14.0. Fixes part of zulip#1434.
jrijul1201
added a commit
to jrijul1201/zulip-terminal
that referenced
this issue
Dec 22, 2023
Pygments 2.16.0 introduced a style to support a combination of bold and italic styling in pygments/pygments#2444. Both of our gruvbox themes and the light native theme gain a 'bold strong' style via pygments as a result, which urwid fails to parse and blocks the application from loading. This work would represent a clean fix for zulip#1431, which was temporarily fixed by pinning pygments at ~=2.15.1 in zulip#1433. Add method `translate_styles` that manually converts the pygments `bold italic` style into the urwid-compatible `bold, italics`. Tests added. Fixes part of zulip#1434.
jrijul1201
added a commit
to jrijul1201/zulip-terminal
that referenced
this issue
Dec 22, 2023
The previous commit in zulip#1452 fixes the issue zulip#1431 that was temporarily fixed by pinning Pygments at ~=2.15.1. Combined with <2.18.0, since that unreleased version depends on Python3.8 and could make ZT uninstallable on older pythons. Fixes part of zulip#1434.
|
@jrijul1201 You have been unassigned from this issue because you have not made any updates for over 14 days. Please feel free to reclaim the issue if you decide to pick up again. Thanks! |
|
@zulipbot |
|
@zulipbot claim |
neiljp
pushed a commit
to jrijul1201/zulip-terminal
that referenced
this issue
Jan 8, 2024
Pygments 2.16.0 introduced a style to support a combination of bold and italic styling in pygments/pygments#2444. Both of our gruvbox themes and the light native theme gain a 'bold italic' style via pygments as a result, which urwid fails to parse and blocks the application from loading. This work would represent a clean fix for zulip#1431, which was temporarily fixed by pinning pygments at ~=2.15.1 in zulip#1433. This minimally handles spaces and the shortened `italic` in pygments styles, sufficient to resolve the current style issue. Note that other pygments styles in themes that we do not yet use may need additional translation or adjustment. Tests added. Fixes part of zulip#1434.
neiljp
pushed a commit
to jrijul1201/zulip-terminal
that referenced
this issue
Jan 8, 2024
The previous commit fixes the issue zulip#1431 that was temporarily fixed by pinning pygments at ~=2.15.1 in zulip#1433. Subsequent versions of pygments have since been released, but an upper limit of 2.18.0 is included, since that unreleased version depends on Python3.8 and could make ZT uninstallable on older pythons. Fixes part of zulip#1434.
neiljp
pushed a commit
to jrijul1201/zulip-terminal
that referenced
this issue
Jan 8, 2024
Pygments 2.16.0 introduced a style to support a combination of bold and italic styling in pygments/pygments#2444. Both of our gruvbox themes and the light native theme gain a 'bold italic' style via pygments as a result, which urwid fails to parse and blocks the application from loading. This work would represent a clean fix for zulip#1431, which was temporarily fixed by pinning pygments at ~=2.15.1 in zulip#1433. This minimally handles spaces and the shortened `italic` in pygments styles, sufficient to resolve the current style issue. Note that other pygments styles in themes that we do not yet use may need additional translation or adjustment. Tests added. Fixes part of zulip#1434.
neiljp
pushed a commit
to jrijul1201/zulip-terminal
that referenced
this issue
Jan 8, 2024
The previous commit fixes the issue zulip#1431 that was temporarily fixed by pinning pygments at ~=2.15.1 in zulip#1433. Subsequent versions of pygments have since been released, but an upper limit of 2.18.0 is included, since that unreleased version depends on Python3.8 and could make ZT uninstallable on older pythons. Fixes part of zulip#1434.
neiljp
pushed a commit
that referenced
this issue
Jan 8, 2024
Pygments 2.16.0 introduced a style to support a combination of bold and italic styling in pygments/pygments#2444. Both of our gruvbox themes and the light native theme gain a 'bold italic' style via pygments as a result, which urwid fails to parse and blocks the application from loading. This work would represent a clean fix for #1431, which was temporarily fixed by pinning pygments at ~=2.15.1 in #1433. This minimally handles spaces and the shortened `italic` in pygments styles, sufficient to resolve the current style issue. Note that other pygments styles in themes that we do not yet use may need additional translation or adjustment. Tests added. Fixes part of #1434.
neiljp
pushed a commit
that referenced
this issue
Jan 8, 2024
The previous commit fixes the issue #1431 that was temporarily fixed by pinning pygments at ~=2.15.1 in #1433. Subsequent versions of pygments have since been released, but an upper limit of 2.18.0 is included, since that unreleased version depends on Python3.8 and could make ZT uninstallable on older pythons. Fixes part of #1434.
|
@jrijul1201 You have been unassigned from this issue because you have not made any updates for over 14 days. Please feel free to reclaim the issue if you decide to pick up again. Thanks! |
|
Thanks to @jrijul1201 for completing the first part of this in #1452, which was a great bugfix and allowed an upgrade to the latest version 🎉 The remaining part concerns the checking for redundant overrides in themes, which would help us simplify themes, as well as reduce such overrides being present in user themes. @jrijul1201 Are you interested in continuing this work? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
This work would represent a clean fix for #1431, which was temporarily fixed by pinning pygments at
~=2.15.1in #1433. However, it could also heavily simplify our need for overrides in the themes.As per #1433, the bug could be worked around by using our pygments override option, which manually converts the pygments
bold italicstyle into the urwid-compatiblebold, italics.However, instead of using overrides for this, we should aim to automatically convert pygments styles into urwid-compatible versions. Based on the above, we likely want to consider at least:
italictoitalicsInstead of manually testing directly with an upgraded version of pygments, initial indications are that we can remove an override from an existing style. If when you do so ZT gives an error, you can compare that string with the removed override to see what differs - and if an automatic translation could replace it. Once we have at least the comma insertion (first task above) we could therefore add
Once existing themes are simplified and we have an improved translation in place:
As with the majority of PRs, each automatic conversion and new functionality should be accompanied by an updated or new test.
A possible extension or early work, would be to explore writing a test that fails with an invalid pygments style (eg
bold italic). That should limit the impact of something like #1431 in future, since we should expect a failure in our tests, not just at runtime.The text was updated successfully, but these errors were encountered: