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

Fix/nvim 0.10 - Define CurSearch #121

Merged
merged 1 commit into from
May 31, 2024
Merged

Fix/nvim 0.10 - Define CurSearch #121

merged 1 commit into from
May 31, 2024

Conversation

roosta
Copy link
Member

@roosta roosta commented May 30, 2024

Fix/nvim 0.10 - Define CurSearch

Changes

Define CurSearch along the other search highlights, set it to use xgray/inverse, and be underline and bold. Tweak existing search groups to work with new CurSearch,

Seems some defaults changed or this hl group got added in nvim 0.10, CurSearch is the hlsearch word under the cursor. It was set to default colorscheme yellow, which messed with text selection, and don't work with how search is colorized in srcery.

Ref: #120

Screenshot

pr_cursearch

Define CurSearch along the other search highlights, set it to use
xgray/inverse, and be underline and bold. Tweak existing search groups
to work with new CurSearch,

Seems some defaults changed or this hl group got added in nvim 0.10,
`CurSearch` is the `hlsearch` word under the cursor. It was set to
default colorscheme yellow, which messed with text selection, and don't
work with how search is colorized in srcery.

Ref: #120
@roosta roosta requested review from milogert and MindTooth May 30, 2024 14:56
@MindTooth
Copy link
Member

Which versions have you tested this with? Vim/Nvim? I can help out to test.

@roosta
Copy link
Member Author

roosta commented May 30, 2024

I've only tested on nvim 0.10

@roosta
Copy link
Member Author

roosta commented May 30, 2024

Tested on nvim 0.9.5 and vim 9.1, no issues detected
EDIT: It was fairly quick to check, oversight that I didn't think of that.

@roosta
Copy link
Member Author

roosta commented May 30, 2024

I was able to identify a problem, it's not something introduced with this PR, but rather an issue we've had for a while.

if g:srcery_inverse == 1 && g:srcery_inverse_matches == 1

this expression forces user to use both visual inverse, and inverse matches, which is less than ideal, but if srcery_inverse is zero it will not let you use the s:inverse variable. So in essence you can't mix options, which would mitigate this issue.

A solution would be to introduce a new variable, since srcery_inverse controls both the visual selection, and if inverse is enabled at all. Something like let g:srcery_inverse_visual.

I think maybe this could go in a separate PR, as it isn't something related to nvim 0.10, and isn't an issue for most users, me included. I just use the default.

EDIT: zero, not non-zero.

@MindTooth
Copy link
Member

Nice find.

I agree that it could come later as a separate pull request. That way we can get this merged now.

@milogert
Copy link
Member

I finally had a chance to look at this. It looks good to me. But any chance for the CurSearch highlight to actually be different or is that just up to the end user to configure? I kind of like it myself, it's just the wrong yellow currently

@roosta
Copy link
Member Author

roosta commented May 31, 2024

I finally had a chance to look at this. It looks good to me. But any chance for the CurSearch highlight to actually be different or is that just up to the end user to configure? I kind of like it myself, it's just the wrong yellow currently

It is different, it's bold and got underline. The problem with setting it to yellow is that visual selection kinda breaks, not highlighting correctly, and it doesn't match the other search hl groups.

@milogert
Copy link
Member

Fair enough. I completely missed the underline 🤦‍♂️ thanks! This looks good

@roosta roosta merged commit c7398b0 into master May 31, 2024
1 check passed
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.

None yet

3 participants