Use regular font style in sighelp popup if already highlighted by color scheme

[jwortmann]

The signature help popup uses bold & underlined font style for the active parameter, which looks a bit ugly in my opinion.
With this PR, it will keep the regular font style if the color scheme already defines different colors for active and non-active parameters. This is the case for example in my own color scheme, (though I will probably adjust the colors a bit if the PR gets merged).

Also the number of View.style_for_scope API calls is reduced by storing the results into variables and avoiding redundant calls.

[rchl]

Is it expected that there is no change with Mariana?

Before

After

[jwortmann]

Is it expected that there is no change with Mariana?

Yes, that is expected. An alternative to the bold & underline font style by default, would be to use a different scope for the active parameter color. But that would probably also complicate things, because you can't really be sure that it creates a sufficient contrast to the color for non-active parameters.

[predragnikolic]

Should we also add those rules to the https://github.com/sublimelsp/LSP/tree/main/ColorSchemes?

I noticed that I can spot the active parameter easier when the color is different from the inactive parameter.
then when the active param has bold and underline, but is the same color as the inactive.

        {
            "scope": "variable.parameter.sighelp.lsp",
            "foreground": "var(white3)"
        },
        {
            "scope": "variable.parameter.sighelp.active.lsp",
            "foreground": "var(orange)"
        },

[frou]

Has this change taken away the ability for someone to use custom sighelp colours and have bolding/underlining behaviour?

[predragnikolic]

Yes

[jwortmann]

Yes, this was the point of the change. If you had the custom sighelp colors before, the bold font style used to make the color appear a bit brighter (on dark color schemes), in comparison to regular font style. So if you prefer to keep a similar contrast as before, you might need to update your custom colors now.

Now that I think about it, we could in theory add an unused CSS class to the tag in that case, like this:

-        '; font-weight: bold; text-decoration: underline' if emphasize else '',
+        '; font-weight: bold; text-decoration: underline' if emphasize else '" class="active-parameter',

Then it would be the same default as it is now, but users could re-add the bold/underline via mdpopups.css if they want.

---

In regard to shipping with custom rules for the default color schemes, we could do that, but then I would do it for all 5 color schemes, and I would also drop the .lsp suffix from the scope selector in those rules, to keep user customizations working if they don't have this suffix.

[frou]

Thank you. To me, It's not all about contrast. The underline in particular made it very easy to pick out the active parameter at a glance.

Then it would be the same default as it is now, but users could re-add the bold/underline via mdpopups.css if they want.

Maybe I'm misunderstanding the relative importance of the HTML/CSS aspect, but wouldn't it be more correct to delegate the bold/underline decision to the Colour Scheme?

i.e. a rule could be:

{"scope": "variable.parameter.sighelp.active.lsp", "foreground": "var(green)", "font_style": "bold underline"}

and that desire for bold/underline would be reflected in the dict returned by view.style_for_scope("variable.parameter.sighelp.active.lsp"), just as the foreground colour already is.

[jwortmann]

wouldn't it be more correct to delegate the bold/underline decision to the Colour Scheme?

Actually not a bad idea. I think it would work, but we need to apply a little "trick" which is already used for the diagnostics unnecessary tag, to preserve the bold & underline when there is no color scheme rule for the sighelp-specific scopes (otherwise, the active parameters couldn't be distinguished at all anymore in the built-in and probably all third-party color schemes, because they don't have set bold & underline font style for those scopes by default). I think I can make a PR with that.

[jwortmann]

The "trick" that I had in mind was actually used for the semantic tokens, and not for diagnostic tags.

LSP/plugin/session_buffer.py

Line 679 in a6edba3

if token_general_style["source_line"] != token_type_style["source_line"] or \

I tried to implement using the font style from the color scheme for the active parameter, but there seems to be a bug in the style_for_scope API method, which makes parts of the results unreliable. In particular it doesn't seem to work correctly for the variable.parameter.sighelp.active.lsp scope when I tested it with my own color scheme. I filed a report on the ST issue tracker at sublimehq/sublime_text#5983.

Unfortunately this probably means that we're stuck here in regard to implementing such a solution, unless someone has another idea how to retain the bold & underline if there is no color scheme rule for the scope. Alternatively we could use the approach of the CSS class for now.

[frou]

Thanks for trying it. Let's see if that issue possibly gets a quick fix.