patch 9.1.0497: termdebug can be further improved

Problem:  termdebug can be further improved
Solution: refactor save/restore, update docs,
          add a new save/restore test (Ubaldo Tiberi)

closes: #15032

Signed-off-by: Ubaldo Tiberi <ubaldo.tiberi@google.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>

runtime/doc/terminal.txt

@@ -1,4 +1,4 @@
-*terminal.txt*	For Vim version 9.1.  Last change: 2024 Jun 17
+*terminal.txt*	For Vim version 9.1.  Last change: 2024 Jun 18
 
 
 		  VIM REFERENCE MANUAL	  by Bram Moolenaar
@@ -1524,6 +1524,9 @@ If there is no g:termdebug_config you can use: >
 	let g:termdebug_use_prompt = 1
 <
 Mappings ~
+The termdebug plugin enables a few default mappings.  All those mappings
+are reset to their original values once the termdebug session concludes.
+
 					*termdebug_map_K* *termdebug-mappings*
 The K key is normally mapped to |:Evaluate| unless a buffer local (|:map-local|)
 mapping to K already exists.  If you do not want this use: >

runtime/pack/dist/opt/termdebug/plugin/termdebug.vim

@@ -75,9 +75,11 @@ var varbufname: string
 var asmbufnr: number
 var asmbufname: string
 var promptbuf: number
-# This is for the "debugged program" thing
+# This is for the "debugged-program" thing
 var ptybufnr: number
+var ptybufname: string
 var commbufnr: number
+var commbufname: string
 
 var gdbjob: job
 var gdb_channel: channel
@@ -110,7 +112,7 @@ var evalexpr: string
 # Remember the old value of 'signcolumn' for each buffer that it's set in, so
 # that we can restore the value for all buffers.
 var signcolumn_buflist: list<number>
-var save_columns: number
+var saved_columns: number
 
 var allleft: bool
 # This was s:vertical but I cannot use vertical as variable name
@@ -120,9 +122,9 @@ var winbar_winids: list<number>
 
 var saved_mousemodel: string
 
-var k_map_saved: dict<any>
+var saved_K_map: dict<any>
-var plus_map_saved: dict<any>
+var saved_plus_map: dict<any>
-var minus_map_saved: dict<any>
+var saved_minus_map: dict<any>
 
 
 def InitScriptVariables()
@@ -155,8 +157,10 @@ def InitScriptVariables()
   asmbufnr = 0
   asmbufname = 'Termdebug-asm-listing'
   promptbuf = 0
-  # This is for the "debugged program" thing
+  # This is for the "debugged-program" thing
+  ptybufname = "debugged-program"
   ptybufnr = 0
+  commbufname = "gdb-communication"
   commbufnr = 0
 
   gdbjob = null_job
@@ -190,16 +194,16 @@ def InitScriptVariables()
   # Remember the old value of 'signcolumn' for each buffer that it's set in, so
   # that we can restore the value for all buffers.
   signcolumn_buflist = [bufnr()]
-  save_columns = 0
+  saved_columns = &columns
 
   winbar_winids = []
 
-  k_map_saved = null_dict
+  saved_K_map = maparg('K', 'n', 0, 1)
-  plus_map_saved = null_dict
+  saved_plus_map = maparg('+', 'n', 0, 1)
-  minus_map_saved = null_dict
+  saved_minus_map = maparg('-', 'n', 0, 1)
 
   if has('menu')
-    saved_mousemodel = null_string
+    saved_mousemodel = &mousemodel
   endif
 enddef
 # The command that starts debugging, e.g. ":Termdebug vim".
@@ -292,7 +296,6 @@ def StartDebug_internal(dict: dict<any>)
   endif
   if wide > 0
     if &columns < wide
-      save_columns = &columns
       &columns = wide
       # If we make the Vim window wider, use the whole left half for the debug
       # windows.
@@ -328,11 +331,10 @@ enddef
 
 # Use when debugger didn't start or ended.
 def CloseBuffers()
-  var bufnames = ['debugged\ program', 'gdb\ communication', asmbufname, varbufname]
+  var buf_numbers = [ptybufnr, commbufnr, asmbufnr, varbufnr]
-  for bufname in bufnames
+  for buf_nr in buf_numbers
-    var buf_nr = bufnr(bufname)
     if buf_nr > 0 && bufexists(buf_nr)
-      exe $'bwipe! {bufname}'
+      exe $'bwipe! {buf_nr}'
     endif
   endfor
 
@@ -340,7 +342,6 @@ def CloseBuffers()
   gdbwin = 0
 enddef
 
-# IsGdbRunning(): bool may be a better name?
 def IsGdbStarted(): bool
   var gdbproc_status = job_status(term_getjob(gdbbufnr))
   if gdbproc_status !=# 'run'
