search

martanne / vis

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

Theming is sometimes partially applied or ignored #1151

Closed lobre closed 3 months ago

lobre commented 8 months ago

It seems I cannot define the foreground color of a visual selection.

lexers.STYLE_SELECTION = 'fore:yellow'

This above does not work. Not sure if the problem is in this repo or in https://github.com/orbitalquark/scintillua/

lobre commented 8 months ago

Thank you for the fix. It also seems to me that the "matching pair" is colored using the style of STYLE_SELECTION. However, the foreground color is not applied either, despite your above patch.

rnpnr commented 8 months ago

Sorry I knew about that but I didn't actually check if this fixed it. I'll look into it as well.

rnpnr commented 8 months ago

Fixed again! A lot of that code looks like it could do with a bit of a refactoring. Most of it is just duplicated in multiple places.

lobre commented 8 months ago

Thank you a lot for those quick fixes!

I am currently building a simple 16-color theme so I discover those small issues during the process.

Allow me to report another small one. Here is an extract of the theme:

lexers.STYLE_IDENTIFIER = 'fore:white'
lexers.STYLE_OPERATOR = 'fore:white' -- this one includes parenthesis
lexers.STYLE_STRING = 'fore:yellow'
lexers.STYLE_SELECTION = 'back:white,fore:black'

And let's take this example of code:

vis_code_noselect

You can see that labels layoutDatetime, layoutDate and layoutTime match STYLE_IDENTIFIER. Then the quoted dates match STYLE_STRING while the parenthesis match STYLE_OPERATOR.

But if I go into visual selection, the fore:black is not applied on labels and parenthesis, while it applies to the strings. See in action:

vis_code_select

I don't know if this is as trivial as the others.

I don't know also the extensive list of items for which selection does not apply. In my opinion, the selection should always override what is on the other styles.

Also, it seems putting reverse for the cursor does not work. It stays back:white and fore:black. For me it would reverse the colors of the underlying style under the cursor. For my initial screenshot, when on the date strings, it would turn back:yellow, fore:black. But I may misunderstand it.

lexers.STYLE_CURSOR_PRIMARY = 'reverse'
rnpnr commented 8 months ago

Thank you a lot for those quick fixes!

Actually I think I introduced a different bug. Now if you don't set the foreground color for STYLE_SELECTION it changes everything to the terminal foreground color instead of keeping the underlying foreground color.

In my opinion, the selection should always override what is on the other styles.

I agree. If STYLE_SELECTION has anything set and you make a selection it should apply. Otherwise it should just keep the underlying style.

Also, it seems putting reverse for the cursor does not work.

I've also noticed that the attributes (reverse, bold, italic, blink) don't always get applied.

All in all that code is very crusty. It needs to be broken out into a separate "apply style" function so that fixes can be made in one place instead of all over.