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

More 'nosplitscroll' problems #11166

Closed
wants to merge 5 commits into from
Closed

Conversation

luukvbaal
Copy link
Contributor

@luukvbaal luukvbaal commented Sep 19, 2022

  " No scroll when splitting window in terminal callback
  norm ggL
  let top = line('w0')
  func s:ExitCb(a, b)
    close | help
  endfunc
  let buf = term_start(&shell, { 'hidden': 1, 'exit_cb': 's:ExitCb' })
  call popup_create(buf, {})
  call term_sendkeys(buf, "exit\<CR>")
  sleep 200m
  redraw!
  wincmd k
  call assert_equal(top, line('w0'))

but this only passes with the redraw, at which point it also passes before the fix.

Moreover, if we move the wincmd k to the callback function, the current fix is not enough to catch that and scrolling will still happen. Trying to fix it with a watchpoint on firstwin->w_topline and backtracing led me to trying to add workarounds in redraw_after_callback() or ex_docmd.c but it got quite complicated. Not sure if it's worth to catch this issue.

Problem:	Text flickers if we always skip update_topline() in
curs_columns() for 'nosplitscroll'.
Solution:	Only skip update_topline() while executing
win_fix_scroll()(Fix vim#11158).
Problem:	Text is scrolled for 'nosplitscroll' when changing
tab page.
Solution:	Skip win_fix_scroll() call when changing tab page
(Fix vim#11159).
Problem:	Text scrolls for 'nosplitscroll' in terminal callback
because State is not updated yet.
Solution:	Treat terminal mode as normal mode in win_fix_cursor()
(Fix vim#11160).
Problem:	'nosplitscroll' tests are outdated.
Solution:	Add tests for various fixes, fix 'scrolloff' not being
tested.
Problem:	Cursor position changes in insert mode with
'nosplitscroll' and 'scrolloff' in a small window.
Solution	Replace previously erroneous workaround by
validate_botline win() call in win_fix_cursor().
@luukvbaal
Copy link
Contributor Author

See commit messages for some justifications.

@codecov
Copy link

codecov bot commented Sep 19, 2022

Codecov Report

Merging #11166 (319a9ae) into master (b2f0ca8) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master   #11166   +/-   ##
=======================================
  Coverage   81.77%   81.78%           
=======================================
  Files         162      162           
  Lines      188611   188613    +2     
  Branches    42887    42886    -1     
=======================================
+ Hits       154246   154257   +11     
+ Misses      21795    21788    -7     
+ Partials    12570    12568    -2     
Flag Coverage Δ
huge-clang-none 82.67% <100.00%> (-0.01%) ⬇️
huge-gcc-none 54.00% <100.00%> (+<0.01%) ⬆️
huge-gcc-testgui 52.43% <87.50%> (+<0.01%) ⬆️
huge-gcc-unittests 0.29% <0.00%> (ø)
linux 82.44% <100.00%> (+<0.01%) ⬆️
mingw-x64-HUGE 76.47% <100.00%> (+0.01%) ⬆️
mingw-x86-HUGE 77.31% <87.50%> (-0.02%) ⬇️
windows 78.11% <100.00%> (+<0.01%) ⬆️

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

Impacted Files Coverage Δ
src/move.c 90.30% <100.00%> (ø)
src/window.c 88.81% <100.00%> (+0.16%) ⬆️
src/beval.c 64.02% <0.00%> (-2.16%) ⬇️
src/gui_w32.c 37.05% <0.00%> (-0.18%) ⬇️
src/regexp_bt.c 85.95% <0.00%> (-0.13%) ⬇️
src/terminal.c 77.61% <0.00%> (-0.06%) ⬇️
src/normal.c 90.96% <0.00%> (-0.04%) ⬇️
src/screen.c 79.72% <0.00%> (ø)
src/version.c 85.18% <0.00%> (ø)
src/gui.c 72.88% <0.00%> (+0.04%) ⬆️
... and 5 more

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

@brammool brammool closed this in faf1d41 Sep 19, 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