@@ -355,7 +356,7 @@ enddef
 # Open a terminal window without a job, to run the debugged program in.
 def StartDebug_term(dict: dict<any>)
   ptybufnr = term_start('NONE', {
-    term_name: 'debugged program',
+    term_name: ptybufname,
     vertical: vvertical})
   if ptybufnr == 0
     Echoerr('Failed to open the program terminal window')
@@ -376,7 +377,7 @@ def StartDebug_term(dict: dict<any>)
 
   # Create a hidden terminal window to communicate with gdb
   commbufnr = term_start('NONE', {
-    term_name: 'gdb communication',
+    term_name: commbufname,
     out_cb: function('CommOutput'),
     hidden: 1
   })
@@ -872,16 +873,12 @@ enddef
 
 def EndDebugCommon()
   var curwinid = win_getid()
-
+  var buf_numbers = [ptybufnr, asmbufnr, varbufnr]
-  if ptybufnr > 0 && bufexists(ptybufnr)
+  for buf_nr in buf_numbers
-    exe $'bwipe! {ptybufnr}'
+    if buf_nr > 0 && bufexists(buf_nr)
-  endif
+      exe $'bwipe! {buf_nr}'
-  if asmbufnr > 0 && bufexists(asmbufnr)
+    endif
-    exe $'bwipe! {asmbufnr}'
+  endfor
-  endif
-  if varbufnr > 0 && bufexists(varbufnr)
-    exe $'bwipe! {varbufnr}'
-  endif
   running = false
 
   # Restore 'signcolumn' in all buffers for which it was set.
@@ -904,9 +901,7 @@ def EndDebugCommon()
 
   win_gotoid(curwinid)
 
-  if save_columns > 0
+  &columns = saved_columns
-    &columns = save_columns
-  endif
 
   if has("balloon_eval") || has("balloon_eval_term")
     set balloonexpr=
@@ -1137,8 +1132,7 @@ def InstallCommands()
   endif
 
   if map
-    k_map_saved = maparg('K', 'n', 0, 1)
+    if !empty(saved_K_map) && !saved_K_map.buffer || empty(saved_K_map)
-    if !empty(k_map_saved) && !k_map_saved.buffer || empty(k_map_saved)
       nnoremap K :Evaluate<CR>
     endif
   endif
@@ -1148,8 +1142,7 @@ def InstallCommands()
     map = get(g:termdebug_config, 'map_plus', 1)
   endif
   if map
-    plus_map_saved = maparg('+', 'n', 0, 1)
+    if !empty(saved_plus_map) && !saved_plus_map.buffer || empty(saved_plus_map)
-    if !empty(plus_map_saved) && !plus_map_saved.buffer || empty(plus_map_saved)
       nnoremap <expr> + $'<Cmd>{v:count1}Up<CR>'
     endif
   endif
@@ -1159,8 +1152,7 @@ def InstallCommands()
     map = get(g:termdebug_config, 'map_minus', 1)
   endif
   if map
-    minus_map_saved = maparg('-', 'n', 0, 1)
+    if !empty(saved_minus_map) && !saved_minus_map.buffer || empty(saved_minus_map)
-    if !empty(minus_map_saved) && !minus_map_saved.buffer || empty(minus_map_saved)
       nnoremap <expr> - $'<Cmd>{v:count1}Down<CR>'
     endif
   endif
@@ -1177,7 +1169,6 @@ def InstallCommands()
     endif
 
     if pup
-      saved_mousemodel = &mousemodel
       &mousemodel = 'popup_setpos'
       an 1.200 PopUp.-SEP3-	<Nop>
       an 1.210 PopUp.Set\ breakpoint	:Break<CR>
@@ -1232,38 +1223,32 @@ def DeleteCommands()
   delcommand Var
   delcommand Winbar
 
-  if k_map_saved isnot null_dict
+
-    if !empty(k_map_saved) && k_map_saved.buffer
+  if !empty(saved_K_map) && saved_K_map.buffer
-      # pass
+    # pass
-    elseif !empty(k_map_saved) && !k_map_saved.buffer
+  elseif !empty(saved_K_map) && !saved_K_map.buffer
-      nunmap K
+    nunmap K
-      mapset(k_map_saved)
+    mapset(saved_K_map)
-    elseif empty(k_map_saved)
+  elseif empty(saved_K_map)
-      silent! nunmap K
+    silent! nunmap K
-    endif
-    k_map_saved = null_dict
   endif
