[vim/vim] Save/restore feature refactored, added tests, updated docs. (PR #15032)

ubaldot

unread,

Jun 17, 2024, 8:52:39 AM (9 days ago) Jun 17

to vim/vim, Subscribed

Save/restore feature refactored,
Tests added,
Variable naming consistency improved,
Docs updated,
Additional refactoring performed.

You can view, comment on, or merge this pull request online at:

https://github.com/vim/vim/pull/15032

Commit Summary

0be5ca0 Refactored save and restore feature.
206e387 Name consistency, further refactoring.
60f5176 Updated docs.
e005fc4 Termdebug already loaded check re-enabled

File Changes

(3 files)

M runtime/doc/terminal.txt (5)
M runtime/pack/dist/opt/termdebug/plugin/termdebug.vim (150)
M src/testdir/test_termdebug.vim (32)

Patch Links:

—
Reply to this email directly, view it on GitHub.
You are receiving this because you are subscribed to this thread.

ubaldot

unread,

Jun 17, 2024, 8:55:03 AM (9 days ago) Jun 17

to vim/vim, Subscribed

@yegappan Could you please check with your setting of g:termdebug_config['wide'] if you get the desired result? That feature is a bit difficult to test through unit testing.

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.

ubaldot

unread,

Jun 17, 2024, 9:07:50 AM (9 days ago) Jun 17

to vim/vim, Push

@ubaldot pushed 1 commit.

f7c6583 Removed debug echom

—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.

Christian Brabandt

unread,

Jun 17, 2024, 12:04:49 PM (9 days ago) Jun 17

to vim/vim, Subscribed

@chrisbra commented on this pull request.

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

> -    elseif empty(plus_map_saved)
-      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
-      # pass
-    elseif !empty(minus_map_saved) && !minus_map_saved.buffer
-      nunmap -
-      mapset(minus_map_saved)
-    elseif empty(minus_map_saved)
-      silent! nunmap -
-    endif
-    minus_map_saved = null_dict
+  if !empty(saved_k_map)

you dropped the !k_map_saved.buffer intentionally here?

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.

ubaldot

unread,

Jun 18, 2024, 2:26:11 AM (8 days ago) Jun 18

to vim/vim, Subscribed

@ubaldot commented on this pull request.

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

> -    elseif empty(plus_map_saved)
-      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
-      # pass
-    elseif !empty(minus_map_saved) && !minus_map_saved.buffer
-      nunmap -
-      mapset(minus_map_saved)
-    elseif empty(minus_map_saved)
-      silent! nunmap -
-    endif
-    minus_map_saved = null_dict
+  if !empty(saved_k_map)

Yes. The logic now is much easier and it goes like the following:

At termdebug startup user-defined mapping are always saved no matter of the user settings. Maps are always saved even if they are empty (that happens in the Init function).
Termdebug set the mapping depending on user settings.
At teadown, termdebug restore the saved mapping exactly as they were before staring gdb.

The empty condition is a guard in case the saved_k_map is empty. In that case mapset would return an error.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.

Christian Brabandt

unread,

Jun 18, 2024, 2:29:03 AM (8 days ago) Jun 18

to vim/vim, Subscribed

@chrisbra commented on this pull request.

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

> -    elseif empty(plus_map_saved)
-      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
-      # pass
-    elseif !empty(minus_map_saved) && !minus_map_saved.buffer
-      nunmap -
-      mapset(minus_map_saved)
-    elseif empty(minus_map_saved)
-      silent! nunmap -
-    endif
-    minus_map_saved = null_dict
+  if !empty(saved_k_map)

But the problem is, if the mapping was a buffer-local (the buffer key is set) mapping, we may restore it wrongly here (because it get's re-created in the wrong buffer).

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.

ubaldot

unread,

Jun 18, 2024, 2:47:03 AM (8 days ago) Jun 18

to vim/vim, Subscribed

@ubaldot commented on this pull request.

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

> -    elseif empty(plus_map_saved)
-      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
-      # pass
-    elseif !empty(minus_map_saved) && !minus_map_saved.buffer
-      nunmap -
-      mapset(minus_map_saved)
-    elseif empty(minus_map_saved)
-      silent! nunmap -
-    endif
-    minus_map_saved = null_dict
+  if !empty(saved_k_map)

Correct. The maparg only stores info if the mapping was buffer-local but not to what buffer it relates. I'll look into that. Thanks!

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.

ubaldot

unread,

Jun 18, 2024, 3:08:05 AM (8 days ago) Jun 18

to vim/vim, Push

@ubaldot pushed 1 commit.

f51b43c fixed local-buffer save/restore mappings

—
View it on GitHub or unsubscribe.

You are receiving this because you are subscribed to this thread.

ubaldot

unread,

Jun 18, 2024, 3:10:33 AM (8 days ago) Jun 18

to vim/vim, Subscribed

@ubaldot commented on this pull request.

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

> -    elseif empty(plus_map_saved)
-      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
-      # pass
-    elseif !empty(minus_map_saved) && !minus_map_saved.buffer
-      nunmap -
-      mapset(minus_map_saved)
-    elseif empty(minus_map_saved)
-      silent! nunmap -
-    endif
-    minus_map_saved = null_dict
+  if !empty(saved_k_map)

Done! Check it now. I also updated the related variables names with capital letter for K since it is K mapping and not k.

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.

Christian Brabandt

unread,

Jun 18, 2024, 2:11:46 PM (8 days ago) Jun 18

to vim/vim, Subscribed

thanks. I'd like that you include a new test!

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.

Christian Brabandt

unread,

Jun 18, 2024, 2:26:51 PM (8 days ago) Jun 18

to vim/vim, Subscribed

Closed #15032 via a48637c.

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.

ubaldot

unread,

Jun 18, 2024, 2:45:53 PM (8 days ago) Jun 18

to vim/vim, Subscribed

thanks. I'd like that you include a new test!

Sure thing!
I'll add it tomorrow. :)

—
Reply to this email directly, view it on GitHub.

You are receiving this because you are subscribed to this thread.

Christian Brabandt

unread,

Jun 18, 2024, 2:48:16 PM (8 days ago) Jun 18

to vim/vim, Subscribed

😆 What I was trying to say: "Thanks for adding a test for the save/restore function". But I take more tests ;)

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.

ubaldot

unread,

Jun 18, 2024, 3:06:10 PM (8 days ago) Jun 18

to vim/vim, Subscribed

😀 ok! Yet, the use-case you raised is not covered by any test, otherwise some test must have failed before you notice the bug. Better add one test ;)

Related: I don't know what is your take on that 'maparg' store if a mapping has buffer scope but not the related buffer number/name. Is it worth opening an issue labeled as enhancement?

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.

Shane-XB-Qian

unread,

Jun 18, 2024, 3:31:21 PM (8 days ago) Jun 18

to vim/vim, Subscribed

really? the test was there but just the err was from vim itself when K mapping was none at all.
i may suggest you stop burning on this topic anymore, thank you tidy up some var naming here this ticket, but It maybe no interested to argue it too much, but/and this refactor totally actually was no adding any funcy thing, but left much argument.

--
shane.xb.qian