Closed neutaaaaan closed 1 year ago
I think Pmenu/PmenuSel clashes too much with Search:
rant: It took me more than 20 minutes of trying things out to realize what I was seeing in those screenshots.
Why wildoptions=pum
even exists is beyond me, that's contextually completely different from the buffer being worked on and should never be allowed to overlap with it in any significant way. This is downright horrible ui/ux.
Playing around with permutations of the search colors doesn't really help here, the box itself would need a border to separate contexts.
In a normal editing context I don't really see that being an issue outside of pretty contrived examples.
Can't do anything about or for users that don't know about or use :nohlsearch
proactively.
If anything, I'd even argue the issue is that :nohlsearch
doesn't have its own mapping out of the box or in defaults.vim
tbh.
@neutaaaaan :)
Nonetheless, current quiet works better in this case:
I've seen the current version of quiet stomp DiffText
and DiffChange
in annoying ways that I did not anticipate initially. That seems more important to me than supporting a non-default wildoptions
parameter that should have used a semantically separate window (or a different way of arranging its contents) instead of piggybacking on Pmenu[xxx]
.
The "new" popup windows inheriting their settings from Pmenu
is a much bigger issue to me, and someone will have to climb into the code and decouple those systems. I'll take a look at it, but my C is the stuff of nightmares.
I am ok with this as long as it follows your vision.
Check with @romainl too.
I, too, am not a fan of wildoptions=pum
(and don't use it) but it is there, for better or worse, and we must handle it. Pmenu
being the same color as Search
is definitely a problem when search matches are highlighted and the popup menu comes over/beside them. There would be less conflict if using dark12
and uiblue
, IMO.
dark12
isn't different enough from uiblue
to make the change worth considering, and doesn't help at all inside diffs.
I can think of 4 possibilities right now:
Pmenu
on top of a popup window, but that's an issue with every colorscheme.CurSearch
to highlight the selection. It's ugly, but mostly in line with the 8/16c fallbacks.wildoptions=pum
and popup windows were implemented: decouple them from the popup menu so we can control their colors properly, whilst still using Pmenu
as the default fallback if the colorscheme doesn't specify them.The fourth solution is the only viable one in my opinion, but we'd then have to deal with the implicit assumption that the function/plugin authors will somehow clean up the output of whatever they're going to populate the popup window with
so that it's legible.
I assume that some vimscript magic could be performed to detect the colors being used, and swap out colors to at least get good enough contrast, but I highly doubt plugin authors will want to ship half of colortemplate
with their own stuff.
Hopefully, I missed something when I read the documentation for popup windows.
This should fix all of the outstanding issues with Pmenu[xyz]
.
I've tested just about every edge case I could think of and using a shade of light grey for Pmenu
's background in both versions seemed to work out better than using a darker shade for the dark version. It doesn't look as nice, but doesn't get confusing when the completion window is small and overlaps with text, or suffer from a lack of contrast when using a small font.
I don't believe anything links against the various Pmenu[xyz]
foreground settings, but I definitely could use some feedback if you know any built-in features that recycle those colors.
I'm also not quite sure about whether the behaviour of PmenuThumb
and PmenuSbar
is consistent between t_Co
s anymore but quite frankly I'm somewhat brainfried from messing with this on and off during the day and could use another pair of eyes.
Line numbers are marginally brighter in the dark version. This is a late christmas gift for @romainl and @habamax :smile:
I like it
Do you think VertSplit
could have bg as NONE
? One less thing to override if your normal bg is different.
@habamax depends on how likely it is to blow up in the user's face because of their terminal emulator or their environment.
This still happens for example.
@neutaaaaan ah, that urxvt.
Anyway, this is ok to include in my opinion.
We should probably use this opportunity to document which groups can or even should have an empty background, and which shouldn't then.
Implement
![light](https://user-images.githubusercontent.com/35198994/210759342-05fd7266-b0ae-4b1f-82c2-812e9dfa61c6.png)
CurSearch
.Rebase
![image](https://user-images.githubusercontent.com/35198994/210759895-16064050-c010-4e9a-9bd1-a314fb1e48ec.png)
QuickFixLine
onCursearch
. RebasePmenu
/PmenuSel
and amenities onSearch
/Cursearch
. This can result in clashes if the user doesn't use:nohlsearch
proactively, but I figure it's a better compromise than piggybacking on diff colors. Example of a worst case scenario:Specific to the 8/16c versions: Disambiguate
![image](https://user-images.githubusercontent.com/35198994/210761114-fae0157f-c884-412d-bbc4-f447faa164bb.png)
Error
andTodo
. There is no way to do this without clashing with diffs or search results. I figure errors are less likely to happen inside diffs. Not great, not terrible.@habamax Do you think these changes would clash with
PopupNotification
andMessageWindow
?