What are legal mode values?

From :help maparg():

{mode} can be one of these strings:
"n" Normal
"v" Visual (including Select)
"o" Operator-pending
"i" Insert
"c" Cmd-line
"s" Select
"x" Visual
"l" langmap |language-mapping|
"t" Terminal-Job
"" Normal, Visual and Operator-pending
When {mode} is omitted, the modes for "" are used.

What kind of issues should be considered where dict.mode covers
more than one mode?

From this comment in issue #6118:

Then maparg() would need to return a list of dictionaries for all
matching modes. That's getting complicated.

But I still don't understand why we would need to return a list of dictionaries, and not just a single dictionary. Although, to be able to return a single dictionary, we would need maparg() (and possibly other functions) to support more pseudo-modes like !, nox, ...

Example:

vim9script
noremap! <F3> aaa
echomsg maparg('<F3>', '!', false, true)

{'lnum': 2, 'script': 0, 'mode': '!', 'silent': 0, 'noremap': 1, 'lhs': '<F3>', 'lhsraw': '<80>k3', 'nowait': 0, 'expr' : 0, 'sid': 1, 'rhs': 'aaa', 'buffer': 0, 'scriptversion': 999999}

Should we doc that the dict from maparg/maplist should not be modified?

Maybe we could just modify the current example to lock the variable so that it doesn't get unexpectedly changed:

diff --git a/runtime/doc/builtin.txt b/runtime/doc/builtin.txt
index e6e6dc272..7360b77c4 100644
--- a/runtime/doc/builtin.txt
+++ b/runtime/doc/builtin.txt
@@ -5364,7 +5364,7 @@ mapset({mode}, {abbr}, {dict})					*mapset()*
 		{mode} is used to define the mode in which the mapping is set,
 		not the "mode" entry in {dict}.
 		Example for saving and restoring a mapping: >
-			let save_map = maparg('K', 'n', 0, 1)
+			const save_map = maparg('K', 'n', 0, 1)
 			nnoremap K somethingelse
 			...
 			call mapset('n', 0, save_map)

Unrelated, but there are other pitfalls with maparg()/mapset().

First, a mapping can be global or local to a buffer, but there is no way to specify to maparg() in which scope we're interested. This means that we can ask for a local mapping and get a dict related to a global one, and vice versa. This is tricky to handle. For example, to get a dict about a global mapping, we might need to temporarily remove a buffer-local shadowing mapping.

Second, for a non-mapped key, maparg() gives an empty dict, which does not contain enough info to restore the mapping later. For example:

vim9script
const map_save = maparg('<F3>', 'n', false, true)
nnoremap <F3> <Cmd>echomsg 'this is just a temporary mapping'<CR>
mapset('n', false, map_save)

E460: entries missing in mapset() dict argument

maparg() should have given us at least a lhs key, a mode key, a buffer key, and maybe a new boolean unmapped key (or maybe an empty rhs key).

Third, for a buffer-local mapping, maparg() does not give us the number of the buffer from which it was saved.

vim9script
edit one
nnoremap <buffer> <F3> <ScriptCmd>echomsg 'should be restored in buffer one'<CR>
const map_save = maparg('<F3>', 'n', false, true)
nunmap <buffer><F3>
edit two
mapset('n', false, map_save)
feedkeys("\<F3>")

should be restored in buffer one

Notice that the mapping has been restored in the buffer two, while it was originally local to the buffer one. That's because mapset() doesn't care about finding the right buffer; instead, it just uses the current one.

