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

Preserve buffer order when loading session #9520

Closed
wants to merge 1 commit into from

Conversation

echasnovski
Copy link
Contributor

This is a PR to fix #4352. It basically reverts 8.1.0829 and fixes its problem differently.

Current behavior is that all visible buffers along with alternative file are loaded first and then the rest ones. This happens because badd commands in session file are sourced near the end, after edit and balt commands. Placing badd commands near the end was a fix for extra empty buffers created when hidden was on and there were several tab pages to be restored.

After some investigation, the main reason for empty buffers seems to be plain tabnew calls in the beginning of session file to restore tabs (which in itself is a fix introduced in #3152, patch 8.1.0149). As a result of tabnew, each tab is created with new empty buffer. After later calls of tabnext and edit <file> these buffers disappear unless <file> is already loaded in another buffer.

My proposed fix is to open placeholder tab with empty buffer intentionally set to be wiped out after it is no longer needed. This change doesn't break any existing tests. The only situation I can think of where this local setting might matter is when user intentionally created session for tab with empty buffer. In that case its local bufhidden will be restored in later options restoration (if "options" is present in sessionoptions, meaning user cares about those options at all).

What I also tried

Additional justification

In original issue it is stated that there is no clearly documented guarantee of preserving buffer order, which is true. Also it might seem like a minor thing and cosmetic change. However, besides use case from this comment (relying on manual buffer order in workflow), it becomes more visible when using tabline to show buffers instead of tabs, which seems to be quite common practise. In that case ever changing order is pretty inconvenient.

@codecov
Copy link

codecov bot commented Jan 13, 2022

Codecov Report

Merging #9520 (aadf55e) into master (4050305) will increase coverage by 0.53%.
The diff coverage is 63.63%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9520      +/-   ##
==========================================
+ Coverage   82.62%   83.15%   +0.53%     
==========================================
  Files         154      154              
  Lines      171015   174164    +3149     
  Branches    39178    39181       +3     
==========================================
+ Hits       141294   144830    +3536     
- Misses      16669    17216     +547     
+ Partials    13052    12118     -934     
Flag Coverage Δ
huge-clang-none 81.86% <63.63%> (?)
huge-gcc-none 82.21% <70.00%> (-0.01%) ⬇️
huge-gcc-testgui ?
huge-gcc-unittests 2.03% <0.00%> (ø)

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

Impacted Files Coverage Δ
src/session.c 64.26% <63.63%> (-1.09%) ⬇️
src/gui_gtk_x11.c 51.59% <0.00%> (-4.55%) ⬇️
src/gui_gtk.c 24.23% <0.00%> (-3.47%) ⬇️
src/highlight.c 78.66% <0.00%> (-2.21%) ⬇️
src/gui_xim.c 21.35% <0.00%> (-1.44%) ⬇️
src/libvterm/src/vterm.c 66.37% <0.00%> (-1.32%) ⬇️
src/buffer.c 83.56% <0.00%> (-1.21%) ⬇️
src/misc2.c 86.86% <0.00%> (-1.14%) ⬇️
src/filepath.c 81.08% <0.00%> (-0.91%) ⬇️
... and 127 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4050305...aadf55e. Read the comment docs.

@brammool brammool closed this in 26ebf1f Jan 14, 2022
echasnovski added a commit to echasnovski/neovim that referenced this pull request Jan 16, 2022
…te different

Problem:    After restoring a session buffer order can be quite different.
Solution:   Create buffers first. (Evgeni Chasnovski, closes vim/vim#9520)
vim/vim@26ebf1f

---------------
Also update functional test about restoring same terminals. It was a bit
too restrictive with using actual buffer ids, which changed with this
patch (now they should be in the same order as at `mksession` call).
echasnovski added a commit to echasnovski/neovim that referenced this pull request Jan 16, 2022
…te different

Problem:    After restoring a session buffer order can be quite different.
Solution:   Create buffers first. (Evgeni Chasnovski, closes vim/vim#9520)
vim/vim@26ebf1f

---------------
As in Vim, this basically reverts 8.1.0829 providing different solution
(see vim/vim#9520).

Regarding Neovim, this basically reverts changes from neovim#15062. Test about
restoring same terminals was a bit too restrictive with using actual
buffer ids, which changed with this patch (now they should be in the
same order as at `mksession` call), so I tweaked it.
@echasnovski echasnovski deleted the session-buffer-order branch January 16, 2022 11:43
seandewar pushed a commit to neovim/neovim that referenced this pull request Jan 29, 2022
…te different (#17112)

Problem:    After restoring a session buffer order can be quite different.
Solution:   Create buffers first. (Evgeni Chasnovski, closes vim/vim#9520)
vim/vim@26ebf1f

---------------
As in Vim, this basically reverts 8.1.0829 providing different solution
(see vim/vim#9520).

Regarding Neovim, this basically reverts changes from #15062. Test about
restoring same terminals was a bit too restrictive with using actual
buffer ids, which changed with this patch (now they should be in the
same order as at `mksession` call), so I tweaked it.
dmitmel pushed a commit to dmitmel/neovim that referenced this pull request Apr 16, 2022
…te different (neovim#17112)

Problem:    After restoring a session buffer order can be quite different.
Solution:   Create buffers first. (Evgeni Chasnovski, closes vim/vim#9520)
vim/vim@26ebf1f

---------------
As in Vim, this basically reverts 8.1.0829 providing different solution
(see vim/vim#9520).

Regarding Neovim, this basically reverts changes from neovim#15062. Test about
restoring same terminals was a bit too restrictive with using actual
buffer ids, which changed with this patch (now they should be in the
same order as at `mksession` call), so I tweaked it.
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.

badd at the end of the session file changes the order of the buffers on load
1 participant