-
-
Notifications
You must be signed in to change notification settings - Fork 176
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): allow adding custom highlights #166
Conversation
@aspeddro sorry for the radio silence on this PR just been busy with a bunch of things it's next on my list to review though |
@akinsho, alright. I'm making some adjustments on highlights. |
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.
@aspeddro thanks for the PR had a read through and have made some comments on tweaks that I think would be good
lua/toggleterm/config.lua
Outdated
Normal = { | ||
guibg = colors.shade_color(colors.get_hex("Normal", "bg"), constants.shading_amount), | ||
}, | ||
NormalFloat = { |
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.
@aspeddro can these be snake_case
rather than PascalCase, not sure why they are if it's because of converting the names, so they can be used in a map then you can use
-- convert 'highlight_value' to 'HighlightValue' -> snake to pascal
local formatted = name:gsub("_(.)", name.upper):gsub("^%l", string.upper)
I think this would look much tidier if the casing was consistent
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.
@akinsho keeping PascalCase makes mapping easier. One problem with the conversion is that it doesn't work for StatusLineNC
, you would have to use status_line_n_c
.
Another facility, passing the arguments to vim.highlights.create
is made simpler.
lua/toggleterm/ui.lua
Outdated
return hi_target | ||
end, vim.tbl_keys(term.highlights)) | ||
|
||
vim.api.nvim_win_set_option(term.window, "winhighlight", table.concat(highlights, ",")) |
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.
moved to window api
rather than assigning them separately lately
and collapse formatting
@aspeddro thanks for your work on this 👍🏿. I made a couple of tweaks to the documentation and some formatting but perhaps most significantly I made sure only |
users usually copy paste these examples without inspecting the a la stack overflow so I'd rather avoid a bunch of issues about the terminal being all black etc.
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.
LGTM
I switch between light mode and dark mode depending on the time of day (the rose-pine theme if it matters). Neovim handles this automatically at startup by doing some magic to detect the background of the shell and setting background to dark or light automatically. I did a manual bisect and before this patch, when I'd open a terminal, the background would always just match. However with this patch I get a dark background always. Can you please point me in the right direction so I can always get a matching background? |
@will the difference since this change is that rather than depending directly on X highlight, the plugin creates its own and derives them at startup from the theme. If you'd like to make a specific highlight follow another e.g. keep the background of the terminal matching the |
Thanks @akinsho for the tip! It ended up being for me:
to get it mostly working. Still need to do some tweaking I think, but now not super wrong. Just for the future if anyone else comes across these comments: |
This PR add support to highlights