geany / geany-plugins

The combined Geany Plugins collection
http://plugins.geany.org/
581 stars 262 forks source link

Vimode: Fix cursor hang with folded lines #1326

Closed scresto09 closed 1 month ago

scresto09 commented 2 months ago

Fix cursor hang when we want to move cursor on top line and this line is folded

To reproduce the problem, when vimode plugin is enabled: Fold a few lines by clicking the "minus" icon. Move the cursor to the bottom of these lines and try to move back to the top. The cursor doesn't go back and seem to hang.

techee commented 2 months ago

I'm not sure this is the correct way to address the issue. I think we should take the folds into account and perform the motion commands on top of the visible lines instead of the document lines. I tried to replace this PR with

https://github.com/geany/geany-plugins/pull/1338

Please let me know what you think about the changes.

scresto09 commented 1 month ago

In response to pull request #1338

After read the https://sourceforge.net/p/scintilla/bugs/2438/ report, I think the best way to automatically set the cursor position on the visible line is surely to track the SCN_MARGINCLICK event. I did some modification to handle this with the commit 5608b74 .

techee commented 1 month ago

After read the https://sourceforge.net/p/scintilla/bugs/2438/ report, I think the best way to automatically set the cursor position on the visible line is surely to track the SCN_MARGINCLICK event. I did some modification to handle this with the commit 5608b74 .

Thanks for noticing the problem with line wrapping. I think your general approach is right - I've just slightly rewritten the doc_line_from_visible_delta() function to be easier to grasp by my poor brain, please check https://github.com/geany/geany-plugins/pull/1338/commits/fa7025ba9d58fb4680fb47e13bd5c05c6f1f1059 if it looks good to you.

There are a few things in your patch which I don't think are completely correct though - I'll add some inline comments to your code.

techee commented 1 month ago

Closing this PR as #1338 was merged. Having SCN_MARGINCLICK handled would however be worth adding as a separate PR.

scresto09 commented 1 month ago

OK, I just create #1349 for SCN_MARGINCLICK part. Thanks