patch 8.2.1662: :mksession does not restore shared terminal buffer properly

Problem:    :mksession does not restore shared terminal buffer properly.
Solution:   Keep a hashtab with terminal buffers. (Rob Pilling, closes #6930)

diff --git a/src/hashtab.c b/src/hashtab.c
index f114b288085e37b6b46a021825b6732e4fbaffca..dc0cbb6b86962da4cc66817e6bbf9c55072fea88 100644 (file)
--- a/src/hashtab.c
+++ b/src/hashtab.c
@@ -81,7 +81,7 @@ hash_clear(hashtab_T *ht)
        vim_free(ht->ht_array);
 }
 
-#if defined(FEAT_SPELL) || defined(PROTO)
+#if defined(FEAT_SPELL) || defined(FEAT_TERMINAL) || defined(PROTO)
 /*
  * Free the array of a hash table and all the keys it contains.  The keys must
  * have been allocated.  "off" is the offset from the start of the allocate

diff --git a/src/proto/terminal.pro b/src/proto/terminal.pro
index e8760d14a73691e01d8dbb88bf3e5738bfba90ab..f3bd5a31c7b237b032aec9de90481c9a3658255d 100644 (file)
--- a/src/proto/terminal.pro
+++ b/src/proto/terminal.pro
@@ -2,7 +2,7 @@
 void init_job_options(jobopt_T *opt);
 buf_T *term_start(typval_T *argvar, char **argv, jobopt_T *opt, int flags);
 void ex_terminal(exarg_T *eap);
-int term_write_session(FILE *fd, win_T *wp);
+int term_write_session(FILE *fd, win_T *wp, hashtab_T *terminal_bufs);
 int term_should_restore(buf_T *buf);
 void free_terminal(buf_T *buf);
 void free_unused_terminals(void);

diff --git a/src/session.c b/src/session.c
index 3f5d4c611fe6e82fb17160d7a643b716d974f026..5e6f0b53e87205e0e0554f8a28d62bb0a2701af2 100644 (file)
--- a/src/session.c
+++ b/src/session.c
@@ -305,8 +305,12 @@ put_view(
     win_T      *wp,
     int                add_edit,       // add ":edit" command to view
     unsigned   *flagp,         // vop_flags or ssop_flags
-    int                current_arg_idx) // current argument index of the window, use
-                                // -1 if unknown
+    int                current_arg_idx // current argument index of the window, use
+                               // -1 if unknown
+#ifdef FEAT_TERMINAL
+    , hashtab_T *terminal_bufs
+#endif
+    )
 {
     win_T      *save_curwin;
     int                f;
@@ -349,7 +353,7 @@ put_view(
 # ifdef FEAT_TERMINAL
        if (bt_terminal(wp->w_buffer))
        {
-           if (term_write_session(fd, wp) == FAIL)
+           if (term_write_session(fd, wp, terminal_bufs) == FAIL)
                return FAIL;
        }
        else
@@ -588,6 +592,12 @@ makeopens(
     frame_T    *tab_topframe;
     int                cur_arg_idx = 0;
     int                next_arg_idx = 0;
+    int                ret = FAIL;
+#ifdef FEAT_TERMINAL
+    hashtab_T  terminal_bufs;
+
+    hash_init(&terminal_bufs);
+#endif
 
     if (ssop_flags & SSOP_BUFFERS)
        only_save_windows = FALSE;              // Save ALL buffers
@@ -596,25 +606,25 @@ makeopens(
     // sessionable variables.
 #ifdef FEAT_EVAL
     if (put_line(fd, "let v:this_session=expand(\"<sfile>:p\")") == FAIL)
-       return FAIL;
+       goto fail;
     if (ssop_flags & SSOP_GLOBALS)
        if (store_session_globals(fd) == FAIL)
-           return FAIL;
+           goto fail;
 #endif
 
     // Close all windows and tabs but one.
     if (put_line(fd, "silent only") == FAIL)
-       return FAIL;
+       goto fail;
     if ((ssop_flags & SSOP_TABPAGES)
            && put_line(fd, "silent tabonly") == FAIL)
-       return FAIL;
+       goto fail;
 
     // Now a :cd command to the session directory or the current directory
     if (ssop_flags & SSOP_SESDIR)
     {
        if (put_line(fd, "exe \"cd \" . escape(expand(\"<sfile>:p:h\"), ' ')")
                                                                      == FAIL)
-           return FAIL;
+           goto fail;
     }
     else if (ssop_flags & SSOP_CURDIR)
     {
@@ -625,7 +635,7 @@ makeopens(
                || put_eol(fd) == FAIL)
        {
            vim_free(sname);
-           return FAIL;
+           goto fail;
        }
        vim_free(sname);
     }
@@ -633,27 +643,27 @@ makeopens(
     // If there is an empty, unnamed buffer we will wipe it out later.
     // Remember the buffer number.
     if (put_line(fd, "if expand('%') == '' && !&modified && line('$') <= 1 && getline(1) == ''") == FAIL)
-       return FAIL;
+       goto fail;
     if (put_line(fd, "  let s:wipebuf = bufnr('%')") == FAIL)
-       return FAIL;
+       goto fail;
     if (put_line(fd, "endif") == FAIL)
-       return FAIL;
+       goto fail;
 
     // Now save the current files, current buffer first.
     if (put_line(fd, "set shortmess=aoO") == FAIL)
-       return FAIL;
+       goto fail;
 
     // the global argument list
     if (ses_arglist(fd, "argglobal", &global_alist.al_ga,
                            !(ssop_flags & SSOP_CURDIR), &ssop_flags) == FAIL)
-       return FAIL;
+       goto fail;
 
     if (ssop_flags & SSOP_RESIZE)
     {
        // Note: after the restore we still check it worked!
        if (fprintf(fd, "set lines=%ld columns=%ld" , Rows, Columns) < 0
                || put_eol(fd) == FAIL)
-           return FAIL;
+           goto fail;
     }
 
 #ifdef FEAT_GUI
@@ -665,7 +675,7 @@ makeopens(
        {
            // Note: after the restore we still check it worked!
            if (fprintf(fd, "winpos %d %d", x, y) < 0 || put_eol(fd) == FAIL)
-               return FAIL;
+               goto fail;
        }
     }
 #endif
@@ -677,7 +687,7 @@ makeopens(
     if (p_stal == 1 && first_tabpage->tp_next != NULL)
     {
        if (put_line(fd, "set stal=2") == FAIL)
-           return FAIL;
+           goto fail;
        restore_stal = TRUE;
     }
 
@@ -695,9 +705,9 @@ makeopens(
        // later local options won't be copied to the new tabs.
        FOR_ALL_TABPAGES(tp)
            if (tp->tp_next != NULL && put_line(fd, "tabnew") == FAIL)
-               return FAIL;
+               goto fail;
        if (first_tabpage->tp_next != NULL && put_line(fd, "tabrewind") == FAIL)
-           return FAIL;
+           goto fail;
     }
     for (tabnr = 1; ; ++tabnr)
     {
@@ -739,13 +749,13 @@ makeopens(
                    )
            {
                if (need_tabnext && put_line(fd, "tabnext") == FAIL)
-                   return FAIL;
+                   goto fail;
                need_tabnext = FALSE;
 
                if (fputs("edit ", fd) < 0
                              || ses_fname(fd, wp->w_buffer, &ssop_flags, TRUE)
                                                                       == FAIL)
-                   return FAIL;
+                   goto fail;
                if (!wp->w_arg_idx_invalid)
                    edited_win = wp;
                break;
@@ -754,17 +764,17 @@ makeopens(
 
        // If no file got edited create an empty tab page.
        if (need_tabnext && put_line(fd, "tabnext") == FAIL)
-           return FAIL;
+           goto fail;
 
        // Save current window layout.
        if (put_line(fd, "set splitbelow splitright") == FAIL)
-           return FAIL;
+           goto fail;
        if (ses_win_rec(fd, tab_topframe) == FAIL)
-           return FAIL;
+           goto fail;
        if (!p_sb && put_line(fd, "set nosplitbelow") == FAIL)
-           return FAIL;
+           goto fail;
        if (!p_spr && put_line(fd, "set nosplitright") == FAIL)
-           return FAIL;
+           goto fail;
 
        // Check if window sizes can be restored (no windows omitted).
        // Remember the window number of the current window after restoring.
@@ -781,7 +791,7 @@ makeopens(
 
        // Go to the first window.
        if (put_line(fd, "wincmd t") == FAIL)
-           return FAIL;
+           goto fail;
 
        // If more than one window, see if sizes can be restored.
        // First set 'winheight' and 'winwidth' to 1 to avoid the windows being
@@ -794,9 +804,9 @@ makeopens(
                || put_line(fd, "set winheight=1") == FAIL
                || put_line(fd, "set winminwidth=0") == FAIL
                || put_line(fd, "set winwidth=1") == FAIL)
-           return FAIL;
+           goto fail;
        if (nr > 1 && ses_winsizes(fd, restore_size, tab_firstwin) == FAIL)
-           return FAIL;
+           goto fail;
 
        // Restore the tab-local working directory if specified
        // Do this before the windows, so that the window-local directory can
@@ -806,7 +816,7 @@ makeopens(
            if (fputs("tcd ", fd) < 0
                    || ses_put_fname(fd, tp->tp_localdir, &ssop_flags) == FAIL
                    || put_eol(fd) == FAIL)
-               return FAIL;
+               goto fail;
            did_lcd = TRUE;
        }
 
@@ -815,11 +825,14 @@ makeopens(
        {
            if (!ses_do_win(wp))
                continue;
-           if (put_view(fd, wp, wp != edited_win, &ssop_flags,
-                                                        cur_arg_idx) == FAIL)
-               return FAIL;
+           if (put_view(fd, wp, wp != edited_win, &ssop_flags, cur_arg_idx
+#ifdef FEAT_TERMINAL
+                                                        , &terminal_bufs
+#endif
+                ) == FAIL)
+               goto fail;
            if (nr > 1 && put_line(fd, "wincmd w") == FAIL)
-               return FAIL;
+               goto fail;
            next_arg_idx = wp->w_arg_idx;
        }
 
@@ -831,12 +844,12 @@ makeopens(
        // Restore cursor to the current window if it's not the first one.
        if (cnr > 1 && (fprintf(fd, "%dwincmd w", cnr) < 0
                                                      || put_eol(fd) == FAIL))
-           return FAIL;
+           goto fail;
 
        // Restore window sizes again after jumping around in windows, because
        // the current window has a minimum size while others may not.
        if (nr > 1 && ses_winsizes(fd, restore_size, tab_firstwin) == FAIL)
-           return FAIL;
+           goto fail;
 
        // Don't continue in another tab page when doing only the current one
        // or when at the last tab page.
@@ -848,10 +861,10 @@ makeopens(
     {
        if (fprintf(fd, "tabnext %d", tabpage_index(curtab)) < 0
                || put_eol(fd) == FAIL)
-           return FAIL;
+           goto fail;
     }
     if (restore_stal && put_line(fd, "set stal=1") == FAIL)
-       return FAIL;
+       goto fail;
 
     // Now put the remaining buffers into the buffer list.
     // This is near the end, so that when 'hidden' is set we don't create extra
@@ -872,38 +885,43 @@ makeopens(
            if (fprintf(fd, "badd +%ld ", buf->b_wininfo == NULL ? 1L
                                           : buf->b_wininfo->wi_fpos.lnum) < 0
                    || ses_fname(fd, buf, &ssop_flags, TRUE) == FAIL)
-               return FAIL;
+               goto fail;
        }
     }
 
     // Wipe out an empty unnamed buffer we started in.
     if (put_line(fd, "if exists('s:wipebuf') && len(win_findbuf(s:wipebuf)) == 0")
                                                                       == FAIL)
-       return FAIL;
+       goto fail;
     if (put_line(fd, "  silent exe 'bwipe ' . s:wipebuf") == FAIL)
-       return FAIL;
+       goto fail;
     if (put_line(fd, "endif") == FAIL)
-       return FAIL;
+       goto fail;
     if (put_line(fd, "unlet! s:wipebuf") == FAIL)
-       return FAIL;
+       goto fail;
 
     // Re-apply 'winheight', 'winwidth' and 'shortmess'.
     if (fprintf(fd, "set winheight=%ld winwidth=%ld shortmess=%s",
                               p_wh, p_wiw, p_shm) < 0 || put_eol(fd) == FAIL)
-       return FAIL;
+       goto fail;
     // Re-apply 'winminheight' and 'winminwidth'.
     if (fprintf(fd, "set winminheight=%ld winminwidth=%ld",
                                      p_wmh, p_wmw) < 0 || put_eol(fd) == FAIL)
-       return FAIL;
+       goto fail;
 
     // Lastly, execute the x.vim file if it exists.
     if (put_line(fd, "let s:sx = expand(\"<sfile>:p:r\").\"x.vim\"") == FAIL
            || put_line(fd, "if filereadable(s:sx)") == FAIL
            || put_line(fd, "  exe \"source \" . fnameescape(s:sx)") == FAIL
            || put_line(fd, "endif") == FAIL)
-       return FAIL;
+       goto fail;
 
-    return OK;
+    ret = OK;
+fail:
+#ifdef FEAT_TERMINAL
+    hash_clear_all(&terminal_bufs, 0);
+#endif
+    return ret;
 }
 
 /*
@@ -1084,6 +1102,11 @@ ex_mkrc(exarg_T  *eap)
     char_u     *viewFile = NULL;
     unsigned   *flagp;
 #endif
+#ifdef FEAT_TERMINAL
+    hashtab_T  terminal_bufs;
+
+    hash_init(&terminal_bufs);
+#endif
 
     if (eap->cmdidx == CMD_mksession || eap->cmdidx == CMD_mkview)
     {
@@ -1240,8 +1263,11 @@ ex_mkrc(exarg_T  *eap)
            }
            else
            {
-               failed |= (put_view(fd, curwin, !using_vdir, flagp,
-                                                                -1) == FAIL);
+               failed |= (put_view(fd, curwin, !using_vdir, flagp, -1
+#ifdef FEAT_TERMINAL
+                                                              , &terminal_bufs
+#endif
+                           ) == FAIL);
            }
            if (put_line(fd, "let &so = s:so_save | let &siso = s:siso_save")
                                                                      == FAIL)

diff --git a/src/terminal.c b/src/terminal.c
index fe84a9fe7b99f03a29fa5ad919cf693b515608a5..e2fcf407a16af34ba93df4677edfc3f1cc51d4db 100644 (file)
--- a/src/terminal.c
+++ b/src/terminal.c
@@ -935,9 +935,30 @@ theend:
  * Return FAIL if writing fails.
  */
     int
