patch 9.0.0710: quitting/unloading/hiding a terminal does not work properlyv9.0.0710

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)

4 files changed, 46 insertions, 11 deletions

diff --git a/runtime/doc/terminal.txt b/runtime/doc/terminal.txt
index 0b94f8a04..4d02558e4 100644
--- 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 97b41e0a8..b589894b7 100644
--- 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 80f5db61d..69faa3dfc 100644
--- 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 765f6dfc7..2b922f3ab 100644
--- a/src/version.c
+++ b/src/version.c
@@ -700,6 +700,8 @@ static char *(features[]) =
 static int included_patches[] =
 {   /* Add new patch number below this line */
 /**/
+    710,
+/**/
     709,
 /**/
     708,