[bug] Session file does not restore correct current window on each tab page
Jason Franklin
window for each tab page is not restored as expected. The new test
below currently fails but should pass:
diff --git a/src/testdir/test_mksession.vim b/src/testdir/test_mksession.vim
index bc41396..80bedfc 100644
--- a/src/testdir/test_mksession.vim
+++ b/src/testdir/test_mksession.vim
@@ -307,7 +307,6 @@ endfunc
endif
-
func Test_mksession_blank_windows()
split
split
@@ -323,6 +322,35 @@ func Test_mksession_blank_windows()
call delete('Xtest_mks.out')
endfunc
+func Test_mksession_tab_curwin()
+ only | tabonly
+
+ " tab with 2 windows
+ split
+ 2 wincmd w
+ tabnew
+
+ " tab with 3 windows
+ split | split
+ 3 wincmd w
+ tabnew
+
+ " tab with 4 windows
+ split | split | split
+ 3 wincmd w
+ tabnew
+
+ mksession! Xtest_mks.out
+ source Xtest_mks.out
+ call assert_equal(2, tabpagewinnr(1))
+ call assert_equal(3, tabpagewinnr(2))
+ call assert_equal(3, tabpagewinnr(3))
+ call assert_equal(1, tabpagewinnr(4))
+ call delete('Xtest_mks.out')
+
+ %bw!
+endfunc
+
if has('terminal')
func Test_mksession_terminal_shell()
The patch below fixes the problem and cleans up some of the code a bit.
diff --git a/src/ex_docmd.c b/src/ex_docmd.c
index 16f5059..3c41b3d 100644
--- a/src/ex_docmd.c
+++ b/src/ex_docmd.c
@@ -9727,6 +9727,7 @@ makeopens(
win_T *edited_win = NULL;
int tabnr;
int restore_stal = FALSE;
+ win_T *tab_curwin;
win_T *tab_firstwin;
frame_T *tab_topframe;
int cur_arg_idx = 0;
@@ -9842,7 +9843,8 @@ makeopens(
* Don't use goto_tabpage(), it may change directory and trigger
* autocommands.
*/
- tab_firstwin = firstwin; /* first window in tab page "tabnr" */
+ tab_curwin = curwin; // current window in tab page "tabnr"
+ tab_firstwin = firstwin; // first window in tab page "tabnr"
tab_topframe = topframe;
if ((ssop_flags & SSOP_TABPAGES))
{
@@ -9866,20 +9868,15 @@ makeopens(
{
tp = find_tabpage(tabnr);
+ // done all tab pages
if (tp == NULL)
- break; /* done all tab pages */
- if (tp == curtab)
- {
- tab_firstwin = firstwin;
- tab_topframe = topframe;
- }
- else
- {
- tab_firstwin = tp->tp_firstwin;
- tab_topframe = tp->tp_topframe;
- }
- if (tabnr > 1)
- need_tabnext = TRUE;
+ break;
+
+ tab_curwin = (tp == curtab) ? curwin : tp->tp_curwin;
+ tab_firstwin = (tp == curtab) ? firstwin : tp->tp_firstwin;
+ tab_topframe = (tp == curtab) ? topframe : tp->tp_topframe;
+
+ need_tabnext = (tabnr > 1);
}
/*
@@ -9938,7 +9935,7 @@ makeopens(
++nr;
else
restore_size = FALSE;
- if (curwin == wp)
+ if (tab_curwin == wp)
cnr = nr;
}
Thanks,
Jason Franklin
Christian Brabandt
On Mo, 20 Mai 2019, Jason Franklin wrote:
> When restoring a session that has multiple tab pages, the current
> window for each tab page is not restored as expected. The new test
> below currently fails but should pass:
Best,
Christian
--
Deine Seelenruhe läßt sich stören durch des Pöbels Stimme, die nie
recht urtreilt, nie die Dinge bei ihrem rechten Namen nennt?
-- Francesco Petrarca (Gespräche über die Weltverachtung)
Jason Franklin
No, it does not fix that issue.
However, I'm not exactly sure that the issue referenced is a valid
complaint. It was my understanding that buffer numbers are simply id
numbers and shouldn't be relied upon for anything other than referencing
a particular buffer. The fact that the order changed shouldn't matter.
I'm not opposed to fixing this... I just don't get why it would be
important since the point of a session is to restore the views on all
of the files you were editing when the session was saved.
Am I wrong about this?
Christian Brabandt
guarantees, that buffer numbers stay stable. Perhaps that should be
mentioned in the documentation if one can expect it or not.
I did have a look at fixing this, but my attempts could not make
guarantee exactly this. So I am actually sympathetic with closing this
issue, but wasn't sure.
Mit freundlichen Grüßen
Christian
--
Es gibt zwei Arten von Menschen: Solche die Glück haben und solche wie mich.
Tony Mechelynck
buffers including "hidden" and "unlisted" buffers — to all buffers
opened for editing or displaying, that is, but not to sourced scripts.
These numbers are attributed to the corresponding buffer when it is
first opened during a session; the full list can be seen with the
":ls!" command (without the exclamation mark the so-called unlisted
buffers would be omitted even though they still have an associated
buffer number). Numbers of wiped buffers are voided but not made
available for reuse during the same session; OTOH the :bdel command
clears the buffer text but retains the buffer metadata and makes the
buffer "unlisted", retaining its buffer number. So buffer numbers are
usually not kept from one session to the next. Whan can be relied upon
between saving a session and restarting it is the full-path-name of
the editfile and AFAIK that's what :mksession uses.
I don't know much about the workings of :mksession because the
generated session scripts usually do much more than I need; I start
vim as "gvim -S" but with a handwritten ~/Session.vim, so I have full
control over what it does (and doesn't do); the :scriptnames command
tells me that this script is sourced at the very end of an otherwise
completely normal startup, at the VimEnter event or extremely near it
AFAICT.
Best regards,
Tony.
Bram Moolenaar
Christian wrote:
> On Di, 21 Mai 2019, Jason Franklin wrote:
>
> > > Does this also fix https://github.com/vim/vim/issues/4352 I suppose not?
> >
> > No, it does not fix that issue.
> >
> > However, I'm not exactly sure that the issue referenced is a valid
> > complaint. It was my understanding that buffer numbers are simply id
> > numbers and shouldn't be relied upon for anything other than referencing
> > a particular buffer. The fact that the order changed shouldn't matter.
> > I'm not opposed to fixing this... I just don't get why it would be
> > important since the point of a session is to restore the views on all
> > of the files you were editing when the session was saved.
> >
> > Am I wrong about this?
>
> That's what I was thinking about. I am not sure that a session
> guarantees, that buffer numbers stay stable. Perhaps that should be
> mentioned in the documentation if one can expect it or not.
you edit a bunch of files in sequence you can do ":brewind" and then
":bnext" to go through the same files again.
> I did have a look at fixing this, but my attempts could not make
> guarantee exactly this. So I am actually sympathetic with closing this
> issue, but wasn't sure.
works to create the buffers without actually opening them or any other
side effects. Then later reset the argument list.
--
hundred-and-one symptoms of being an internet addict:
3. Your bookmark takes 15 minutes to scroll from top to bottom.
/// Bram Moolenaar -- Br...@Moolenaar.net -- http://www.Moolenaar.net \\\
/// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\ an exciting new programming language -- http://www.Zimbu.org ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///
Jason Franklin
> Perhaps using :argadd works to create the buffers without actually
> opening them or any other side effects. Then later reset the
> argument list.
I think it's even trickier than using :argadd. For example...
vim --clean
:argadd test.txt
:e test.txt
Notice that you're now editing buffer #2. This breaks the previous
patch I submitted to avoid extra empty buffers when loading a session.
Also, consider the case where several buffers are open and the user opens
a session with :so Session.vim. Then, cycling through the buffers won't
work anyway because there will be several buffers open from before the
session was loaded.
I'm unable to come up with perfect solutions to these issues at this time.
However, the patch under discussion here seems obviously correct to me. The
test here does a nice job of proving that the fix works, and should pass in
all future changes to the session code to fix any of the other issues named
here.
-- Jason
Bram Moolenaar
Jason Franklin wrote:
> > Perhaps using :argadd works to create the buffers without actually
> > opening them or any other side effects. Then later reset the
> > argument list.
>
> I think it's even trickier than using :argadd. For example...
>
> vim --clean
> :argadd test.txt
> :e test.txt
>
> Notice that you're now editing buffer #2. This breaks the previous
> patch I submitted to avoid extra empty buffers when loading a session.
file2". The first buffer is wiped out, we just don't have buffer number
one.
Not sure if there is any difference in using ":argadd" instead of
":badd". The loop over all buffers near the end of makeopens() could be
done much earlier. Or perhaps it needs to be done twice.
> Also, consider the case where several buffers are open and the user opens
> a session with :so Session.vim. Then, cycling through the buffers won't
> work anyway because there will be several buffers open from before the
> session was loaded.
to care about saving a session, exit and opening Vim with the same
session. It should be functionally in the same state then.
> I'm unable to come up with perfect solutions to these issues at this time.
>
> However, the patch under discussion here seems obviously correct to me. The
> test here does a nice job of proving that the fix works, and should pass in
> all future changes to the session code to fix any of the other issues named
> here.
--
The Feynman problem solving Algorithm:
1) Write down the problem
2) Think real hard
3) Write down the answer
Jason Franklin
> file2". The first buffer is wiped out, we just don't have buffer number
> to care about saving a session, exit and opening Vim with the same
> session. It should be functionally in the same state then.