To: vim_dev@googlegroups.com Subject: Patch 8.2.1662 Fcc: outbox From: Bram Moolenaar Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ------------ Patch 8.2.1662 Problem: :mksession does not restore shared terminal buffer properly. Solution: Keep a hashtab with terminal buffers. (Rob Pilling, closes #6930) Files: src/hashtab.c, src/proto/terminal.pro, src/session.c, src/terminal.c, src/testdir/test_mksession.vim *** ../vim-8.2.1661/src/hashtab.c 2020-07-14 16:15:27.576652590 +0200 --- src/hashtab.c 2020-09-11 20:25:10.560975560 +0200 *************** *** 81,87 **** vim_free(ht->ht_array); } ! #if defined(FEAT_SPELL) || 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 --- 81,87 ---- vim_free(ht->ht_array); } ! #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 *** ../vim-8.2.1661/src/proto/terminal.pro 2020-02-02 15:25:10.097976117 +0100 --- src/proto/terminal.pro 2020-09-11 20:22:35.637566597 +0200 *************** *** 2,8 **** 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_should_restore(buf_T *buf); void free_terminal(buf_T *buf); void free_unused_terminals(void); --- 2,8 ---- 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, hashtab_T *terminal_bufs); int term_should_restore(buf_T *buf); void free_terminal(buf_T *buf); void free_unused_terminals(void); *** ../vim-8.2.1661/src/session.c 2020-04-16 21:04:38.597969228 +0200 --- src/session.c 2020-09-11 20:33:10.267264224 +0200 *************** *** 305,312 **** 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 { win_T *save_curwin; int f; --- 305,316 ---- 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 ! #ifdef FEAT_TERMINAL ! , hashtab_T *terminal_bufs ! #endif ! ) { win_T *save_curwin; int f; *************** *** 349,355 **** # ifdef FEAT_TERMINAL if (bt_terminal(wp->w_buffer)) { ! if (term_write_session(fd, wp) == FAIL) return FAIL; } else --- 353,359 ---- # ifdef FEAT_TERMINAL if (bt_terminal(wp->w_buffer)) { ! if (term_write_session(fd, wp, terminal_bufs) == FAIL) return FAIL; } else *************** *** 588,593 **** --- 592,603 ---- 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,620 **** // sessionable variables. #ifdef FEAT_EVAL if (put_line(fd, "let v:this_session=expand(\":p\")") == FAIL) ! return FAIL; if (ssop_flags & SSOP_GLOBALS) if (store_session_globals(fd) == FAIL) ! return FAIL; #endif // Close all windows and tabs but one. if (put_line(fd, "silent only") == FAIL) ! return FAIL; if ((ssop_flags & SSOP_TABPAGES) && put_line(fd, "silent tabonly") == FAIL) ! return 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(\":p:h\"), ' ')") == FAIL) ! return FAIL; } else if (ssop_flags & SSOP_CURDIR) { --- 606,630 ---- // sessionable variables. #ifdef FEAT_EVAL if (put_line(fd, "let v:this_session=expand(\":p\")") == FAIL) ! goto fail; if (ssop_flags & SSOP_GLOBALS) if (store_session_globals(fd) == FAIL) ! goto fail; #endif // Close all windows and tabs but one. if (put_line(fd, "silent only") == FAIL) ! goto fail; if ((ssop_flags & SSOP_TABPAGES) && put_line(fd, "silent tabonly") == 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(\":p:h\"), ' ')") == FAIL) ! goto fail; } else if (ssop_flags & SSOP_CURDIR) { *************** *** 625,631 **** || put_eol(fd) == FAIL) { vim_free(sname); ! return FAIL; } vim_free(sname); } --- 635,641 ---- || put_eol(fd) == FAIL) { vim_free(sname); ! goto fail; } vim_free(sname); } *************** *** 633,659 **** // 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; if (put_line(fd, " let s:wipebuf = bufnr('%')") == FAIL) ! return FAIL; if (put_line(fd, "endif") == FAIL) ! return FAIL; // Now save the current files, current buffer first. if (put_line(fd, "set shortmess=aoO") == FAIL) ! return FAIL; // the global argument list if (ses_arglist(fd, "argglobal", &global_alist.al_ga, !(ssop_flags & SSOP_CURDIR), &ssop_flags) == FAIL) ! return 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; } #ifdef FEAT_GUI --- 643,669 ---- // 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) ! goto fail; if (put_line(fd, " let s:wipebuf = bufnr('%')") == FAIL) ! goto fail; if (put_line(fd, "endif") == FAIL) ! goto fail; // Now save the current files, current buffer first. if (put_line(fd, "set shortmess=aoO") == FAIL) ! goto fail; // the global argument list if (ses_arglist(fd, "argglobal", &global_alist.al_ga, !(ssop_flags & SSOP_CURDIR), &ssop_flags) == 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) ! goto fail; } #ifdef FEAT_GUI *************** *** 665,671 **** { // Note: after the restore we still check it worked! if (fprintf(fd, "winpos %d %d", x, y) < 0 || put_eol(fd) == FAIL) ! return FAIL; } } #endif --- 675,681 ---- { // Note: after the restore we still check it worked! if (fprintf(fd, "winpos %d %d", x, y) < 0 || put_eol(fd) == FAIL) ! goto fail; } } #endif *************** *** 677,683 **** if (p_stal == 1 && first_tabpage->tp_next != NULL) { if (put_line(fd, "set stal=2") == FAIL) ! return FAIL; restore_stal = TRUE; } --- 687,693 ---- if (p_stal == 1 && first_tabpage->tp_next != NULL) { if (put_line(fd, "set stal=2") == FAIL) ! goto fail; restore_stal = TRUE; } *************** *** 695,703 **** // 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; if (first_tabpage->tp_next != NULL && put_line(fd, "tabrewind") == FAIL) ! return FAIL; } for (tabnr = 1; ; ++tabnr) { --- 705,713 ---- // 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) ! goto fail; if (first_tabpage->tp_next != NULL && put_line(fd, "tabrewind") == FAIL) ! goto fail; } for (tabnr = 1; ; ++tabnr) { *************** *** 739,751 **** ) { if (need_tabnext && put_line(fd, "tabnext") == FAIL) ! return FAIL; need_tabnext = FALSE; if (fputs("edit ", fd) < 0 || ses_fname(fd, wp->w_buffer, &ssop_flags, TRUE) == FAIL) ! return FAIL; if (!wp->w_arg_idx_invalid) edited_win = wp; break; --- 749,761 ---- ) { if (need_tabnext && put_line(fd, "tabnext") == FAIL) ! goto fail; need_tabnext = FALSE; if (fputs("edit ", fd) < 0 || ses_fname(fd, wp->w_buffer, &ssop_flags, TRUE) == FAIL) ! goto fail; if (!wp->w_arg_idx_invalid) edited_win = wp; break; *************** *** 754,770 **** // If no file got edited create an empty tab page. if (need_tabnext && put_line(fd, "tabnext") == FAIL) ! return FAIL; // Save current window layout. if (put_line(fd, "set splitbelow splitright") == FAIL) ! return FAIL; if (ses_win_rec(fd, tab_topframe) == FAIL) ! return FAIL; if (!p_sb && put_line(fd, "set nosplitbelow") == FAIL) ! return FAIL; if (!p_spr && put_line(fd, "set nosplitright") == FAIL) ! return FAIL; // Check if window sizes can be restored (no windows omitted). // Remember the window number of the current window after restoring. --- 764,780 ---- // If no file got edited create an empty tab page. if (need_tabnext && put_line(fd, "tabnext") == FAIL) ! goto fail; // Save current window layout. if (put_line(fd, "set splitbelow splitright") == FAIL) ! goto fail; if (ses_win_rec(fd, tab_topframe) == FAIL) ! goto fail; if (!p_sb && put_line(fd, "set nosplitbelow") == FAIL) ! goto fail; if (!p_spr && put_line(fd, "set nosplitright") == FAIL) ! goto fail; // Check if window sizes can be restored (no windows omitted). // Remember the window number of the current window after restoring. *************** *** 781,787 **** // Go to the first window. if (put_line(fd, "wincmd t") == FAIL) ! return FAIL; // If more than one window, see if sizes can be restored. // First set 'winheight' and 'winwidth' to 1 to avoid the windows being --- 791,797 ---- // Go to the first window. if (put_line(fd, "wincmd t") == 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,802 **** || put_line(fd, "set winheight=1") == FAIL || put_line(fd, "set winminwidth=0") == FAIL || put_line(fd, "set winwidth=1") == FAIL) ! return FAIL; if (nr > 1 && ses_winsizes(fd, restore_size, tab_firstwin) == FAIL) ! return FAIL; // Restore the tab-local working directory if specified // Do this before the windows, so that the window-local directory can --- 804,812 ---- || put_line(fd, "set winheight=1") == FAIL || put_line(fd, "set winminwidth=0") == FAIL || put_line(fd, "set winwidth=1") == FAIL) ! goto fail; if (nr > 1 && ses_winsizes(fd, restore_size, tab_firstwin) == FAIL) ! goto fail; // Restore the tab-local working directory if specified // Do this before the windows, so that the window-local directory can *************** *** 806,812 **** if (fputs("tcd ", fd) < 0 || ses_put_fname(fd, tp->tp_localdir, &ssop_flags) == FAIL || put_eol(fd) == FAIL) ! return FAIL; did_lcd = TRUE; } --- 816,822 ---- if (fputs("tcd ", fd) < 0 || ses_put_fname(fd, tp->tp_localdir, &ssop_flags) == FAIL || put_eol(fd) == FAIL) ! goto fail; did_lcd = TRUE; } *************** *** 815,825 **** { if (!ses_do_win(wp)) continue; ! if (put_view(fd, wp, wp != edited_win, &ssop_flags, ! cur_arg_idx) == FAIL) ! return FAIL; if (nr > 1 && put_line(fd, "wincmd w") == FAIL) ! return FAIL; next_arg_idx = wp->w_arg_idx; } --- 825,838 ---- { if (!ses_do_win(wp)) continue; ! 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) ! goto fail; next_arg_idx = wp->w_arg_idx; } *************** *** 831,842 **** // 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; // 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; // Don't continue in another tab page when doing only the current one // or when at the last tab page. --- 844,855 ---- // 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)) ! 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) ! goto fail; // Don't continue in another tab page when doing only the current one // or when at the last tab page. *************** *** 848,857 **** { if (fprintf(fd, "tabnext %d", tabpage_index(curtab)) < 0 || put_eol(fd) == FAIL) ! return FAIL; } if (restore_stal && put_line(fd, "set stal=1") == FAIL) ! return 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 --- 861,870 ---- { if (fprintf(fd, "tabnext %d", tabpage_index(curtab)) < 0 || put_eol(fd) == FAIL) ! goto fail; } if (restore_stal && put_line(fd, "set stal=1") == 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,909 **** 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; } } // 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; if (put_line(fd, " silent exe 'bwipe ' . s:wipebuf") == FAIL) ! return FAIL; if (put_line(fd, "endif") == FAIL) ! return FAIL; if (put_line(fd, "unlet! s:wipebuf") == FAIL) ! return 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; // Re-apply 'winminheight' and 'winminwidth'. if (fprintf(fd, "set winminheight=%ld winminwidth=%ld", p_wmh, p_wmw) < 0 || put_eol(fd) == FAIL) ! return FAIL; // Lastly, execute the x.vim file if it exists. if (put_line(fd, "let s:sx = expand(\":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; ! return OK; } /* --- 885,927 ---- 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) ! 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) ! goto fail; if (put_line(fd, " silent exe 'bwipe ' . s:wipebuf") == FAIL) ! goto fail; if (put_line(fd, "endif") == FAIL) ! goto fail; if (put_line(fd, "unlet! s:wipebuf") == 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) ! goto fail; // Re-apply 'winminheight' and 'winminwidth'. if (fprintf(fd, "set winminheight=%ld winminwidth=%ld", p_wmh, p_wmw) < 0 || put_eol(fd) == FAIL) ! goto fail; // Lastly, execute the x.vim file if it exists. if (put_line(fd, "let s:sx = expand(\":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) ! goto fail; ! ret = OK; ! fail: ! #ifdef FEAT_TERMINAL ! hash_clear_all(&terminal_bufs, 0); ! #endif ! return ret; } /* *************** *** 1084,1089 **** --- 1102,1112 ---- 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,1247 **** } else { ! failed |= (put_view(fd, curwin, !using_vdir, flagp, ! -1) == FAIL); } if (put_line(fd, "let &so = s:so_save | let &siso = s:siso_save") == FAIL) --- 1263,1273 ---- } else { ! 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) *** ../vim-8.2.1661/src/terminal.c 2020-09-06 18:22:47.245500821 +0200 --- src/terminal.c 2020-09-11 20:31:11.743675062 +0200 *************** *** 935,943 **** * Return FAIL if writing fails. */ int ! term_write_session(FILE *fd, win_T *wp) { ! term_T *term = wp->w_buffer->b_term; // Create the terminal and run the command. This is not without // risk, but let's assume the user only creates a session when this --- 935,964 ---- * Return FAIL if writing fails. */ int ! term_write_session(FILE *fd, win_T *wp, hashtab_T *terminal_bufs) { ! 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,956 **** --- 972,990 ---- #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); } *** ../vim-8.2.1661/src/testdir/test_mksession.vim 2020-08-12 18:50:31.883655785 +0200 --- src/testdir/test_mksession.vim 2020-09-11 20:22:35.637566597 +0200 *************** *** 455,460 **** --- 455,486 ---- 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. *** ../vim-8.2.1661/src/version.c 2020-09-11 19:28:16.019436292 +0200 --- src/version.c 2020-09-11 20:24:34.165112150 +0200 *************** *** 752,753 **** --- 752,755 ---- { /* Add new patch number below this line */ + /**/ + 1662, /**/ -- BRIDGEKEEPER: What is your favorite colour? GAWAIN: Blue ... No yelloooooww! "Monty Python and the Holy Grail" PYTHON (MONTY) PICTURES LTD /// Bram Moolenaar -- Bram@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 ///