feat: add background highlight to LSP signature help

[AndreasNasman]

I used surface1 as the color, the same as those used by LspReferenceText, LspReferenceRead, and LspReferenceWrite.

This change helps to highlight the current parameter under the cursor when invoking vim.lsp.buf.signature_help.

Before the change

name: string has a peach foreground but the same background as the other parameters, making it hard to distinguish.

After the change

name: string has a peach foreground and a surface1 background, making distinguishing from the other parameters easier.

[igorlfs]

Hello! I was thinking about making a similar change for this group myself. I do agree that a background color would be nice, but I also think that by having a different background color, there's no need to set a foreground color at all. By not setting a foreground color, we can have a better context for the parameter using treesitter (that's specially nice when it comes to complex parameters). Another reason to not set a foreground at all is that there will always be some overlap between the foreground color and another highlight group for a given language (for instance, using the default peach as the fg, there's overlap with @constant.builtin, which shows up fairly often in parameters in typescript (as undefined)).

Currently, I'm using

                    LspSignatureActiveParameter = {
                        fg = mocha.none,
                        bg = mocha.surface2,
                        style = { "bold" },
                    },

Which shows up as

What do you think?

[AndreasNasman]

Your change looks great, @igorlfs. Your version makes it even more apparent which parameter is active.

A couple of questions came to mind:

1. Is the fg = mocha.none, part needed? I tested your version and removed it and couldn't spot a difference.
2. Which background color is most suited? I chose to use surface1 since LspReferenceText, LspReferenceRead and LspReferenceWrite use that background color, but surface2 also works great.
3. Could you explain this part in more detail?

   By not setting a foreground color, we can have a better context for the parameter using treesitter (that's specially nice when it comes to complex parameters).

[igorlfs]

Your change looks great, @igorlfs. Your version makes it even more apparent which parameter is active.

Thanks!

1. Is the fg = mocha.none, part needed? I tested your version and removed it and couldn't spot a difference.

I don't think that's needed. I assumed it was necessary since I was overriding the default one, but that doesn't seem to be the case

2. Which background color is most suited? I chose to use surface1 since LspReferenceText, LspReferenceRead and LspReferenceWrite use that background color, but surface2 also works great.

Either one is fine IMO

3. Could you explain this part in more detail?
   > By not setting a foreground color, we can have a better context for the parameter using treesitter (that's specially nice when it comes to complex parameters).

Using the same example I gave, but defining a foreground color:

To me, it looks way messier. Since the color falls back to treesitter, I find it more appealing / clearer what type is expected for the parameter.

[AndreasNasman]

Thanks for the answers! I updated the PR accordingly. I kept the background as surface1 to align with the other Lsp values.

[sgoudham]

This is pretty embarassing but I reviewed the wrong repository, will look at this PR later!

[vollowx]

LGTM!

[AndreasNasman]

Two checks are failing, but the same errors happen on the main branch.

[vollowx]

@AndreasNasman Don't worry about the failing checks, we'll fix them separately. Before merging this PR, another change is requested in order to follow the style doc: bg surface1 => surface0.

Sorry for the long delay, I'm always offline at weekdays.

[sgoudham]

Sorry it's taken me a while to come back to this.

The only reason why I wouldn't want to use surface1 is that it looks quite harsh on Latte.

Are you okay with changing it to surface0 ? @AndreasNasman

[AndreasNasman]

Thanks for the feedback 👍 I rebased on main and changed surface1 to surface0.