-term_write_session(FILE *fd, win_T *wp)
+term_write_session(FILE *fd, win_T *wp, hashtab_T *terminal_bufs)
 {
-    term_T *term = wp->w_buffer->b_term;
+    const int  bufnr = wp->w_buffer->b_fnum;
+    term_T     *term = wp->w_buffer->b_term;
+
+    if (wp->w_buffer->b_nwindows > 1)
+    {
+       // There are multiple views into this terminal buffer. We don't want to
+       // create the terminal multiple times. If it's the first time, create,
+       // otherwise link to the first buffer.
+       char        id_as_str[NUMBUFLEN];
+       hashitem_T  *entry;
+
+       vim_snprintf(id_as_str, sizeof(id_as_str), "%d", bufnr);
+
+       entry = hash_find(terminal_bufs, (char_u *)id_as_str);
+       if (!HASHITEM_EMPTY(entry))
+       {
+           // we've already opened this terminal buffer
+           if (fprintf(fd, "execute 'buffer ' . s:term_buf_%d", bufnr) < 0)
+               return FAIL;
+           return put_eol(fd);
+       }
+    }
 
     // Create the terminal and run the command.  This is not without
     // risk, but let's assume the user only creates a session when this
@@ -951,6 +972,19 @@ term_write_session(FILE *fd, win_T *wp)
 #endif
     if (term->tl_command != NULL && fputs((char *)term->tl_command, fd) < 0)
        return FAIL;
+    if (put_eol(fd) != OK)
+       return FAIL;
+
+    if (fprintf(fd, "let s:term_buf_%d = bufnr()", bufnr) < 0)
+       return FAIL;
+
+    if (wp->w_buffer->b_nwindows > 1)
+    {
+       char *hash_key = alloc(NUMBUFLEN);
+
+       vim_snprintf(hash_key, NUMBUFLEN, "%d", bufnr);
+       hash_add(terminal_bufs, (char_u *)hash_key);
+    }
 
     return put_eol(fd);
 }

