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

Higlight group definitions with discriminators and without variant restrictions check has('gui_running') (v3) #83

Closed
ghost opened this issue Feb 7, 2024 · 9 comments

Comments

@ghost
Copy link

ghost commented Feb 7, 2024

In a terminal with true colors, has('gui_running') returns false, but it should be able to render the guifg/guibg colors. Perhaps I am misunderstanding, but the conditional check seems unnecessary since all other gui highlight definitions appear at the top level.

For example,

#const bool = get(g: 'my_bool', 0)

SpellRare omit omit s=colorA undercurl
ConditionalGroup -> omit
ConditionalGroup +bool 1 colorA omit

generates

const bool = get(g:, 'my_bool', 0)

SpellRare guisp=#abcdef gui=undercurl

if has('gui_running')
  if bool == 1
    hi ConditionalGroup guifg=#abcdef guisp=none gui=none
  endif
endif

The gui_running check seems to be incorrect; I think just the boolean check would be appropriate.

@ghost
Copy link
Author

ghost commented Feb 7, 2024

#76 (comment)

This is actually insightful. The default should probably be top-level and then an explicit /gui does the gui_running check? Although that doesn't really make sense if /gui isn't needed anywhere else to generate the true color entries... what is the purpose of /gui?

Edit: Maybe make gui an optional variant that generates a whole gui_running section, actually? I'm just rambling at this point, but hopefully it helps you get to the right conclusion.

@ghost ghost changed the title Higlight group definitions with discriminators and without variant restrictions check has('gui_running') Higlight group definitions with discriminators and without variant restrictions check has('gui_running') (v3) Feb 7, 2024
@lifepillar
Copy link
Owner

Thanks for the feedback! And thanks for your patience with v3, which is currently lacking thorough documentation.

There are two parts to the issue you are raising: one is the intended semantics of the template (correctness), the other is how the code is generated (optimization).

Regarding the semantics: Colortemplate supports a gui variant (for which it generates code by default) and an optional number of terminal variants (256 colors, 16 colors, etc…). The gui variant is, well, for GUI Vim. So, the template you are showing does not support any terminal, regardless of the number of colors.

If you add Variants: 256, then you are declaring that the template must also support terminals with at least 256 colors. So, Variants is mandatory if you want to support true-color terminals, and, afaict, Colortemplate generates code that conforms to the template's semantics, so I wouldn't call it “incorrect”.

