Bug: onyo cat syntax highlighting is unreadable in some terminals

[aqw]

Depending on the terminal color scheme, the syntax highlighting can be either wonderful, or entirely illegible.

The color theme should be configurable by the user. Rich.Syntax uses Pygments Styles.

I propose adding a flag and config option:

onyo cat --style monokai <asset>

And the config:

[onyo "cat"]
	style = monokai

The current default (inherited from Rich) is monokai.

It may be necessary also to look at Rich.Syntax's background_color:

background_color (str, optional) – Optional background color, or None to use theme color. Defaults to None.

In addition to available styles, we should allow 'none' (or 'disable') to turn off syntax highlighting in cat.

To list available styles:

In [1]: from pygments.styles import get_all_styles
In [2]: list(get_all_styles())
Out[2]: 
['abap',
 'algol',
 'algol_nu',
 'arduino',
 'autumn',
 'bw',
 'borland',
 'coffee',
 'colorful',
 'default',
 'dracula',
 'emacs',
 'friendly_grayscale',
 'friendly',
 'fruity',
 'github-dark',
 'gruvbox-dark',
 'gruvbox-light',
 'igor',
 'inkpot',
 'lightbulb',
 'lilypond',
 'lovelace',
 'manni',
 'material',
 'monokai',
 'murphy',
 'native',
 'nord-darker',
 'nord',
 'one-dark',
 'paraiso-dark',
 'paraiso-light',
 'pastie',
 'perldoc',
 'rainbow_dash',
 'rrt',
 'sas',
 'solarized-dark',
 'solarized-light',
 'staroffice',
 'stata-dark',
 'stata-light',
 'tango',
 'trac',
 'vim',
 'vs',
 'xcode',
 'zenburn']

Open Question:

• what should be the default?

[TobiasKadelka]

I like the ideas for the flag and the option, it makes a lot of sense to me.

I have no strong oppinions about default colors. Of course it makes sense to select something that works for most configurations, i.e. a strong contrast between writing and background, and there are still discussions in the chat going on about transperancy, but in the end... This might be a good oportunity for us to test in the group to usage of local config files.

[bpoldrack]

I think the default should actually be something that does not use colors, but only bold, underlined, italic .. attributes.
It's the only thing that should work everywhere and for everyone.

We need a pretty generic config mechanism for this and figure out how to also apply that to other outputs including help.
So, we may end up with more generic config-override options. However, --style could then become an alias, so I'm fine with it as a quick solution for cat right now. But figure out how we want to implement it more generally before spreading that option to other commands.

[aqw]

I think the default should actually be something that does not use colors, but only bold, underlined, italic .. attributes.
It's the only thing that should work everywhere and for everyone.

I agree.

We need a pretty generic config mechanism for this and figure out how to also apply that to other outputs including help.

Oof. Good point. That one... will be difficult and may require its own issue. @j-leo you're the one running a terminal with a bright background. How is the legibility of the text (and colorized text) in onyo set --help?

Looking at the highlighting in a complex one (onyo set --help, the color style changes between the help text and the code blocks. There's already room for improvement and standardizing there.

[j-leo]

I think the default should actually be something that does not use colors, but only bold, underlined, italic .. attributes.
It's the only thing that should work everywhere and for everyone.

I agree.

We need a pretty generic config mechanism for this and figure out how to also apply that to other outputs including help.

Oof. Good point. That one... will be difficult and may require its own issue. @j-leo you're the one running a terminal with a bright background. How is the legibility of the text (and colorized text) in onyo set --help?

Looking at the highlighting in a complex one (onyo set --help, the color style changes between the help text and the code blocks. There's already room for improvement and standardizing there.

onyo set --help looks good IMO:

[aqw]

I looked into this, and have this "working", except... it fails in piping.

So, short form is that the bw style is exactly what we want, and will work everywhere. That should be the default, and a config option added for users to override.

The problem is that Rich's Syntax is doing something wrong here. It's either:

1. forcing the background where there is none (and unhelpfully having None mean "use default background of the style").
2. not using Terminal256Formatter where it should.

I'm not sure which is the actual issue yet. I'm looking into it.

The current workaround is to use pygments directly (which is what Rich is using):

from pygments import highlight
from pygments.formatters import Terminal256Formatter
from pygments.lexers import YamlLexer

And then replace

highlighted_text = Syntax('', 'yaml').highlight(asset_text)

with

highlighted_text = highlight(asset_text, YamlLexer(), Terminal256Formatter(style='bw'))

This works wonderfully, except it doesn't get to use Rich's nice auto-detection of pipes, etc, so all those situations break.

I did find an upstream bug that is exactly our problem. It doesn't really dig into the underlying problem though. I will look into this problem further, and either comment upstream, or open a PR.

Textualize/rich#3144

[aqw]

Now that I look closer, though Rich is using pygments, they are not using pygments.highlight() (as I had assumed). Figuring out the fix will require more digging.

[aqw]

The beautiful thing about the pygments.highlight() solution is that it somehow detects the background color and inverts it as needed.

It's unclear to me if I can tickle Syntax() to behave the same way.

[aqw]

Hah. I discovered background_color='default', which is not documented in the API docs but is mentioned elsewhere in RTD. 'default' meaning "use the console's background" (aka: none) is not to be confused with the "default" being None, which does not mean "no background" and rather means "use the default background of the theme.

Sigh.

So, that's one problem resolved. Now the issue is that the color does not invert as needed.

[aqw]

The issue is not so much that it doesn't invert, but rather that the color is set at all.

The bw theme (and the custom one I'm playing with) set no colors. They merely try to bold, italics, etc.

I have isolated the line that hardcodes this: https://github.com/Textualize/rich/blob/43d3b04725ab9731727fb1126e35980c62f32377/rich/syntax.py#L165

color="#" + color if color else "#000000",

IMO, this default should either be None or self._foreground_color. I will look into creating a PR upstream, perhaps next week.

[aqw]

I have opened a PR upstream: Textualize/rich#3582