patch 9.0.0710: quitting/unloading/hiding a terminal does not work properly

Problem:    Quitting/unloading/hiding a terminal buffer does not always work
            properly.
Solution:   Avoid that ":q!" leaves an empty buffer behind.  ":bunload!" also
            kills the job and unloads the buffer.  ":hide" does not unload the
            buffer. (Yee Cheng Chin, closes #11323)

diff --git a/runtime/doc/terminal.txt b/runtime/doc/terminal.txt
index 0b94f8a043493622e6624235590ed60fce828908..4d02558e4811fccdf8213768d582405709378362 100644 (file)
--- a/runtime/doc/terminal.txt
+++ b/runtime/doc/terminal.txt
@@ -288,9 +288,8 @@ way to kill or interrupt the job.  For example: >
 
 So long as the job is running the window behaves like it contains a modified
 buffer.  Trying to close the window with `CTRL-W :quit` fails.  When using
-`CTRL-W :quit!` the job is ended.  The text in the window is lost.  The buffer
-still exists, but getting it in a window with `:buffer` will show an empty
-buffer.
+`CTRL-W :quit!` the job is ended.  The text in the window is lost, the buffer
+is deleted.  With `CTRL-W :bunload!` the buffer remains but will be empty.
 
 Trying to close the window with `CTRL-W :close` also fails.   Using
 `CTRL-W :close!` will close the window and make the buffer hidden.

diff --git a/src/buffer.c b/src/buffer.c
index 97b41e0a89c510d32ffabcb1bb71f654e6e06923..b589894b7e1ec95520d2a5f2f7b39f6e9443100a 100644 (file)
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -538,7 +538,8 @@ close_buffer(
        unload_buf = TRUE;
 
 #ifdef FEAT_TERMINAL
-    if (bt_terminal(buf) && (buf->b_nwindows == 1 || del_buf))
+    // depending on how we get here b_nwindows may already be zero
+    if (bt_terminal(buf) && (buf->b_nwindows <= 1 || del_buf))
     {
        CHECK_CURBUF;
        if (term_job_running(buf->b_term))
@@ -550,6 +551,11 @@ close_buffer(
 
                // Wiping out or unloading a terminal buffer kills the job.
                free_terminal(buf);
+
+               // A terminal buffer is wiped out when job has finished.
+               del_buf = TRUE;
+               unload_buf = TRUE;
+               wipe_buf = TRUE;
            }
            else
            {
@@ -565,10 +571,16 @@ close_buffer(
        }
        else
        {
-           // A terminal buffer is wiped out if the job has finished.
-           del_buf = TRUE;
-           unload_buf = TRUE;
-           wipe_buf = TRUE;
+           if (del_buf || unload_buf)
+           {
+               // A terminal buffer is wiped out if the job has finished.
+               // We only do this when there's an intention to unload the
+               // buffer. This way, :hide and other similar commands won't
+               // wipe the buffer.
+               del_buf = TRUE;
+               unload_buf = TRUE;
+               wipe_buf = TRUE;
+           }
        }
        CHECK_CURBUF;
     }

diff --git a/src/testdir/test_terminal.vim b/src/testdir/test_terminal.vim
index 80f5db61d7ac0bf0cabae1f3fdba100d18db8496..69faa3dfc48de9000e80fa9a64e39aba0d6a8071 100644 (file)
--- a/src/testdir/test_terminal.vim
+++ b/src/testdir/test_terminal.vim
@@ -96,6 +96,16 @@ func Test_terminal_paste_register()
   unlet g:job
 endfunc
 
+func Test_terminal_unload_buffer()
+  let buf = Run_shell_in_terminal({})
+  call assert_fails(buf . 'bunload', 'E948:')
+  exe buf . 'bunload!'
+  call WaitForAssert({-> assert_equal('dead', job_status(g:job))})
+  call assert_equal("", bufname(buf))
+
+  unlet g:job
+endfunc
+
 func Test_terminal_wipe_buffer()
   let buf = Run_shell_in_terminal({})
   call assert_fails(buf . 'bwipe', 'E948:')
@@ -202,7 +212,7 @@ func Test_terminal_quit()
   quit!
   call assert_notequal(buf, bufnr())
   call WaitForAssert({-> assert_equal('dead', job_status(g:job))})
-  exec buf .. 'bwipe!'
+  call assert_equal("", bufname(buf))
 
   unlet g:job
 endfunc
@@ -237,7 +247,7 @@ func Test_terminal_split_quit()
   quit!
   call WaitForAssert({-> assert_equal('dead', job_status(g:job))})
 
-  exe buf . 'bwipe'
+  call assert_equal("", bufname(buf))
   unlet g:job
 endfunc
 
@@ -261,16 +271,28 @@ endfunc
 func Test_terminal_hide_buffer_job_finished()
   term echo hello
   let buf = bufnr()
-  setlocal bufhidden=hide
   call WaitForAssert({-> assert_equal('finished', term_getstatus(buf))})
+
+  call assert_true(bufloaded(buf))
+  call assert_true(buflisted(buf))
+
+  " Test :hide
+  hide
   call assert_true(bufloaded(buf))
   call assert_true(buflisted(buf))
+  split
+  exe buf .. 'buf'
+  call assert_equal(buf, bufnr())
+
+  " Test bufhidden, which exercises a different code path
+  setlocal bufhidden=hide
   edit Xasdfasdf
   call assert_true(bufloaded(buf))
   call assert_true(buflisted(buf))
   exe buf .. 'buf'
   call assert_equal(buf, bufnr())
   setlocal bufhidden=
+
   edit Xasdfasdf
   call assert_false(bufloaded(buf))
   call assert_false(buflisted(buf))

diff --git a/src/version.c b/src/version.c
index 765f6dfc721f6633f780308bb991b35c08a5182f..2b922f3ab0db7b15fa6d44cb15b25a0ae0e1bccf 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -699,6 +699,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    710,
 /**/
     709,
 /**/