diff --git a/src/testdir/test_mksession.vim b/src/testdir/test_mksession.vim
index 5693b97ac0bc2c359282c276037f8fab7e766c6e..66cbca15a0d87776f8a2a4560345dbd968d3e56c 100644 (file)
--- a/src/testdir/test_mksession.vim
+++ b/src/testdir/test_mksession.vim
@@ -455,6 +455,32 @@ func Test_mksession_terminal_restore_other()
   call delete('Xtest_mks.out')
 endfunc
 
+func Test_mksession_terminal_shared_windows()
+  terminal
+  let term_buf = bufnr()
+  new
+  execute "buffer" term_buf
+  mksession! Xtest_mks.out
+
+  let lines = readfile('Xtest_mks.out')
+  let found_creation = 0
+  let found_use = 0
+
+  for line in lines
+    if line =~ '^terminal'
+      let found_creation = 1
+      call assert_match('terminal ++curwin ++cols=\d\+ ++rows=\d\+', line)
+    elseif line =~ "^execute 'buffer ' . s:term_buf_" . term_buf . "$"
+      let found_use = 1
+    endif
+  endfor
+
+  call assert_true(found_creation && found_use)
+
+  call StopShellInTerminal(term_buf)
+  call delete('Xtest_mks.out')
+endfunc
+
 endif " has('terminal')
 
 " Test :mkview with a file argument.

diff --git a/src/version.c b/src/version.c
index 77f17f621458dcc2d64d919edb06d881590255fa..74ed214b1d36524dc614b28b494a7b282ef8bfb6 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -750,6 +750,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    1662,
 /**/
     1661,
 /**/