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

Gruvbox & zt_light themes don't work with pygments 2.16.x #1431

Closed
laomuon opened this issue Sep 30, 2023 · 3 comments · Fixed by #1433
Closed

Gruvbox & zt_light themes don't work with pygments 2.16.x #1431

laomuon opened this issue Sep 30, 2023 · 3 comments · Fixed by #1433
Labels
Milestone

Comments

@laomuon
Copy link

laomuon commented Sep 30, 2023

Description

When I launch with the option -t gruvbox or -t gruvbox_light, I encountered this error:

Possible theme error: Unrecognised color specification 'bold italic' in foreground ('bold italic').

Im on the main branch of the repo

Logs

Traceback (most recent call last):
  File "/home/muon/zulip-terminal/zulipterminal/cli/run.py", line 559, in main
    Controller(
  File "/home/muon/zulip-terminal/zulipterminal/core.py", line 99, in __init__
    self.loop = urwid.MainLoop(self.view, self.theme, screen=screen)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/muon/zulip-terminal/zl-env/lib/python3.11/site-packages/urwid/main_loop.py", line 118, in __init__
    screen.register_palette(palette)
  File "/home/muon/zulip-terminal/zl-env/lib/python3.11/site-packages/urwid/display_common.py", line 856, in register_palette
    self.register_palette_entry(*item)
  File "/home/muon/zulip-terminal/zl-env/lib/python3.11/site-packages/urwid/display_common.py", line 938, in register_palette_entry
    high_256 = AttrSpec(foreground_high, background_high, 256)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/muon/zulip-terminal/zl-env/lib/python3.11/site-packages/urwid/display_common.py", line 537, in __init__
    self.foreground = fg
    ^^^^^^^^^^^^^^^
  File "/home/muon/zulip-terminal/zl-env/lib/python3.11/site-packages/urwid/display_common.py", line 637, in _set_foreground
    raise AttrSpecError(("Unrecognised color specification %s " +
urwid.display_common.AttrSpecError: Unrecognised color specification 'bold italic' in foreground ('bold italic')

Additional context

It seems that pygment:ges has a property bold italic which I think is not directly supported by urwid. A possible workaround:

--- a/zulipterminal/themes/gruvbox_dark.py
+++ b/zulipterminal/themes/gruvbox_dark.py
@@ -93,6 +93,7 @@ META = {
             'n'   : '#bdae93',             # gruvbox: light4
             'p'   : '#bdae93',             # gruvbox: light4
             'w'   : '#bdae93',             # gruvbox: light4
+            'ges'   : '#bdae93, bold, italics',             # gruvbox: light4
         }
     }
 }
diff --git a/zulipterminal/themes/gruvbox_light.py b/zulipterminal/themes/gruvbox_light.py
index e477a9f..dd2c6e4 100644
--- a/zulipterminal/themes/gruvbox_light.py
+++ b/zulipterminal/themes/gruvbox_light.py
@@ -92,6 +92,7 @@ META = {
             'n'   : '#93A1A1',             # gruvbox: light4
             'p'   : '#93A1A1',             # gruvbox: light4
             'w'   : '#93A1A1',             # gruvbox: light4
+            'ges'   : '#bdae93, bold, italics',             # gruvbox: light4
         }
     }
 }

Im not sure however of the color choice. Any thought?

@neiljp
Copy link
Collaborator

neiljp commented Oct 1, 2023

@laomuon Thanks for the detailed bug report and suggestion!

I'm not in a position to manually debug the code right now, though it was a little surprising since gruvbox - and other themes - have worked for me recently. However, after a little dig it looks like this might be due to pygments/pygments#2444, which was new in 2.16.0. Since we only specify pygments>=2.14.0, I suspect that this may be the underlying cause for you.

A short term fix for main - and yourself - might be to install 2.15.x instead, ie. pygments~=2.15.1 or pygments<2.16.0.

Based on the pygments PR, I suspect this may impact multiple or all of our themes, so to unblock pygments>=2.16 we likely want to improve the translation from pygments to urwid. We could try using bold, italics overrides for that entry or others in each theme - if that works, without specifying a color? - but ideally it would be good to avoid lots of unnecessary patches to themes which might be resolved by a more general fix. It is also possible that our current overrides are only necessary due to an issue like this, but that'd be something that would need digging into further.

Thanks again - any feedback on the above would be great!

@neiljp neiljp modified the milestones: 0.6.0, Next Release Oct 1, 2023
@neiljp neiljp added the high priority should be done as soon as possible label Oct 4, 2023
neiljp added a commit to neiljp/zulip-terminal that referenced this issue Oct 4, 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.

Longer-term we should improve the pygments to urwid translation logic to
allow these styles to work and an upgrade to later pygments versions,
but for now this allows these themes to continue working as before.

Fixes zulip#1431.
@neiljp
Copy link
Collaborator

neiljp commented Oct 4, 2023

@laomuon Following up on this, I can confirm locally:

  • This affects gruvbox_dark, gruvbox_light and zt_light
  • This issue does not occur with pygments 2.15.1, but does on 2.16.x
  • Your suggested fix does work, as does bold, italics

I'd like to get this working ASAP first, so I'm capping pygments via #1433 for now.

@neiljp neiljp closed this as completed in 2d25ba9 Oct 4, 2023
@neiljp neiljp changed the title Gruvbox themes doesnt work Gruvbox & zt_light themes don't work with pygments 2.16.x Oct 4, 2023
@neiljp
Copy link
Collaborator

neiljp commented Oct 4, 2023

@laomuon Thanks again for the report - the workaround should solve this for now. If this doesn't fix it or you have other problems or ideas, please continue to file issues or come to chat in #zulip-terminal on chat.zulip.org :)

rsashank pushed a commit to rsashank/zulip-terminal that referenced this issue Oct 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.

Longer-term we should improve the pygments to urwid translation logic to
allow these styles to work and an upgrade to later pygments versions,
but for now this allows these themes to continue working as before.

Fixes zulip#1431.
Subhasish-Behera pushed a commit to Subhasish-Behera/zulip-terminal that referenced this issue Oct 27, 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.

Longer-term we should improve the pygments to urwid translation logic to
allow these styles to work and an upgrade to later pygments versions,
but for now this allows these themes to continue working as before.

Fixes zulip#1431.
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.
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 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 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.
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants