-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Warn that text_fg_override_threshold can cause confusing issues with block characters #6767
Comments
Not worth the effort to implement per character overrides not to mention Feel free to send a PR with an appropriate warning added to the text of |
I was also thinking that it might make sense to have a few keymaps for things that might help with figuring out various issues, or maybe one key that switches through a few different settings after a short delay. Currently this seems only possible if multiple settings files are used, in which case keeping the settings you don't want to change in sync between multiple files is more of a challenge (as is updating them as features are added and changed). I suggest three new mappable actions: The first could be used to map changing any setting (except those that don't work with config file reloads) while the second would set to value1 unless the current setting is equal to value1 in which case value2 would be used (where value1 and value2 are multiple arguments if needed, I guess they should all need to be the current setting for value2 to be used). unset would set name to the default value. To make longer settings changes possible maybe combine could allow newline as a separator and then end on an empty line so that you could do something like: I guess action_alias could be used to make everything fit on one line but it seems easier to be able to put everything in one place. I don't know python but might be able to learn enough to make a patch. Another idea I had along those lines is config file variants, where load_config_file could be given a --variant argument and allow settings to be given multiple times for different variants (but for each setting the no variant specified version is used unless the current variant has a different value for that setting). So you could do something like: |
You have include, use it in your config files to establish common And yes, you can make a PR to allow load_config_file to accept an |
Is your feature request related to a problem? Please describe.
I noticed that pastel uses unicode block characters (particularly U+2580 ▀ Upper Half Block) to display a checkerboard pattern around the displayed color (which works even with text_fg_override_threshold set to 40) but then also for the displayed color with foreground and background set to the same color (which ends up with white or black lines in the color patch). I have a patch that I'll submit to pastel unless you want to make an exception to this override for non-text characters.
Describe the solution you'd like
I love the text_fg_override_threshold feature since lots of programs print unreadable combinations, although I worry that if this kind of graphics issue is common it might cause confusing bug reports here and elsewhere (it took me a bit to figure out the issue). The best option I can think of is adding something like "WARNING: if you use this option make sure to try turning it off if you notice ANY kind of graphics issues." to the end of the comment below the option (and maybe something in the FAQ as well).
Describe alternatives you've considered
Possibly it might make sense to not override for non-text characters, although I'm not sure how easy that would be to determine and I suspect that sometimes it would be better to make the non-text characters more visible as currently happens.
Additional context
While I didn't try different themes, I found text_fg_override_threshold set to 35 was slightly less than easily readable with my color settings (testing with ansi --color-table) while 40 was good. I suspect 40 is high enough that it is likely to work well in terms of readability with most if not all themes and might be worth mentioning as a possible starting point.
The text was updated successfully, but these errors were encountered: