Skip to content

Conversation

luukvbaal
Copy link
Contributor

@luukvbaal luukvbaal commented Sep 14, 2022

Problem: Cursor moves when cmdwin is closed when 'splitscroll' is off.
Solution: Skip win_fix_cursor if called when cmdwin is open or closing.

Reverts change to ex_getln.c from e697d48, keep the test. Can you confirm this works @mityu.

@codecov
Copy link

codecov bot commented Sep 14, 2022

Codecov Report

Merging #11134 (cf52c37) into master (e697d48) will increase coverage by 0.17%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master   #11134      +/-   ##
==========================================
+ Coverage   81.61%   81.78%   +0.17%     
==========================================
  Files         162      162              
  Lines      188258   188331      +73     
  Branches    42786    42833      +47     
==========================================
+ Hits       153645   154031     +386     
+ Misses      22136    21750     -386     
- Partials    12477    12550      +73     
Flag Coverage Δ
huge-clang-none 82.68% <100.00%> (-0.01%) ⬇️
huge-gcc-none 54.56% <100.00%> (+<0.01%) ⬆️
huge-gcc-testgui 53.00% <100.00%> (-0.01%) ⬇️
huge-gcc-unittests 0.29% <0.00%> (-0.01%) ⬇️
linux 82.46% <100.00%> (-0.02%) ⬇️
mingw-x64-HUGE 76.46% <100.00%> (?)
mingw-x86-HUGE 77.33% <100.00%> (+<0.01%) ⬆️
windows 78.11% <100.00%> (+0.79%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/ex_getln.c 85.80% <100.00%> (-0.01%) ⬇️
src/window.c 88.84% <100.00%> (+<0.01%) ⬆️
src/sound.c 64.00% <0.00%> (-1.72%) ⬇️
src/gui_dwrite.cpp 45.93% <0.00%> (-0.56%) ⬇️
src/regexp_nfa.c 89.44% <0.00%> (-0.27%) ⬇️
src/misc2.c 89.26% <0.00%> (-0.10%) ⬇️
src/textprop.c 89.55% <0.00%> (-0.07%) ⬇️
src/gui_w32.c 37.23% <0.00%> (-0.06%) ⬇️
src/ex_cmds.c 85.62% <0.00%> (-0.03%) ⬇️
src/filepath.c 82.06% <0.00%> (+<0.01%) ⬆️
... and 29 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@mityu
Copy link
Contributor

mityu commented Sep 14, 2022

Thank you for suggesting a new solution for cursor moves with cmdwin.
I found one problem with this patch at least. With win_execute() function, you can change cursor positions on other windows while in cmdwin. However, with this patch, cursor position changes for the current window by user-side scripts will be reverted when leaving cmdwin.
Example:

  • `vim -u NONE --cmd "set nosplitscroll | call setline(1, range(&lines))"
  • Type Gq::call win_execute(winnr('#')->win_getid(), 'call cursor(1, 1)')<CR><C-w>q

Without this patch, the cursor position will be (1, 1), but with this patch, it's not.

  • With this patch
    with-11134

  • Without this patch
    without-11134

@luukvbaal
Copy link
Contributor Author

Right. What we actually want to do is delay the win_fix_cursor until after the win_restore_size in open_cmdwin, or skip it altogether. Not sure how to achieve this. Would have to pass along args to a a lot of functions I think.

@luukvbaal
Copy link
Contributor Author

luukvbaal commented Sep 14, 2022

Or set a global of course. I think the latest commit should fix the cmdwin cursor issue without side-effects.

Problem:    Cursor moves when cmdwin is closed when 'splitscroll' is off.
Solution:   Skip win_fix_cursor if called when cmdwin is closed.
@mityu
Copy link
Contributor

mityu commented Sep 15, 2022

It seems that this patch works well. LGTM.

@luukvbaal luukvbaal changed the title cursor moves when cmdwin is closed when 'splitscroll' cursor moves when cmdwin is closed when 'splitscroll' is off Sep 15, 2022
@brammool brammool closed this in 3735f11 Sep 15, 2022
zeertzjq added a commit to luukvbaal/neovim that referenced this pull request Oct 6, 2022
vim-patch:9.0.0445: when opening/closing window text moves up/down

Problem:    When opening/closing window text moves up/down.
Solution:   Add the 'splitscroll' option.  When off text will keep its
            position as much as possible.
vim/vim@29ab524

vim-patch:9.0.0455: a few problems with 'splitscroll'

Problem:    A few problems with 'splitscroll'.
Solution:   Fix 'splitscroll' problems. (Luuk van Baal, closes vim/vim#11117)
vim/vim@5ed3917

vim-patch:9.0.0461: 'scroll' is not always updated

Problem:    'scroll' is not always updated.
Solution:   Call win_init_size() at the right place.
vim/vim@470a141

vim-patch:9.0.0465: cursor moves when cmdwin is closed when 'splitscroll' is off

Problem:    Cursor moves when cmdwin is closed when 'splitscroll' is off.
Solution:   Temporarily set 'splitscroll' when jumping back to the original
            window. (closes vim/vim#11128)
vim/vim@e697d48

vim-patch:9.0.0469: cursor moves if cmdwin is closed when 'splitscroll' is off

Problem:    Cursor moves if cmdwin is closed when 'splitscroll' is off.
Solution:   Skip win_fix_cursor if called when cmdwin is open or closing.
            (Luuk van Baal, closes vim/vim#11134)
vim/vim@3735f11

vim-patch:9.0.0478: test for 'splitscroll' takes too much time

Problem:    Test for 'splitscroll' takes too much time.
Solution:   Only test some of the combinations. (Luuk van Baal, closes vim/vim#11139)
vim/vim@594f9e0

vim-patch:9.0.0486: text scrolled with 'nosplitscroll', autocmd win and help

Problem:    Text scrolled with 'nosplitscroll', autocmd win opened and help
            window closed.
Solution:   Skip win_fix_scroll() in more situations. (Luuk van Baal,
            closes vim/vim#11150)
vim/vim@d5bc762

vim-patch:9.0.0505: various problems with 'nosplitscroll'

Problem:    Various problems with 'nosplitscroll'.
Solution:   Fix 'nosplitscroll' problems. (Luuk van Baal, closes vim/vim#11166)
vim/vim@faf1d41

vim-patch:9.0.0555: scrolling with 'nosplitscroll' in callback changing curwin

Problem:    Scrolling with 'nosplitscroll' in callback changing curwin.
Solution:   Invalidate w_cline_row in the right place. (Luuk van Baal,
            closes vim/vim#11185)
vim/vim@20e5856

vim-patch:9.0.0603: with 'nosplitscroll' folds are not handled correctly

Problem:    With 'nosplitscroll' folds are not handled correctly.
Solution:   Take care of closed folds when moving the cursor. (Luuk van Baal,
            closes vim/vim#11234)
vim/vim@7c1cbb6

vim-patch:9.0.0605: dump file missing

Problem:    Dump file missing.
Solution:   Add the missing dump file. (issue vim/vim#11234)
vim/vim@439a2ba

vim-patch:9.0.0647: the 'splitscroll' option is not a good name

Problem:    The 'splitscroll' option is not a good name.
Solution:   Rename 'splitscroll' to 'splitkeep' and make it a string option,
            also supporting "topline". (Luuk van Baal, closes vim/vim#11258)
vim/vim@13ece2a

vim-patch:9.0.0667: ml_get error when 'splitkeep' is "screen"

Problem:    ml_get error when 'splitkeep' is "screen". (Marius Gedminas)
Solution:   Check the botline is not too large. (Luuk van Baal,
            closes vim/vim#11293, closes vim/vim#11292)
vim/vim@346823d
zeertzjq pushed a commit to neovim/neovim that referenced this pull request Oct 6, 2022
vim-patch:9.0.0445: when opening/closing window text moves up/down

Problem:    When opening/closing window text moves up/down.
Solution:   Add the 'splitscroll' option.  When off text will keep its
            position as much as possible.
vim/vim@29ab524

vim-patch:9.0.0455: a few problems with 'splitscroll'

Problem:    A few problems with 'splitscroll'.
Solution:   Fix 'splitscroll' problems. (Luuk van Baal, closes vim/vim#11117)
vim/vim@5ed3917

vim-patch:9.0.0461: 'scroll' is not always updated

Problem:    'scroll' is not always updated.
Solution:   Call win_init_size() at the right place.
vim/vim@470a141

vim-patch:9.0.0465: cursor moves when cmdwin is closed when 'splitscroll' is off

Problem:    Cursor moves when cmdwin is closed when 'splitscroll' is off.
Solution:   Temporarily set 'splitscroll' when jumping back to the original
            window. (closes vim/vim#11128)
vim/vim@e697d48

vim-patch:9.0.0469: cursor moves if cmdwin is closed when 'splitscroll' is off

Problem:    Cursor moves if cmdwin is closed when 'splitscroll' is off.
Solution:   Skip win_fix_cursor if called when cmdwin is open or closing.
            (Luuk van Baal, closes vim/vim#11134)
vim/vim@3735f11

vim-patch:9.0.0478: test for 'splitscroll' takes too much time

Problem:    Test for 'splitscroll' takes too much time.
Solution:   Only test some of the combinations. (Luuk van Baal, closes vim/vim#11139)
vim/vim@594f9e0

vim-patch:9.0.0486: text scrolled with 'nosplitscroll', autocmd win and help

Problem:    Text scrolled with 'nosplitscroll', autocmd win opened and help
            window closed.
Solution:   Skip win_fix_scroll() in more situations. (Luuk van Baal,
            closes vim/vim#11150)
vim/vim@d5bc762

vim-patch:9.0.0505: various problems with 'nosplitscroll'

Problem:    Various problems with 'nosplitscroll'.
Solution:   Fix 'nosplitscroll' problems. (Luuk van Baal, closes vim/vim#11166)
vim/vim@faf1d41

vim-patch:9.0.0555: scrolling with 'nosplitscroll' in callback changing curwin

Problem:    Scrolling with 'nosplitscroll' in callback changing curwin.
Solution:   Invalidate w_cline_row in the right place. (Luuk van Baal,
            closes vim/vim#11185)
vim/vim@20e5856

vim-patch:9.0.0603: with 'nosplitscroll' folds are not handled correctly

Problem:    With 'nosplitscroll' folds are not handled correctly.
Solution:   Take care of closed folds when moving the cursor. (Luuk van Baal,
            closes vim/vim#11234)
vim/vim@7c1cbb6

vim-patch:9.0.0605: dump file missing

Problem:    Dump file missing.
Solution:   Add the missing dump file. (issue vim/vim#11234)
vim/vim@439a2ba

vim-patch:9.0.0647: the 'splitscroll' option is not a good name

Problem:    The 'splitscroll' option is not a good name.
Solution:   Rename 'splitscroll' to 'splitkeep' and make it a string option,
            also supporting "topline". (Luuk van Baal, closes vim/vim#11258)
vim/vim@13ece2a

vim-patch:9.0.0667: ml_get error when 'splitkeep' is "screen"

Problem:    ml_get error when 'splitkeep' is "screen". (Marius Gedminas)
Solution:   Check the botline is not too large. (Luuk van Baal,
            closes vim/vim#11293, closes vim/vim#11292)
vim/vim@346823d
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

Successfully merging this pull request may close these issues.

2 participants