fix(highlight): forward search / matching in long lines

[xiyaowong]

• fix: cursor position after scrolling with incsearch
• refactor: remove viewport hack when in cmdline mode

---

• Close fix(highlight): forward search / for long line #1975
• Fix forward search / incorrectly matches for long line #161
• Related:
  • fix(incsearch): viewport is not updating during searching #1575

Because it involves a few historical matters and dealing with hacks, I won't go into too much detail. Here are some debugging notes.

Cursor position for incsearch

Commit History	Nvim 0.9	Nvim 0.10
Add test	❌	✅
Remove hack	❌	✅
Update cursor position on window-scroll	❌	✅
Fire cursor change event on window-scroll	❌	✅
custom viewport changed event	❌	✅
after searching, perform a redraw and wait for a while	✅	✅

Forward search matching in long line

Only tested in Nvim 0.10.

Conclusion

Highly recommend to upgrade to Nvim 0.10.

[ollien]

Am I reading your table correctly to mean that this breaks functionality for 0.9 users?

I'm not sure how I feel about breaking support for 0.9 so quickly after 0.10's release. Even our automated test runners aren't updated to support it. That said, if we already have precedent for staying on the bleeding edge, I guess it's not so bad?

[xiyaowong]

The table corresponds to the commits sequence from top to bottom. I added the test case first, which failed in nvim 0.9.

This is actually a long-standing but not serious issue.

[xiyaowong]

I tested it out, and indeed, this PR completely breaks the 0.9 search highlight. This is unacceptable.

[xiyaowong]

@ollien Can you help test this pr in nvim0.9, it needs actual use for testing. And some problems are related to the platform. . .

[ollien]

@xiyaowong absolutely. I won't be in front of an IDE much for the next couple of days, so my testing may be a bit superficial, but I can definitely do some more soak testing otherwise. Are there any particular cases you'd like like to see tested?

Also, is this what you wanted to hold up the 1.11.5 release for? Maybe we can release that now and then push this as a minor version upgrade?

[xiyaowong]

Are there any particular cases you'd like like to see tested?

Just use it normally

Also, is this what you wanted to hold up the 1.11.5 release for?

Yeah.

Maybe we can release that now and then push this as a minor version upgrade?

Agreed, this PR will still take some time to confirm.

[ollien]

Just for my education, what's the difference between this and the WinScrolled autocmd? Is it just that on_win happens before any redraw, while WinScrolled is literally just that, scrolling?

[xiyaowong]

WinScrolled won't be triggered under cmdline. We also need timely and accurate viewport information neovim/neovim#28800 (comment).

[ollien]

Huh, ok.

This comment befuddles me a little bit (how can the cursor position be different in the window than the grid?), but the reason for your change makes sense :)

Well yes the cursor position in the current window changes for incsearch but the cursor position on the grid does not and should not(implicitly). What problem does this cause?

[xiyaowong]

• "grid cursor" refers to the visible cursor position.
• One grid includes not only the window viewport but also components like cmdline, statuscolumn, winbar, and so on. However, vscode-neovim doesn't (can't) need these components, we only dealing with the window viewport.

We should use grid_scroll to adjust the editor, but this is not possible in vscode.

---

• You may notice that in the cursor manager, we only use grid_cursor_goto to detect changes in cursor position, but we actually use data from the viewport.
• With ext_cmdline enabled, when entering cmdline, the visual cursor position is managed by the UI, so there is no grid_cursor_goto event (In Nvim0.10).

[xiyaowong]

I spent almost 5 hours, and couldn't find out why the failure of the virtual text highlighting test😢.
Only know it passes when run alone (when it's the first test being run). There seems to be an issue with the method used for testing the highlight.

[xiyaowong]

😩😩😩

[xiyaowong]

Ready for review&test, but not ready for merge

[theol0403]

So far so good.

[ollien]

Things have mostly been fine here. There is one bug I'm seeing occasionally, which is that a highlighted keyword or curly brace will float in the editor (often very far from where I would expect it) when the window loads. I've reproduced it a couple times, but I've not managed to get a screenshot or anything of it yet

[xiyaowong]

It may be normal to have a little flaw. Many of them are caused by viewport. Is this problem brought by this pr?

[ollien]

I'd never seen it before, but given I can't easily repro it, I wouldn't sweat it

[xiyaowong]

Weired. This PR is just a more comprehensive ViewPort information, how can it cause new problems

[ollien]

Weired. This PR is just a more comprehensive ViewPort information, how can it cause new problems

It might not be. It could be another PR from master :)

Like I said, don't sweat it