There might be other pitfalls like the fact that maparg() translates <SID> inconsistently (it does when {dict} is false, but not when it's true).

maplist() only finds buffer-local mappings in the current buffer. Thus to restore these one has to be in that same buffer.
I don't think trying to have mapset() use the same buffer is what we want. Just documenting this behavior should be sufficient.

Nit: arg_string_or_dict_any() arguments are too long, should be using the one argument per line formatting.

dict_get_string() with "save" FALSE returns the pointer into the typval, thus a change to the dict may make it invalid.
The pointer should only be used for as long as it's certain the dict member won't change.

Related to issue #6118, I'm wondering if the new get_map_mode_string() should be used by mapset('',bool,dict). Then you could do mapset('nox', false, dict) or '!'. But this is slightly incompatible, since it return an error if the mode does not "fit" into a single mapping, like trying to use nc.

for a non-mapped key, maparg() gives an empty dict

Unmapping an lhs is messy. I've got the following.

    const unmap_cmds = [ 'unmap', 'unmap!', 'tunmap', 'lunmap' ]
    def UnmapAll(lhs: string)
        for cmd in unmap_cmds
            try | execute(cmd .. ' ' .. lhs) | catch /E31/ | endtry
        endfor
    enddef

Adding a unmapped key seems a little weird. I'd think an unmapall builtin would be better.

buffer-local mapping ... not the buffer from which it was saved.

That's a feature which allows copying local mappings between buffers. :-)

like the fact that maparg() translates inconsistently

I don't really understand that or the implications.

Nit: arg_string_or_dict_any() arguments are too long,

Got it. That file is heavily infected with that issue.

Tests on the way, and looks like there are merge conflicts...

Make sure to note/check

• get_map_mode_string()
• mapset(mode, abbr, dict) now uses get_map_mode_string()
  An existing script that had an illegal mode, will now fail. That should be the only change that affects existing scripts. And now the mode in the 3arg version can use the extended mode strings like '!', 'nox'
• @lacygoill Can "maparg()" does not support the "!" mode #6118 be closed?
• There is not a compile time check that if first arg is dict, it is the only arg. Something like arg_mapset1() could be introduced to do that (I think).

What else?

Codecov Report

Merging #10295 (884d339) into master (12e21e3) will increase coverage by 1.48%.
The diff coverage is 90.41%.

@@            Coverage Diff             @@
##           master   #10295      +/-   ##
==========================================
+ Coverage   80.98%   82.46%   +1.48%     
==========================================
  Files         161      148      -13     
  Lines      185638   171751   -13887     
  Branches    41973    38832    -3141     
==========================================
- Hits       150335   141639    -8696     
+ Misses      22778    17500    -5278     
- Partials    12525    12612      +87     

Flags with carried forward coverage won't be shown. Click here to find out more.

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 12e21e3...884d339. Read the comment docs.

I closed the issue #6118, because this PR helps a lot, but it doesn't fix it entirely.

Before the PR:

vim9script
noremap! <C-A> <C-B>
echo 'BEFORE restoration'
noremap! <C-A>
const map_save = {
    i: maparg('<C-A>', 'i', false, true),
    c: maparg('<C-A>', 'c', false, true)
}
unmap! <C-A>
mapset('i', false, map_save.i)
mapset('c', false, map_save.c)
echo 'AFTER restoration'
noremap! <C-A>

BEFORE restoration
!  <C-A>       * <C-B>
AFTER restoration
c  <C-A>       * <C-B>
i  <C-A>       * <C-B>

The code was needlessly complex (in part because of too many invocations of maparg() and mapset()), and couldn't even restore the mapping exactly as it was defined originally.

After the PR:

vim9script
noremap! <C-A> <C-B>
echo 'BEFORE restoration'
noremap! <C-A>
const map_save = maparg('<C-A>', 'i', false, true)
unmap! <C-A>
mapset(map_save)
echo 'AFTER restoration'
noremap! <C-A>

BEFORE restoration
!  <C-A>       * <C-B>
AFTER restoration
!  <C-A>       * <C-B>

The code is simpler, and the mapping is restored exactly as it was defined.

However, when we've saved the mapping, notice that we had to use one of the submodes from ! (here i):

const map_save = maparg('<C-A>', 'i', false, true)
                                  ^

We couldn't write that:

const map_save = maparg('<C-A>', '!', false, true)
                                  ^
                                  ✘

Could maparg() support !, nox, ... ?
IOW, could this snippet be made to work like the previous one:

vim9script
noremap! <C-A> <C-B>
echo 'BEFORE restoration'
noremap! <C-A>
const map_save = maparg('<C-A>', '!', false, true)
unmap! <C-A>
mapset(map_save)
echo 'AFTER restoration'
noremap! <C-A>

Expected:

BEFORE restoration
!  <C-A>       * <C-B>
AFTER restoration
!  <C-A>       * <C-B>

Actual:

E119: Not enough arguments for function: mapset

If that's not possible, no worry; the PR is already very helpful; thank you very much for working on this.

could this snippet be made to work like the previous one

To get ! to work, what are the semantics. currently, from what you're saying, it returns the mapping that is a superset of the specified mode. kind of return mapping that contains mode. With this in mind, if there are two maps, i, c, and you ask for !, an enhanced maparg() should return empty dict {}; maparg() doesn't return a list.

It seems you need to know exactly how it is mapped. In which case, using i instead of ! doesn't really cost anything; except that it it doesn't feel good, or look good. However, I haven't looked at maparg (been purposely avoiding it), but if it's easy to do, I'm all for it while I still remember what I did.

Seems like doing

save_maps = maplist()->filter(ModeFilter(lhs, mode_strig))

might be the way to go. A generic or specialized ModeFilter could probably be built. Or just spell out a filter lamda. Either way, this seems to be a more robust solution; should pick up any mappings that are used for insert or command.

Out of curiosity, what's the vim9 way to make a ModeFilter(lhs, mode_string)? A function that returns a function that takes two args, index and dict, and remembers lhs and fmode_string.

I've thought a little more about this, and I don't think there is an issue anymore.

My use case was this:

1. I need to temporarily override a mapping
2. I save the mapping (with maparg())
3. I override the mapping
4. when I don't it anymore, I restore the mapping

But I've just realized that I never want to override a mapping in a set of modes (!, nox, ...). I always want to override a mapping in a single mode (n, x, i, c, t). So the current situation (once the PR is merged) is actually perfect. I save in a single mode, and if for some reason the key is mapped in a superset of modes, the new enhanced mapset() will correctly restore the mapping in all of the original modes.

Out of curiosity, what's the vim9 way to make a ModeFilter(lhs, mode_string)? A function that returns a function that takes two args, index and dict, and remembers lhs and fmode_string.

In Vim9 script (I don't know C)? If so, could you give some pseudo-code showing an example of what you want?

I don't think there is an issue anymore

Good to here

pseudo-code showing an example of what you want

Right, in vim9 script. Say I want to implement a ModeFilter function that is used like in the following

save_maps = maplist()->filter(ModeFilter('^K$', '! '))

to return all the mappings with 'K' as the left hand side, and that are used in all modes except terminal and lang.
I'd need something like

def ModeFilter(lhs_pattern: string, mode_string: string): func

that returns function that can be used with filter.

Would something like this work? Guess the question is whether lhs_pattern and mode_string are captured in the lamda that gets returned.

def ModeFilter(lhs_pattern: string, mode_string: string): func
    return (_, d:) => {
        return matches(d.lhs, lhs_pattern)
            && IsContainedIn(d.mode, mode_string)
    }
enddef

IsContainedIn() needs to be figured out; it probably uses a dict like

const mode_expander = {
    ' ': ['n', 'x', 's', 'o'],
    '!': ['i', 'c'],
    'v': ['x', 's']
    }

Hmm, I'm a little unsure of how to set up dict keys of ' ' and '!' as well.

Guess the question is whether lhs_pattern and mode_string are captured in the lamda that gets returned.

It should work, because the lambda is then a closure:

vim9script
def Func(s: string): func
    return () => s
enddef
var Ref = Func('captured')
echo Ref()

captured

Untested, but I would try something like this:

vim9script

def ModeFilter(lhs_pattern: string, mode_string: string): func
    return (_, d) => {
        return d.lhs =~ lhs_pattern
        && IsContainedIn(d.mode, mode_string)
    }
enddef

const mode_expander = {
    '':  ['n', 'x', 's', 'o'],
    ' ': ['n', 'x', 's', 'o'],
    '!': ['i', 'c'],
    'v': ['x', 's']
    }

def IsContainedIn(mode: string, modes: string): bool
    for m in modes
        if mode =~ get(mode_expander, m, m)->join('\|')
            return true
        endif
    endfor
    return false
enddef

Without looking at the code - it should work to use maplist() to find a mapping (or a few of them), store the list of dicts somewhere, do some random stuff, then use mapset() with those dicts to restore the mappings as they were. No matter what modes the dict contains.

I think this is ready to go, if questions raised in #10295 (comment) are not an issue.

I think this is ready to go, if questions raised in #10295 (comment) are not an issue.

This didn't get through to the list, github was taking a break this morning when it was posted.

Are there any issues with this patch?

I guess we can now simplify the restoration of the K mapping in the termdebug plugin:

diff --git a/runtime/pack/dist/opt/termdebug/plugin/termdebug.vim b/runtime/pack/dist/opt/termdebug/plugin/termdebug.vim
index 63d25db52..3d518a09e 100644
--- a/runtime/pack/dist/opt/termdebug/plugin/termdebug.vim
+++ b/runtime/pack/dist/opt/termdebug/plugin/termdebug.vim
@@ -915,7 +915,7 @@ func s:DeleteCommands()
     if empty(s:k_map_saved)
       nunmap K
     else
-      call mapset('n', 0, s:k_map_saved)
+      call mapset(s:k_map_saved)
     endif
     unlet s:k_map_saved
   endif

There might be other pitfalls like the fact that maparg() translates inconsistently (it does when {dict} is false, but not when it's true).

Hello, with this patch, are now <SID>s consistently translated when restoring a map?

There might be other pitfalls like the fact that maparg() translates inconsistently (it does when {dict} is false, but not when it's true).

Hello, with this patch, are now <SID>s consistently translated when restoring a map?

You could try it and open an issue if there's a problem.