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

feat(highlights): neotest #29

Merged
merged 3 commits into from
May 25, 2024
Merged

feat(highlights): neotest #29

merged 3 commits into from
May 25, 2024

Conversation

windowsrefund
Copy link
Contributor

@windowsrefund windowsrefund commented May 10, 2024

Disclaimer: I have no clue what I'm doing and just pulled bits in from other support files.

Looks decent but I don't know how to remedy the "chicken/egg" scenario involving the background color of signcolumn vs. the background color of the neotest window.

neotest

@windowsrefund
Copy link
Contributor Author

I refactored this around what I found in this theme and ended up removing the dark SignColumn in order to get past the previously mentioned inconsistency. I hope you'll consider the change. This also has a nice side-effect of the first indentation line looking nicer since it isn't "mashed up against" the once-darker column.

neotest

@ramojus
Copy link
Owner

ramojus commented May 16, 2024

Hey, sorry for the delay, this looks great, but I have some ideas on how it could be improved. I will look into it sometime at the end of the month, as I will have much more free time by then.

@antoineco
Copy link
Contributor

antoineco commented May 21, 2024

I saw this PR right after posting #30. Sorry for the duplicate!

Maybe it would be a good idea to pick ideas from both implementations? For example, the colored NeotestFile and NeotestDir are quite nice IMO, and we can make use of the ui_green, ui_red, etc. colors for signs (without an explicit background, preferably).


before
Screenshot 2024-05-21 142338

after
Screenshot 2024-05-21 142101

@antoineco antoineco mentioned this pull request May 21, 2024
@@ -48,7 +48,7 @@ function M.set(hl, colors)
or (config.flat_background.line_numbers and hl.get('Normal').bg)
or colors.dark_bg
}) -- Line number for ':number' and ':#' commands, and when 'number' or 'relativenumber' option is set.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should leave this file unchanged. Default neovim setups have signcolumn to the left of numbercolumn, so this change will make it look inconsistent when signcolumn is between file-tree and numbercolumn, both of which have darker backgrounds.

@ramojus
Copy link
Owner

ramojus commented May 23, 2024

Hey @windowsrefund @antoineco, this is what I've arrived at after considering your suggestions:

    local config = require('mellifluous.config').config

    hl.set('NeotestAdapterName', { fg = colors.fg3 })
    hl.set('NeotestFocused', { fg = colors.fg, style = { bold = true } })
    hl.set('NeotestTarget', { style = { underline = true } })
    hl.set('NeotestIndent', { fg = colors.fg4 })
    hl.set('NeotestExpandMarker', { link = 'NeotestIndent' })

    hl.set('NeotestDir', { fg = hl.get('Directory').fg })
    hl.set('NeotestFile', { fg = colors.fg2 })
    hl.set('NeotestNamespace', { link = '@namespace' })

    hl.set('NeotestTest', { fg = colors.fg })
    hl.set('NeotestRunning', { fg = hl.get('Function').fg })
    hl.set('NeotestPassed', { link = '@markup.list.checked' })
    hl.set('NeotestFailed', { fg = colors.ui_red })
    hl.set('NeotestSkipped', { fg = colors.fg })
    hl.set('NeotestUnknown', { fg = colors.fg })
    hl.set('NeotestMarked', { bg = (config.is_bg_dark and colors.bg5) or colors.bg4 })

image

I'm not a neotest user, so please tell me if something could be better.

I like function for NeotestRunning, as it indicates action. NeotestSkipped and NeotestUnknown have normal fg, because I don't think they are important. NeotestMarked could be anything (e.g. folder, namespaceq), so I decided to not touch the fg for this one and instead made it seem almost like it's visually selected.
I'm not sure if there can be more than one NeotestAdapter for one project, so I made it a bit hidden (like root folder for file-tree). If there can be, we should probably link it to Title.

I have no idea what NeotestWinSelect is for, so please tell me if you know.

Edit: On second thought, we should only take the fg of Function for NeotestRunning (not link to it), as it wouldn't make sense to apply any possible style that Function could have to an icon. Updated the snippet.

@antoineco
Copy link
Contributor

I'm happy with those suggestions 👍
I have never encountered NeotestWinSelect either.

@windowsrefund
Copy link
Contributor Author

windowsrefund commented May 25, 2024

@ramojus Thank you for the help. Everything looks great but the background color in the signcolumn is not cohesive. That's what got me wanting to move away from the darker color. Here's where I'm at now after incorporating your update:

left
right

It just seems to me like the darker signcolumn isn't exactly adding any real value while adding 2 problems:

  1. This scenario where the background color of a thing would need to be different from the corresponding background color of the same thing shown elsewhere.
  2. The first horizontal indent line (I think that's the indent-blankline plugin) doesn't quite stand out and is somewhat mashed up against it

If you'd prefer to settle on this, I can surely look into treating the signcolumn color as a personal preference and try to figure out how to override it.

Thanks again

@antoineco
Copy link
Contributor

antoineco commented May 25, 2024

Just one addition: the non-homogeneous background color of Neotest signs in signs columns with darker background is a known issue:

IMO the colorscheme should ignore this and we should seek for a resolution upstream.


@windowsrefund you can set this value to true in your config if you prefer the number/signs/fold column to have the same background as the rest of the window:

flat_background = {
line_numbers = false,

@windowsrefund
Copy link
Contributor Author

@antoineco Thanks for pointing out the upstream issue. Couldn't agree more.

As for the flat_background options, I finally understand. That said, setting line_numbers = true makes it pretty much an all or nothing thing and not what I displayed above in the 2nd screen shot. That said, I think I'll break that concern out into a feature request at some point. This PR should be good to go.

Thanks everyone for all the help.

@ramojus
Copy link
Owner

ramojus commented May 25, 2024

Oh, I did not notice this issue before. @windowsrefund if you just want to customize SignColumn, you can also use this config:

highlight_overrides = {
    dark = function(hl, colors)
        hl.set('SignColumn', { bg = colors.bg })
    end,
}

This will get applied after all of the other highlights though, so GitSigns will now look out of place. Feel free to open an issue for this, there should probably be a better solution.

@ramojus ramojus merged commit 77fa9d0 into ramojus:main May 25, 2024
@windowsrefund
Copy link
Contributor Author

@ramojus Thank you for that tip. I had actually tried to configure that at some point but obviously failed. BTW, I just noticed both tender and kanagawa_dragon are not impacted by the gitsigns issue so I'm now happily using this with the latter. Looks great!

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

Successfully merging this pull request may close these issues.

3 participants