martanne / vis

A vi-like editor based on Plan 9's structural regular expressions
Other
4.19k stars 260 forks source link

RFC: ui: refactor style handling #1154

Closed rnpnr closed 3 months ago

rnpnr commented 7 months ago

Hi everyone, this is a pretty major change to the way styles are handled inside vis. I would really appreciate testing to make sure this doesn't cause major breakage with existing themes. Most likely this will cause settings that were silently ignored before to start working.

Also I would appreciate some tests with the default style. I tested it in the Linux virtual console and it seems to work great there. I have not tested with a transparent terminal emulator so I would appreciate if someone could let me know if it respects that or not.

Commit message follows:

The old style handling had a lot edge cases where one of the colours or the attribute wouldn't get applied correctly. This commit adds a new style_set() method to the Ui which should be called instead of manually touching a cell's style. This also means that the Cell struct can be made opaque since all the handling is now done inside the ui-terminal files.

With this it is now viable to combine the light and dark 16 colour themes into a single base-16 theme. This theme works very well with the Linux virtual console and will now be the default theme regardless of if the terminal supports 256 colours or not. This should address the common complaints about vis not respecting the users default terminal colours.

fixes #1151: Theming is sometimes partially applied or ignored see #1103: terminal no longer has transparency/opacity see #1040: Transparent background and setting options by default

lobre commented 7 months ago

Nice work! Just tested it rapidly and it seems way better than before!

I would like to try creating a full 16-color simple theme with your branch, to see if I manage to have something meaningful. It could help identify problems here.

For now, the default theme in my terminal is not that great, especially the contrast in selections. If I manage to have something simple but efficient, maybe I could then submit it as default to have something great out of the box, that would generally work great in major terminal emulators (with default palettes). Let's see.

lobre commented 7 months ago

Does it make sense to call it a 16-base while it only supports 8 colors? Isn't it the opportunity to bring the missing 8 bright variants?

mcepl commented 7 months ago

It didn’t apply completely cleanly. After some work with merge tools I got 0001-ui-refactor-style-handling.patch

rnpnr commented 7 months ago

Does it make sense to call it a 16-base while it only supports 8 colors? Isn't it the opportunity to bring the missing 8 bright variants?

This is kind of my bad. In the linux virtual console the bold attribute is applied by adding 8 to the underlying colour giving the bright variant. So from that perspective it is a 16 colour theme.

I'm not particularly tied to what is here. If you come with something better I would be fine with making that the default instead. I would prefer if it keeps working with the virtual console though.

It didn’t apply completely cleanly.

That is to be expected. This applies on top of current master not on top of the updated lexers.

mcepl commented 7 months ago

On Fri Nov 10, 2023 at 6:13 PM CET, Randy Palamar wrote:

Hi everyone, this is a pretty major change to the way styles are handled inside vis. I would really appreciate testing to make sure this doesn’t cause major breakage with existing themes. Most likely this will cause settings that were silently ignored before to start working.

Hmm, with my current setup and default theme I get dark yellow cursor on the light yellow background, so the cursor is more or less invisible.

Matěj

-- http://matej.ceplovi.cz/blog/, @@.*** GPG Finger: 3C76 A027 CA45 AD70 98B5 BC1D 7920 5802 880B C9D8

This conversation can serve no purpose anymore. Good bye. -- HAL9000 in 2001: Space Odyssea

rnpnr commented 7 months ago

Hmm, with my current setup and default theme I get dark yellow cursor on the light yellow background, so the cursor is more or less invisible.

Which theme is that? Some settings related to the cursor and selection used to be silently ignored so it could be an issue with the theme.

mcepl commented 7 months ago

Hmm, with my current setup and default theme I get dark yellow cursor on the light yellow background, so the cursor is more or less invisible.

Which theme is that? Some settings related to the cursor and selection used to be silently ignored so it could be an issue with the theme.

Light theme in my foot terminal and no theme in ~/.config/vis/visrc.lua (which I guess means default.lua theme, right?).

rnpnr commented 7 months ago

Light theme in my foot terminal and no theme in ~/.config/vis/visrc.lua (which I guess means default.lua theme, right?).

It looks like foot has a handful of different light themes, some with poor contrast between regular and bright. I'm not sure we can get it right for all possible combinations but I do think the theme can be modified. Can you send a screenshot of your terminal without vis loaded (but with an active cursor) and with vis loaded? Maybe I can at least infer what should be changed.

mcepl commented 7 months ago

It looks like foot has a handful of different light themes, some with poor contrast between regular and bright. I'm not sure we can get it right for all possible combinations but I do think the theme can be modified. Can you send a screenshot of your terminal without vis loaded (but with an active cursor) and with vis loaded? Maybe I can at least infer what should be changed.

Taking this back, I had some really weird (most likely my own work) theme, which is most likely buggy. Unfortunately, I cannot find anything I would like (I am a friend of Plan9/ACME-like very plain styles), but it is not fair to blame vis for it.

mcepl commented 7 months ago

Light theme in my foot terminal and no theme in ~/.config/vis/visrc.lua (which I guess means default.lua theme, right?).

It looks like foot has a handful of different light themes, some with poor contrast between regular and bright. I'm not sure we can get it right for all possible combinations but I do think the theme can be modified. Can you send a screenshot of your terminal without vis loaded (but with an active cursor) and with vis loaded? Maybe I can at least infer what should be changed.

screenshot-2023-11-23_23-11-1700779388

(left is vis with no theme set, right is just plain foot with bash running; both with acme foot style; cursor in vis is actually on line with swayidle, the letter s is slightly darker)

Actually, when using vis-minimal-theme/minimal-clear, it is much better:

screenshot-2023-11-23_23-11-1700779891

I am not sure why the default theme fails so miserably, but at least I can work now.

mcepl commented 6 months ago

To conclude my rambling:

Tested-by: Matěj Cepl mcepl@cepl.eu

rnpnr commented 6 months ago

To conclude my rambling:

Tested-by: Matěj Cepl mcepl@cepl.eu

Thanks, you have a valid point though. I don't want to make the default theme worse. I don't want to do what minimal-clear does though because the point is to only use the terminals colors.

Note for others looking at that theme: local clear = '#00000000' is invalid from vis' perspective and is treated as local clear = ''. Currently the parsing code only accounts for exactly 6 hex digits.

sansfontieres commented 6 months ago

vis.lexers.STYLE_CURSOR_LINE seems to steal the colours of the current line (but not the font style)

image image

(see the "-v" on the last picture)

rnpnr commented 6 months ago

One other issue I noticed, also related to dimming, is that when you have the cursor somewhere within a dimmed section the line to the right of the cursor gets reset to the normal attribute. See attached: image

mcepl commented 3 months ago

Couldn't we just merge this and #1133? I was running this for the last couple of weeks, and it seems to be working. I would hope that with merging these, we could get more testing and bigger chance to fix any remaining issues.

rnpnr commented 3 months ago

I would be ok with that. I've been using both of them for as long has they have been open and while I know of a couple of bugs in both of them they don't get in the way of editing in any way. I've been too busy with other things to track down them down. Most of them (like the screenshots above) are probably quite trivial.

I'll hold off for a couple days for any other opinions but I think its time.

mcepl commented 3 months ago

How long is “a couple of days” in your part of the world?