-
-
Notifications
You must be signed in to change notification settings - Fork 611
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(#2415): highlight overhaul (#2454) #2455
Conversation
I noticed that By the way, I am working on adding type annotations to almost all functions, and I was wondering if would be worth it to do it now and deal with the conflicts with this branch or wait for this to be merged... What do you think? |
That's fantastic! It's been bothering me but I never seem to get a chance. The big one would be a Node class... lot's of surface area last time I experimented... You go first, this branch seems pretty localised and should merge without dramas. |
That's the only one I haven't added yet, I'll try my best.
Yeah, maybe I can skip the |
Oh wow! Sounds like you're making great progress... |
c5b0a0a
to
4700c61
Compare
After some testing I noticed that the "partial" line highlight isn't consistent with all icon types, namely the default file icon is not included in the line highlight whereas (probably?) all other icons are. I took a quick look at the code but I didn't find the issue, but you surely know better where to look. Edit: turns out it's related to the fact that Edit 2: turns out this "incomplete" line highlight is due to my |
Many thanks for all the detailed testing! That should probably be fixed before release - I'm sure you're not the only one. Can you please link your on_attach so that I can attempt a fix? I have little available time right now however I will over the new year so I'll get onto it then. |
This is my minimal on_attach = function()
vim.schedule(function()
local winnr = require("nvim-tree.view").get_winnr()
if winnr then
local ns = vim.api.nvim_create_namespace("NvimTree")
vim.api.nvim_win_set_hl_ns(winnr, ns)
vim.api.nvim_set_hl(ns, "Normal", { fg = "#F8F8F2", bg = "#272822" })
end
end)
end I use this namespace trick to have a different background color in nvim-tree windows.
What I meant here is that on master the current line highlight isn't interrupted, but the background color is not the one I specified in my Edit: I tested the master branch with |
I see, thank you. So long as I'm not causing a regression... |
22bacdd
to
4a24950
Compare
There is a problem with additive highlights, which "limit 2" does not address properly, as it removes highlighting that should not be, such as icon colour. :hi NvimTreeBookmarkHL guibg=blue
:hi NvimTreeCopiedHL guibg=magenta
:hi NvimTreeCutHL guibg=red vim.api.nvim_buf_add_highlight(0, -1, "DevIconC", 1, 2, 5)
vim.api.nvim_buf_add_highlight(0, -1, "NvimTreeBookmarkHL", 1, 2, 5)
vim.api.nvim_buf_add_highlight(0, -1, "NvimTreeCopiedHL", 1, 2, 5)
vim.api.nvim_buf_add_highlight(0, -1, "NvimTreeCutHL", 1, 2, 5) Icon is drawn in C blue, with blue bookmark HL. Move the DevIconC to the end and it's drawn in magenta copied HL. It seems that only one attribute may be overridden. Possible solution: build and apply dynamic highlight groups based on combining highlight groups. These could be cached. |
2569b24
to
94aa0de
Compare
4d0d1a1
to
11c0720
Compare
collapse Builder
Pulling the trigger. It's been stably dogfooded for 2 weeks. I have 3 days to deal with any regressions or questions. |
fixes #2415
@gegoune @Akmadan23 apologies for this very large change; breaking it up was not feasible.
I'd be most grateful if you could test this for a week or so...