-  if plus_map_saved isnot null_dict
+
-    if !empty(plus_map_saved) && plus_map_saved.buffer
+  if !empty(saved_plus_map) && saved_plus_map.buffer
-      # pass
+    # pass
-    elseif !empty(plus_map_saved) && !plus_map_saved.buffer
+  elseif !empty(saved_plus_map) && !saved_plus_map.buffer
-      nunmap +
+    nunmap +
-      mapset(plus_map_saved)
+    mapset(saved_plus_map)
-    elseif empty(plus_map_saved)
+  elseif empty(saved_plus_map)
-      silent! nunmap +
+    silent! nunmap +
-    endif
-    plus_map_saved = null_dict
   endif
-  if minus_map_saved isnot null_dict
+
-    if !empty(minus_map_saved) && minus_map_saved.buffer
+  if !empty(saved_minus_map) && saved_minus_map.buffer
-      # pass
+    # pass
-    elseif !empty(minus_map_saved) && !minus_map_saved.buffer
+  elseif !empty(saved_minus_map) && !saved_minus_map.buffer
-      nunmap -
+    nunmap -
-      mapset(minus_map_saved)
+    mapset(saved_minus_map)
-    elseif empty(minus_map_saved)
+  elseif empty(saved_minus_map)
-      silent! nunmap -
+    silent! nunmap -
-    endif
-    minus_map_saved = null_dict
   endif
 
   if has('menu')
@@ -1280,21 +1265,18 @@ def DeleteCommands()
       endif
     endfor
     win_gotoid(curwinid)
-    winbar_winids = []
+    # winbar_winids = []
 
-    if saved_mousemodel isnot null_string
+    &mousemodel = saved_mousemodel
-      &mousemodel = saved_mousemodel
+    try
-      saved_mousemodel = null_string
+      aunmenu PopUp.-SEP3-
-      try
+      aunmenu PopUp.Set\ breakpoint
-        aunmenu PopUp.-SEP3-
+      aunmenu PopUp.Clear\ breakpoint
-        aunmenu PopUp.Set\ breakpoint
+      aunmenu PopUp.Run\ until
-        aunmenu PopUp.Clear\ breakpoint
+      aunmenu PopUp.Evaluate
-        aunmenu PopUp.Run\ until
+    catch
-        aunmenu PopUp.Evaluate
+      # ignore any errors in removing the PopUp menu
-      catch
+    endtry
-        # ignore any errors in removing the PopUp menu
-      endtry
-    endif
   endif
 
   sign_unplace('TermDebug')
@@ -1342,8 +1324,6 @@ def SetBreakpoint(at: string, tbreak=false)
   else
     cmd = $'-break-insert {AT}'
   endif
-  # OK
-  # echom $"cmsd: {cmd}"
   SendCommand(cmd)
   if do_continue
     ContinueCommand()

src/testdir/test_termdebug.vim

@@ -297,6 +297,9 @@ func Test_termdebug_mapping()
   call assert_true(maparg('+', 'n', 0, 1).buffer)
   call assert_equal(':echom "bK"<cr>', maparg('K', 'n', 0, 1).rhs)
 
+  nunmap K
+  nunmap +
+  nunmap -
   %bw!
 endfunc
 
@@ -339,14 +342,41 @@ func Test_termdebug_bufnames()
 endfunc
 
 function Test_termdebug_save_restore_variables()
+  " saved mousemodel
   let &mousemodel=''
+
+  " saved keys
+  nnoremap K :echo "hello world!"<cr>
+  let expected_map_K = maparg('K', 'n', 0 , 1)
+  nnoremap + :echo "hello plus!"<cr>
+  let expected_map_plus = maparg('+', 'n', 0 , 1)
+  let expected_map_minus = {}
+
+  " saved &columns
+  let expected_columns = &columns
+
+  " We want termdebug to overwrite 'K' map but not '+' map.
+  let g:termdebug_config = {}
+  let g:termdebug_config['map_K'] = 1
+
   Termdebug
   call WaitForAssert({-> assert_equal(3, winnr('$'))})
   call WaitForAssert({-> assert_match(&mousemodel, 'popup_setpos')})
   wincmd t
   quit!
   call WaitForAssert({-> assert_equal(1, winnr('$'))})
-  call WaitForAssert({-> assert_true(empty(&mousemodel))})
+
+  call assert_true(empty(&mousemodel))
+
+  call assert_true(empty(expected_map_minus))
+  call assert_equal(expected_map_K.rhs, maparg('K', 'n', 0, 1).rhs)
+  call assert_equal(expected_map_plus.rhs, maparg('+', 'n', 0, 1).rhs)
+
+  call assert_equal(expected_columns, &columns)
+
+  nunmap K
+  nunmap +
+  unlet g:termdebug_config
 endfunction
 
 

src/version.c

@@ -704,6 +704,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    497,
 /**/
     496,
 /**/