[vim/vim] fix(swayconfig): floating_modifier (PR #14827)
James Eapen
Restores correct highlighting of swayconfig's floating_modifier with the normal and inverse options. Both i3 and sway have this keyword, but i3 does not have the normal|inverse. When this match was set as contained in 679f5ab, it prevented the sway options from being recognized.
Added the none option which was not supported but is a valid option in sway config.
Ideally floating_modifier would be a keyword as it is in i3config.vim, but I'm still working that out.
Fixes #14826
You can view, comment on, or merge this pull request online at:
https://github.com/vim/vim/pull/14827
Commit Summary
- 971d77b fix normal|inverse not being allowed in sway
- 42aa58f fix: allow none as standalone floating_modifier option
File Changes
(1 file)
Patch Links:
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
James Eapen
setting as draft since some issues remain:
image.png (view on web)
The first two lines should show bad normal as errors but do not.
Using
syn match i3ConfigKeyword /floating_modifier \(none$\|$[a-zA-Z0-9+]\+ \(normal\|inverse\)\)$/ contains=i3ConfigVariable,i3ConfigBindModkey,swayConfigFloatingModifierOpts
with the $ outside the alphanumeric match gets
image.png (view on web)
Here it looks better except that Mod keys aren't allowed anything following them.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
James Eapen
@jamespeapen pushed 1 commit.
- dbdf3e1 make floating_modifier option matching more precise
—
View it on GitHub.
You are receiving this because you are subscribed to this thread.
James Eapen
Made the capture groups a little more explicit:
image.png (view on web)
Let me know if you have any suggestions @JosefLitos. Otherwise I'm happy with this. Since both i3 and sway don't share too many different options for the same keyword like floating_modifier, I'm okay with the current fix.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Josef Litoš
This is a workaround to the issue, not actually fixing the root of it. The reall source of the problem is a change in i3config.vim floating_modifier implementation. I went from a match to a keyword, which takes precedence.
To make them work both, I'd suggest leaving the contained keyword in the swayconfig part and replacing i3config part with:
" 4.7 Floating modifier - syn keyword i3ConfigKeyword floating_modifier contained skipwhite nextgroup=i3ConfigVariable,i3ConfigBindModkey + syn match i3ConfigKeyword /floating_modifier [$a-zA-Z0-9+]\+$/ contained contains=i3ConfigVariable,i3ConfigBindModkey
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
James Eapen
@jamespeapen pushed 1 commit.
- 53cde3f fix: support floating_modifier none; revert broken highlighting
—
View it on GitHub.
You are receiving this because you are subscribed to this thread.
James Eapen
@jamespeapen pushed 1 commit.
- af34e45 sway/i3: bump versions
—
View it on GitHub.
You are receiving this because you are subscribed to this thread.
James Eapen
Thanks! I've reverted the i3 change that caused the issue @hiqua. For the precise matches, I think its more consistent to be precise since other syntax elements are precisely matched, but you have a point that its not going to be used more than once and vim needn't act as a sway linter. I'll go with the simpler option for now but will keep trying to see if there's another way to get that precision.
For a start I did notice two of the errors are recognized if there are any leading spaces:
image.png (view on web)
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
Thanks. So is this good to be included? Or are you still working on it? @JosefLitos can you please ACK as well please?
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
James Eapen
Yes its good to go.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Christian Brabandt
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
Josef Litoš
@JosefLitos commented on this pull request.
In runtime/syntax/i3config.vim:
> @@ -67,7 +67,7 @@ syn keyword i3ConfigBindKeyword bindsym bindcode contained skipwhite nextgroup=i
syn region i3ConfigModeBlock matchgroup=i3ConfigKeyword start=/mode\ze\( --pango_markup\)\? \([^'" {]\+\|'[^']\+'\|".\+"\)\s\+{$/ end=/^}\zs$/ contained contains=i3ConfigShParam,@i3ConfigStrVar,i3ConfigBindKeyword,i3ConfigComment,i3ConfigParen fold keepend extend
" 4.7 Floating modifier
-syn keyword i3ConfigKeyword floating_modifier contained skipwhite nextgroup=i3ConfigVariable,i3ConfigBindModkey
+syn match i3ConfigKeyword /^floating_modifier [$0-9A-Za-z]*$/ contains=i3ConfigVariable,i3ConfigBindModkey
I may have miscommunicated the changes in i3. The reverted changes in i3 introduce back the issue for which they were made - leading spaces.
We didn't need to put back the start-of-line anchor, but we should have kept in the contained keyword. I wrote the exact changes in the original comment, but a revert was probably not the best way to describe them.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Josef Litoš
For a start I did notice two of the errors are recognized if there are any leading spaces...
The reason leading spaces play a role is the inclusion of ^ in the i3config regex. Otherwise it would be also recognized as valid syntax. That is because of the simplified highlighting. Since that could be quite a common mistake to write it that way, a small simple change could be made for detecting the first character ->
i3config:
syn match i3ConfigKeyword /floating_modifier [$A-Z][0-9A-Za-z]*$/ contained contains=i3ConfigVariable,i3ConfigBindModkey
swayconfig:
syn match i3ConfigKeyword /floating_modifier \(none\|[$A-Z][a-zA-Z0-9+]\+ \(normal\|inverse\)\)$/ contained contains=i3ConfigVariable,i3ConfigBindModkey,swayConfigFloatingModifierOpts
(added [$A-Z] before the rest of the key/var match)
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.
James Eapen
Ah I did focus more on the revert and the sway suggestion than the i3 diff. Thanks @JosefLitos, and apologies @chrisbra the new pull request should supersede this one.
—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.