Then, there's the code generation. You are right that checking for has('gui_running') is redundant in your case. Code generation does not perform any, or almost any, optimization at the moment because my focus is currently on nailing the template's syntax and semantics (I think we're almost there, but some adjustments are still possible—I welcome suggestions).

what is the purpose of /gui?

/gui is used to override a definition only for GUI Vim. It is mainly useful when combined with a discriminator, and of course it serves no purpose if your only variant is gui. For instance (from Gruvbox 8):

Variants: 256 16 8 0
MatchParen                           none   bg2           bold,underline
            /gui/256    +bold 0      omit   omit s=omit   underline

Here, MatchParen is bold by default, but bold may be disabled in the GUI and capable terminals (but not in terminals with 16 colors or less). Another example is the convoluted definition of SpellBad in Solarized 8:

Variants: 256 16 0
SpellBad                         violet            none  s=orange    undercurl
       /256/16                   violet            none              underline
             +visibility "high"  violet            base3             underline,reverse
                         "low"   violet            none              underline
       /gui  +visibility "high"  violet            base3 s=red       undercurl,reverse
                         "low"   violet            none  s=orange    undercurl

@ghost
Copy link
Author

ghost commented Feb 7, 2024

I currently have Variants: 256 16 so I do get three "sections" wherein one is top-level and the other two check t_Co. For the gui variant, however, the highlight group definitions with discriminators won't apply on the gui variant since has('gui_running') is false. The other highlight group definitions don't exist inside this check and do apply correctly.

In other words, :echo has('gui_running') is 0 in my terminal with true color support.

(Unrelated: how do you produce the nice formatting like that?)

@lifepillar
Copy link
Owner

has('gui_running') being false in (even true-color) terminals is expected. But if you have a Variant directive, then when you set termguicolors, things should work as specified by the template.

Can you post a minimal full template that shows the issue you have?

@ghost
Copy link
Author

ghost commented Feb 7, 2024

Full name: Minimal
Short name: minimal
Author: Me
Variants: 256

Background: dark

#const bool = get(g:, 'something', 0)

Color: base00 #abcdef 153
Color: base01 #012345 235

Normal base00 base01 s=omit omit

Normal +bool 1 base01 base00 s=base00 undercurl
vim9script

# Name:         Minimal
# Authors:      Me
# Last Updated: Wed 07 Feb 2024 06:00:11 PM EST

# Generated by Colortemplate v3.0.0-alpha0

set background=dark

hi clear
g:colors_name = 'minimal'
g:terminal_ansi_colors = []

const bool = get(g:, 'something', 0)

hi Normal guifg=#abcdef guibg=#012345

if has('gui_running')
  if bool == 1
    hi Normal guifg=#012345 guibg=#abcdef guisp=#abcdef gui=undercurl
  endif
endif

if str2nr(&t_Co) >= 256
  hi Normal ctermfg=153 ctermbg=235
  if bool == 1
    hi Normal guifg=#012345 ctermfg=235 guibg=#abcdef ctermbg=153 guisp=#abcdef ctermul=153 cterm=undercurl
  endif
  finish
endif

# vim: et ts=8 sw=2 sts=2

So in the example above

...
if has('gui_running')
  if bool == 1
    hi Normal guifg=#012345 guibg=#abcdef gui=NONE
  endif
endif
...

^ this part is incorrect, no? The 256 variant would correctly style Normal with the discriminator's alternative styling when the boolean is true, but the terminal true color variant would not. Only the GUI would see the difference here.

Edit: Updated build output... there was an error preventing it from building with just the 256 variant.

@lifepillar
Copy link
Owner

lifepillar commented Feb 8, 2024

Ok, let's consider all the cases:

  • GUI Vim: has('gui_running') is true and the block of the first conditional applies; no problems here, I assume.

  • Terminal with >=256 colors, with or without termguicolors: has('gui_running') is false, so the first conditional block is skipped (correct, this is not GUI Vim). But t_Co is greater than or equal to 256, so the second conditional block applies (correct, this is a terminal with at least 256 colors). If bool is 1 then the definition that is applied, overriding the default, is this:

  hi Normal guifg=#012345 ctermfg=235 guibg=#abcdef ctermbg=153 guisp=#abcdef ctermul=153 cterm=undercurl

If termguicolors is set, the attributes of this definition that are used are guifg, guibg, cterm, and ctermul.¹ ² If termguicolors is not set, the attributes of this definition that are used are ctermfg, ctermbg, cterm, and ctermul.

  • Terminal with <256 colors: not supported.

Does it make sense? Isn't this the actual behavior of this color scheme?¹

¹ Normal ignores cterm[ul] and gui[sp], so if you test the color scheme use another highlight group.
² *Note that termguicolors by definition uses the GUI attributes only for the foreground and background colors (see :help termguicolors).

(Unrelated: how do you produce the nice formatting like that?)

With patience :-) That's just manually indented.

@ghost
Copy link
Author

ghost commented Feb 8, 2024

Oh… that’s right (oops). Out of curiosity, why are the gui/cterm properties merged into one hi call there, but not above? Couldn’t gui/256 be merged into one set of highlight calls?

@ghost ghost closed this as completed Feb 8, 2024
@lifepillar
Copy link
Owner

The form of the generated code is not set in stone. It is well possible—likely, I'd say—that in the future the output format will change. One of the reasons I've started the v3 branch is that I wanted to refactor the code so that adding new code generators becomes relatively easy. In fact, there are already two generators: one for legacy Vim script, and one for Vim 9 script.

Couldn’t gui/256 be merged into one set of highlight calls?

The current state of affairs, which is not ideal yet, is partly due to the way Colortemplate has evolved over time. At the beginning, I thought that it would be a good idea to define only the attributes that are relevant for the current environment: only gui* for the GUI, only cterm* for terminal, etc. But things are not so simple: people may set or unsettermguicolor, and may also start a GUI from a terminal with :gui. Eventually, what were very distinct cases slowly started to converge into similar definitions.

So, to answer your question: yes, that's likely the direction Colortemplate will take. Providing full highlight group definitions with all the attributes may simplify some things, at the expense of a slightly slower, but likely imperceptible, loading time.

@ghost
Copy link
Author

ghost commented Feb 9, 2024

Fair enough. I know you've heard a lot from me lately, but I have no further qualms. I completed my first template with v3 and I'm very pleased... and I also find making colorschemes in this way somewhat addicting now. So thank you!

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant