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

[RFC] window specific ui highlighting: part 2 #6700

Merged
merged 2 commits into from Jun 14, 2017

Conversation

Projects
None yet
3 participants
@bfredl
Member

bfredl commented May 8, 2017

continues #6597

If there is any good reason for not making 'highlight' read-only, please speak out now.

Some remarks: vsep highlight is determined by the window to the left. tabline highlight is determined by the last focused window in every tab. popupmenu and modemsg is highlighted according to the current window. Most msg stuff is not made window-specific. Syntax is a non-goal, there already is :ownsyntax .

@fmoralesc

This comment has been minimized.

Contributor

fmoralesc commented May 8, 2017

Btw, I just found another conflict with winhl: when characters are concealed and the lines wrap, the background of the cells after the break uses the original background.

captura de pantalla de 2017-05-08 20-43-59

@bfredl bfredl force-pushed the bfredl:winhl branch from 7c06982 to e7d8a73 May 8, 2017

@bfredl

This comment has been minimized.

Member

bfredl commented May 10, 2017

I wonder if the actual issue is not caused by winhl, it seems that the breaks are off with wrap+conceal anyway (both with and without linebreak), so it's stops drawing the line before it actually is done.

@bfredl bfredl force-pushed the bfredl:winhl branch from e7d8a73 to 1b80276 May 13, 2017

@bfredl

This comment has been minimized.

Member

bfredl commented May 13, 2017

No one has spoken out. I completed the 'highlight' read-only change.

@bfredl

This comment has been minimized.

Member

bfredl commented May 13, 2017

change to not merge overridden highlight with the global highlight. This seems both more flexible and matching expectations.

@bfredl bfredl force-pushed the bfredl:winhl branch from 3e2c724 to b52dc6d May 16, 2017

this option was set. Only builting ui highlights are supported, not
syntax highlighting. For that use |:ownsyntax|.
Most highlights occuring within the frame of a window is supported.

This comment has been minimized.

@fmoralesc

fmoralesc May 16, 2017

Contributor

Are supported

syntax highlighting. For that use |:ownsyntax|.
Most highlights occuring within the frame of a window is supported.
Highlight of vertical separators are determined by the window to the

This comment has been minimized.

@fmoralesc

fmoralesc May 16, 2017

Contributor

Is determined

Highlight of vertical separators are determined by the window to the
left of the separator. The highlight of a tabpage in |tabline| is
determined by the last focused window in the tabpage. Highlight of
modemsg and popupmeny are determined by the current window. Most

This comment has been minimized.

@fmoralesc

fmoralesc May 16, 2017

Contributor

Is

@bfredl bfredl force-pushed the bfredl:winhl branch 4 times, most recently from 42213ed to b66331c May 23, 2017

@bfredl bfredl force-pushed the bfredl:winhl branch from b66331c to 898fa2c Jun 3, 2017

@bfredl

This comment has been minimized.

Member

bfredl commented Jun 3, 2017

Added tests and fixed lint. Marking RFC.

@bfredl bfredl changed the title from [WIP] window specific ui highlighting: part 2 to [RFC] window specific ui highlighting: part 2 Jun 3, 2017

@bfredl bfredl force-pushed the bfredl:winhl branch 3 times, most recently from 6e6baae to 24d9db5 Jun 3, 2017

@bfredl bfredl force-pushed the bfredl:winhl branch 3 times, most recently from 91c96db to a171fc6 Jun 11, 2017

@bfredl

This comment has been minimized.

Member

bfredl commented Jun 13, 2017

All tests are passing. Any more comments, or should we merge this?

int w_hl_id_inactive; ///< 'winhighlight' id for inactive window
int w_hl_attr; ///< 'winhighlight' final attrs
int w_hl_id_normal; ///< 'winhighlight' id
int w_hl_attr_normal; ///< 'winhighlight' final attrs

This comment has been minimized.

@justinmk

justinmk Jun 13, 2017

Member

should these comments be moved to the items below?

@@ -0,0 +1,7 @@

This comment has been minimized.

@justinmk

justinmk Jun 13, 2017

Member

what's this file?

This comment has been minimized.

@bfredl

bfredl Jun 13, 2017

Member

because my git alias was broken

@@ -0,0 +1,126 @@
#ifndef NVIM_HIGHLIGHT_DEFS_H
#define NVIM_HIGHLIGHT_DEFS_H

This comment has been minimized.

@justinmk

justinmk Jun 13, 2017

Member

why not in syntax_defs.h?

This comment has been minimized.

@bfredl

bfredl Jun 13, 2017

Member

I moved them from globals to avoid an include loop. syntax_defs.h would be possible, but these are about ui highlights, not syntax.

This comment has been minimized.

@justinmk

justinmk Jun 13, 2017

Member

should we eventually move highlight logic to highlight.c? (I haven't looked to see if this makes sense, just wondering if you already thought about it)

This comment has been minimized.

@bfredl

bfredl Jun 14, 2017

Member

I started working on it before (#1933), but stopped as it was decided that syntax.c was "frozen". But we already have several separate _defs files even if the .c is not.

]])
end)
it('can override popupmen', function()

This comment has been minimized.

@justinmk

@bfredl bfredl force-pushed the bfredl:winhl branch from a171fc6 to a84e508 Jun 13, 2017

@bfredl

This comment has been minimized.

Member

bfredl commented Jun 13, 2017

Thanks, updated.

@justinmk

This comment has been minimized.

Member

justinmk commented Jun 13, 2017

either a really unlucky CI, or something is wrong.

@bfredl

This comment has been minimized.

Member

bfredl commented Jun 14, 2017

seems timer, retrying

@bfredl bfredl force-pushed the bfredl:winhl branch from a84e508 to c5735b6 Jun 14, 2017

@bfredl bfredl force-pushed the bfredl:winhl branch from c5735b6 to ad73a70 Jun 14, 2017

@bfredl bfredl merged commit 7918845 into neovim:master Jun 14, 2017

0 of 2 checks passed

QuickBuild Build pr-6700 is started
Details
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
@bfredl

This comment has been minimized.

Member

bfredl commented Jun 14, 2017

There might be more glitches, but the best way to find them is letting more people try it...

@bfredl bfredl referenced this pull request Oct 16, 2017

Merged

[RFC] News #153

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment