-
Notifications
You must be signed in to change notification settings - Fork 45
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
Common tree highlight groups #158
base: main
Are you sure you want to change the base?
Conversation
Can you please rather make the changes from the dev branch? |
@5-pebbles I forgot this was going to happen with our dev setup. I assume it's fine having people submit PRs from the dev branch. |
@bini-x Wait, leave this PR as is for now. |
Okay |
@5-pebbles Okay I am going to have to rethink that lol. @bini-x I see your editor formatted the entire file, please undo that. |
Ok |
Can you check it now @AlexvZyl |
Run this in the root of nordic:
You might have to install stylua first. |
I just saw what my formatter did. sorry. i fixed it all now |
Great, now can you share a preview of the new changes? |
@AlexvZyl any comments? |
Will reply, gonna go mountain biking |
Definitely like iteration two more :) |
Check the tests. Somewhere you misspelled @5-pebbles These tests are lovely |
Ah no, you used |
Oh okay. What's the name of the first orange in palette? |
I'll try to find and I'll fix it |
@AlexvZyl you can check it now, because I wasn't sure if these were groups or not |
That looks good. Will test it locally later. |
I actually don't use those tree plugins anymore. Would you mind sharing screenshots? |
yes, I can but just for NeoTree because I don't use nvimTree |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the merger of tree plugins a lot!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything looks good to me : )
@AlexvZyl have you seen the final result yet? |
I don't miss a thing :) I promise I will properly check when I get the chance. |
@5-pebbles what do you think about making the root directory's name bold? |
I actually really like that. Go for it! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That looks great!
👍🏼 To this. Just noticed the same issue. |
lua/nordic/groups/integrations.lua
Outdated
G.TreeRootFolder = { fg = C.gray4 } | ||
G.TreeRootName = { fg = C.fg, bold = true } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these two are the same thing?
lua/nordic/groups/integrations.lua
Outdated
G.TreeFileIcon = { fg = C.blue2 } | ||
G.TreeFileNameOpened = { fg = C.fg } | ||
G.TreeSpecialFile = { fg = C.magenta.bright } | ||
G.TreeGitConflict = { fg = C.magenta.bright } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Conflict rather red?
G.TreeGitModified = { fg = C.git.change } | ||
G.TreeGitDirty = { fg = C.gray4 } | ||
G.TreeGitAdded = { fg = C.git.add } | ||
G.TreeGitNew = { fg = C.gray4 } | ||
G.TreeGitDeleted = { fg = C.gray4 } | ||
G.TreeGitStaged = { fg = C.gray4 } | ||
G.TreeGitUntracked = { fg = C.orange.base } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO these "identifiers" should either:
- All be subtle and grey (I prefer this)
- Or have their respective colors.
@5-pebbles Opinion?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would personally prefer colors as the default.
My repos are usually an unstaged mess, and the colors help prevent the spread of chaos, but that's more of a "me problem".
I think either way works; the people who don't like whatever we pick will just change it in their own configs. Though, it would be worth adding the unchosen variation to the list of common configurations that I plan on writing someday.
TLDR: I don't know which is better...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
which gray do you think should they be?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think most people expect colors. So go with that.
lua/nordic/groups/integrations.lua
Outdated
G.TreeGitDeleted = { fg = C.gray4 } | ||
G.TreeGitStaged = { fg = C.gray4 } | ||
G.TreeGitUntracked = { fg = C.orange.base } | ||
G.TreeTitleBar = { fg = C.black0, bg = C.blue2, bold = true } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should maybe link to Winbar?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it just link = "Winbar", because I can't find anything with the name "Winbar" in this file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it just link = "Winbar", because I can't find anything with the name "Winbar" in this file
Yup, that should work:
nordic.nvim/lua/nordic/groups/native.lua
Line 179 in 1ee4044
G.WinBar = { bg = C.bg_dark, fg = C.gray5 } |
For future reference if you click the search bar on GitHub via this pr it will search the current repos contents. You can do the same locally with something like ripgrep or telescopes built in ripgrep picker.
lua/nordic/groups/integrations.lua
Outdated
G.TreeGitStaged = { fg = C.gray4 } | ||
G.TreeGitUntracked = { fg = C.orange.base } | ||
G.TreeTitleBar = { fg = C.black0, bg = C.blue2, bold = true } | ||
G.TreeFloatBorder = { fg = C.blue2, bg = C.gray0} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should use our float colors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you tell me the float colors?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can find the extended palette values and their definitions here:
nordic.nvim/lua/nordic/colors/init.lua
Lines 59 to 63 in 1ee4044
-- Floating windows | |
C.bg_float = (options.transparent.float and C.none) or ((options.swap_backgrounds and C.gray0) or C.black1) | |
C.fg_float = C.fg | |
C.bg_float_border = C.bg_float | |
C.fg_float_border = C.border_fg |
lua/nordic/groups/integrations.lua
Outdated
G.TreeIndentMarker = { fg = C.gray4 } | ||
G.TreeSymlink = { fg = C.blue2 } | ||
G.TreeFolderName = { fg = C.blue1 } | ||
G.TreeWinSeparator = { fg = C.bg_dark, bg = C.bg } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should link to the existing win sep.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your reply, Alex!
Couldn't reply earlier because I had some other things to do.
I'll see what I can do and I'll try to finish it as soon as I can.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i can't find the existing window separator
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just use: link = "WinSeparator"
see definition:
nordic.nvim/lua/nordic/groups/native.lua
Line 135 in 1ee4044
G.WinSeparator = { fg = C.border_fg, bg = C.border_bg } -- the column separating vertically split windows |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, should just have told you what the hl is 😂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Np, I'm thinking now how could I not find the window separator, it was there the whole time...😅
@AlexvZyl added the requested changes, hopefully I didn't forget anything |
i'm not so sure about line 116 tho |
Sorry for disappearing, again... been a rough year. Will check the changes again when I am back at my PC (around 2 Jan). |
Closes #157.