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

spurious highlight when using match on character at end of soft-wrapped line #9242

Closed
lacygoill opened this issue Nov 29, 2021 · 5 comments
Closed

Comments

@lacygoill
Copy link

Steps to reproduce

Run this shell command:

vim -Nu NONE -S <(cat <<'EOF'
    vim9script
    &l:breakindent = true
    &l:linebreak = true
    set breakat+=]
    printf('%s]%s[xxx]%s', repeat('x', 70), repeat('x', 50), repeat('x', 70))
        ->setline(1)
    normal! 6e
    matchaddpos('ErrorMsg', [[1, 126]])
EOF
)

Notice that on the second soft-wrapped line, the highlight is applied from the cell occupied by the last character, up to the very last cell of the window.

Expected behavior

Maybe it's working as intended, but I think it's still distracting and misleading for the user. There are no real characters on the cells after the last ].

Shouldn't the highlight be applied only on the cell whose coordinates were passed to matchaddpos()? That is:

[1, 126]

And not:

[1, 126]
[1, 127]
[1, 128]
...

Operating system

Ubuntu 20.04.3 LTS

Version of Vim

8.2 Included patches: 1-3692

Screenshot

gif

Additional Context

I found this issue while using the matchparen plugin:

vim -Nu NORC -S <(cat <<'EOF'
    vim9script
    &l:breakindent = true
    &l:linebreak = true
    set breakat+=]
    printf('%s]%s[xxx]%s', repeat('x', 70), repeat('x', 50), repeat('x', 70))
        ->setline(1)
    normal! 4e
EOF
)

Same issue with a text property:

vim9script
&l:breakindent = true
&l:linebreak = true
set breakat+=]
printf('%s]%s[xxx]%s', repeat('x', 70), repeat('x', 50), repeat('x', 70))
    ->setline(1)
normal! 6e
prop_type_add('test', {highlight: 'ErrorMsg'})
prop_add(1, 126, {length: 1, type: 'test'})
@lacygoill
Copy link
Author

I found this issue while using the matchparen plugin

Which gives a confusing user experience when we move on a bracket whose matching pair is at the end of a soft-wrapped line:

gif

@lacygoill lacygoill changed the title spurious highlight when using match on character at end of wrapping line spurious highlight when using match on character at end of soft-wrapped line Nov 29, 2021
@lacygoill
Copy link
Author

The patch 8.2.3698 fixed the issue for a highlight installed by a match, but not for a highlight installed by a text property:

vim9script
&l:breakindent = true
&l:linebreak = true
set breakat+=]
printf('%s]%s[xxx]%s', repeat('x', 70), repeat('x', 50), repeat('x', 70))
    ->setline(1)
normal! 6e
prop_type_add('test', {highlight: 'ErrorMsg'})
prop_add(1, 126, {length: 1, type: 'test'})

In practice, this means that the Vim9 refactoring is still affected by the issue, because it uses text properties.

brammool added a commit that referenced this issue Nov 29, 2021
Problem:    Text property highlighting continues over breakindent.
Solution:   Stop before the end column. (closes #9242)
@lacygoill
Copy link
Author

The patch 8.2.3700 fixed the second issue, but it created a third one which is worse because it occurs much more frequently:

vim9script
&l:linebreak = true
"\t[xxx]"->setline(1)
prop_type_add('test', {highlight: 'ErrorMsg'})
prop_add(1, 2, {length: 1, type: 'test'})

Notice that all the cells covered by the tab character are highlighted, because it's in front of the opening bracket. Before the patch, only the bracket was highlighted.

I have no idea why 'linebreak' affects this issue. The line is too short to be wrapped anyway, so it shouldn't matter; but it does.

If it can't be fixed, I think it would be better to simply revert the patch, because it makes the matchparen refactoring much worse. This is especially noticeable when reading Vim's help pages, where lines are indented with tabs, and often start with brackets.

brammool added a commit that referenced this issue Nov 30, 2021
Problem:    Text property highlighting is used on Tab.
Solution:   Only set in_linebreak when not on a Tab. (closes #9242)
@lacygoill
Copy link
Author

The patch 8.2.3700 introduced a regression:

vim9script
&linebreak = true
&wrap = true
printf('%s+(%s)', 'x'->repeat(&columns / 2), 'x'->repeat(&columns / 2))->setline(1)
prop_type_add('test', {highlight: 'ErrorMsg'})
prop_add(1, (&columns / 2) + 2, {length: 1, type: 'test'})

gif

Notice that all the empty cells after the + character on the first soft-wrapped line are highlighted; only the opening parenthesis on the next line should be.

brammool added a commit that referenced this issue Dec 2, 2021
Problem:    When using 'linebreak' a text property starts too early.
Solution:   Decrement "bcol" when looking for property start. (closes #9242)
@lacygoill
Copy link
Author

Not sure whether it's the same issue, but here is another case where some cells are highlighted even though they are not occupied by any character in the buffer:

vim9script
printf('%s (:%s)', 'x'->repeat(&columns / 2), 'y'->repeat(&columns / 2))
    ->setline(1)
&linebreak = true
matchadd('ErrorMsg', '([^)]*)')

Notice that, on the first line of the screen, every cell after the colon is highlighted. But none of them is occupied by a character.


Same behavior with a syntax rule:

vim9script
printf('%s (:%s)', 'x'->repeat(&columns / 2), 'y'->repeat(&columns / 2))
    ->setline(1)
&linebreak = true
syntax on
syntax match Test /([^)]*)/
highlight link Test ErrorMsg

Same behavior with a text property:

vim9script
printf('%s (:%s)', 'x'->repeat(&columns / 2), 'y'->repeat(&columns / 2))
    ->setline(1)
&linebreak = true
prop_type_add('Test', {highlight: 'ErrorMsg'})
prop_add(1, searchpos('(', 'nW')[1], {length: getline(1)->strlen(), type: 'Test'})

As a workaround, we need to invoke matchaddpos(), and explicitly pass to the latter the coordinates of cells which are occupied by a character in the buffer:

vim9script
printf('%s (:%s)', 'x'->repeat(&columns / 2), 'y'->repeat(&columns / 2))
    ->setline(1)
&linebreak = true
for col: number in range(searchpos('(', 'nW')[1], getline(1)->strlen())
    matchaddpos('ErrorMsg', [[1, col]])
endfor

zeertzjq added a commit to zeertzjq/neovim that referenced this issue Nov 24, 2022
Problem:    Match highlighting continues over breakindent.
Solution:   Stop before the end column. (closes vim/vim#9242)

vim/vim@0c359af

Cherry-pick Test_matchdelete_redraw() from patch 8.2.1077.

Co-authored-by: Bram Moolenaar <Bram@vim.org>
Nero-F pushed a commit to Nero-F/neovim that referenced this issue Dec 16, 2022
Problem:    Match highlighting continues over breakindent.
Solution:   Stop before the end column. (closes vim/vim#9242)

vim/vim@0c359af

Cherry-pick Test_matchdelete_redraw() from patch 8.2.1077.

Co-authored-by: Bram Moolenaar <Bram@vim.org>
yesean pushed a commit to yesean/neovim that referenced this issue Mar 25, 2023
Problem:    Match highlighting continues over breakindent.
Solution:   Stop before the end column. (closes vim/vim#9242)

vim/vim@0c359af

Cherry-pick Test_matchdelete_redraw() from patch 8.2.1077.

Co-authored-by: Bram Moolenaar <Bram@vim.org>
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

No branches or pull requests

1 participant