errael

unread,

Apr 24, 2022, 12:12:56 PM4/24/22

to vim/vim, Subscribed

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

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

Commit Summary

6946314 Add getmaps() builtin

File Changes

(5 files)

Patch Links:

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

errael

unread,

Apr 24, 2022, 12:53:32 PM4/24/22

to vim/vim, Push

@errael pushed 1 commit.

e1918ae Add getmaps() builtin

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

codecov[bot]

unread,

Apr 24, 2022, 1:01:31 PM4/24/22

to vim/vim, Subscribed

Codecov Report

Merging #10273 (e1918ae) into master (ac92ab7) will increase coverage by 1.44%.
The diff coverage is 88.00%.

@@            Coverage Diff             @@

##           master   #10273      +/-   ##

==========================================

+ Coverage   81.00%   82.44%   +1.44%     

==========================================

  Files         161      148      -13     

  Lines      185553   171660   -13893     

  Branches    41946    38812    -3134     

==========================================

- Hits       150301   141533    -8768     

+ Misses      22742    17516    -5226     

- Partials    12510    12611     +101

Flag	Coverage Δ
huge-clang-none	`82.44% <88.00%> (+<0.01%)`	⬆️
linux	`82.44% <88.00%> (+<0.01%)`	⬆️
mingw-x64-HUGE	`?`
mingw-x64-HUGE-gui	`?`
windows	`?`

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

Impacted Files	Coverage Δ
src/evalfunc.c	`89.44% <ø> (-1.19%)`	⬇️
src/map.c	`86.73% <88.00%> (-1.69%)`	⬇️
src/highlight.c	`78.57% <0.00%> (-2.67%)`	⬇️
src/time.c	`86.93% <0.00%> (-2.38%)`	⬇️
src/buffer.c	`84.08% <0.00%> (-2.36%)`	⬇️
src/misc2.c	`86.55% <0.00%> (-2.34%)`	⬇️
src/help.c	`79.89% <0.00%> (-2.20%)`	⬇️
src/libvterm/src/screen.c	`51.96% <0.00%> (-2.04%)`	⬇️
src/session.c	`62.83% <0.00%> (-2.01%)`	⬇️
src/menu.c	`81.02% <0.00%> (-1.87%)`	⬇️
... and 133 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 ac92ab7...e1918ae. Read the comment docs.

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

Yegappan Lakshmanan

unread,

Apr 24, 2022, 1:26:10 PM4/24/22

to vim/vim, Subscribed

@yegappan commented on this pull request.

In runtime/doc/builtin.txt:

> @@ -3570,6 +3571,17 @@ getloclist({nr} [, {what}])				*getloclist()*
 			:echo getloclist(5, {'filewinid': 0})
 
 
+getmaps()						*getmaps()*

All the map related functions start with the map prefix. To be consistent, this function should
be called mapget().

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

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

Yegappan Lakshmanan

unread,

Apr 24, 2022, 1:27:30 PM4/24/22

to vim/vim, Subscribed

@yegappan commented on this pull request.

In runtime/doc/builtin.txt:

> @@ -3570,6 +3571,17 @@ getloclist({nr} [, {what}])				*getloclist()*
 			:echo getloclist(5, {'filewinid': 0})
 
 
+getmaps()						*getmaps()*
+		Returns a |List| of all mappings. Each List item is a |Dict|
+		as defined by |maparg()|. For a given mapping, the Dict from
+		getmaps() is identical to the dict from |maparg()|.

A user can use the Dict returned by getmaps() and call mapset() to recreate the mapping?

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

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

Bram Moolenaar

unread,

Apr 24, 2022, 1:37:52 PM4/24/22

to vim/vim, Subscribed

"maps" isn't commonly used, let's call this "getmappings".

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

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

Bram Moolenaar

unread,

Apr 24, 2022, 1:41:13 PM4/24/22

to vim/vim, Subscribed

Closed #10273 via 659c240.

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

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

Bram Moolenaar

unread,

Apr 24, 2022, 1:44:59 PM4/24/22

to vim/vim, Subscribed

@brammool commented on this pull request.

In runtime/doc/builtin.txt:

> @@ -3570,6 +3571,17 @@ getloclist({nr} [, {what}])				*getloclist()*
 			:echo getloclist(5, {'filewinid': 0})
 
 
+getmaps()						*getmaps()*

Except hasmapto(). It would be nice to start with "map", but what to call it then? mapgetall() ? maplist()?
mapget() suggests getting one mapping.
I used getmappings() for now.

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

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

Bram Moolenaar

unread,

Apr 24, 2022, 1:48:59 PM4/24/22

to vim/vim, Subscribed

@brammool commented on this pull request.

In runtime/doc/builtin.txt:

> @@ -3570,6 +3571,17 @@ getloclist({nr} [, {what}])				*getloclist()*
 			:echo getloclist(5, {'filewinid': 0})
 
 
+getmaps()						*getmaps()*
+		Returns a |List| of all mappings. Each List item is a |Dict|
+		as defined by |maparg()|. For a given mapping, the Dict from
+		getmaps() is identical to the dict from |maparg()|.

I would think the {mode} argument to mapset() could be a special value and then the "mode" from the dict is uesd.
The new function should also have an "abbr" optional argument to get a list of abbreviations instead of mappings.

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

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

LemonBoy

unread,

Apr 24, 2022, 1:55:27 PM4/24/22

to vim/vim, Subscribed

@LemonBoy commented on this pull request.

In runtime/doc/builtin.txt:

> @@ -3570,6 +3571,17 @@ getloclist({nr} [, {what}])				*getloclist()*
 			:echo getloclist(5, {'filewinid': 0})
 
 
+getmaps()						*getmaps()*

Given the presence of getloclist I'd say getmaplist is the name that fits best the existing functions.

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

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

Yegappan Lakshmanan

unread,

Apr 24, 2022, 1:59:30 PM4/24/22

to vim_dev, reply+ACY5DGFUY5AP6H6DGJ...@reply.github.com, vim/vim, Subscribed

Hi Bram,

On Sun, Apr 24, 2022 at 10:44 AM Bram Moolenaar <vim-dev...@256bit.org> wrote:

@brammool commented on this pull request.

In runtime/doc/builtin.txt:
> @@ -3570,6 +3571,17 @@ getloclist({nr} [, {what}])				*getloclist()*
 			:echo getloclist(5, {'filewinid': 0})
 
 
+getmaps()						*getmaps()*
Except hasmapto(). It would be nice to start with "map", but what to call it then? mapgetall() ? maplist()?
mapget() suggests getting one mapping.
I used getmappings() for now.

I think we should call this function maplist() or map_getlist(). There is some precedence for

this with existing functions: digraph_getlist(), prop_type_list(), popup_list(), term_list()

and tabpagebuflist().

On the other hand, we also have the following function names: getchangelist(), getjumplist(),

getloclist(), getmarklist() and getqflist().

Regards,

Yegappan

vim-dev ML

unread,

Apr 24, 2022, 1:59:48 PM4/24/22

to vim/vim, vim-dev ML, Your activity

Hi Bram,

On Sun, Apr 24, 2022 at 10:44 AM Bram Moolenaar ***@***.***>
wrote:

> ***@***.**** commented on this pull request.
> ------------------------------
>
> In runtime/doc/builtin.txt
> <https://github.com/vim/vim/pull/10273#discussion_r857158269>:

>
> > @@ -3570,6 +3571,17 @@ getloclist({nr} [, {what}]) *getloclist()*
> :echo getloclist(5, {'filewinid': 0})
>
>
> +getmaps() *getmaps()*
>
> Except hasmapto(). It would be nice to start with "map", but what to call
> it then? mapgetall() ? maplist()?
> mapget() suggests getting one mapping.
> I used getmappings() for now.
>
>
> I think we should call this function maplist() or map_getlist(). There is
some precedence for
this with existing functions: digraph_getlist(), prop_type_list(),
popup_list(), term_list()
and tabpagebuflist().

On the other hand, we also have the following function
names: getchangelist(), getjumplist(),
getloclist(), getmarklist() and getqflist().

Regards,
Yegappan

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

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

errael

unread,

Apr 24, 2022, 2:05:48 PM4/24/22

to vim/vim, vim-dev ML, Comment

@errael commented on this pull request.

In runtime/doc/builtin.txt:

> @@ -3570,6 +3571,17 @@ getloclist({nr} [, {what}])				*getloclist()*
 			:echo getloclist(5, {'filewinid': 0})
 
 
+getmaps()						*getmaps()*
+		Returns a |List| of all mappings. Each List item is a |Dict|
+		as defined by |maparg()|. For a given mapping, the Dict from
+		getmaps() is identical to the dict from |maparg()|.

The test does assert_equal(getmaps_entry, maparg_return) for all mappings; how could it not work? (Famous last words)

have an "abbr" optional argument

And here we go... :-)

So getmappings([{abbr}]), if abbr true, then return the abbreviations.

It could be getmappings({dict}), where the key abbr can be true/false. Just in case there's more options on the way. With that in mind, I'm in favor of just an optional bool for abbreviations.

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

You are receiving this because you commented.

errael

unread,

Apr 24, 2022, 2:09:46 PM4/24/22

to vim/vim, vim-dev ML, Comment

I'll sit on this for a day, planning on

possible name change
add optional abbr bool

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

You are receiving this because you commented.

errael

unread,

Apr 25, 2022, 1:31:46 PM4/25/22

to vim/vim, vim-dev ML, Comment

@errael commented on this pull request.

In runtime/doc/builtin.txt:

> @@ -3570,6 +3571,17 @@ getloclist({nr} [, {what}])				*getloclist()*
 			:echo getloclist(5, {'filewinid': 0})
 
 
+getmaps()						*getmaps()*
+		Returns a |List| of all mappings. Each List item is a |Dict|
+		as defined by |maparg()|. For a given mapping, the Dict from
+		getmaps() is identical to the dict from |maparg()|.

I would think the {mode} argument to mapset() could be a special value and then the "mode" from the dict is uesd.

I would suggest mapset({dict}) means take abbr and mode from dict, that's easy for a user to understand.

It's unclear to me, whether or not mapcheck() would want a similar change.

Either way, is there a reason not to include "abbr: true/false" in the mapping-dict? I think it could be done right now, without any negative consequences. I'm guessing mapset() wouldn't care today.

I'm a casual vim user, so I'm mostly clueless about a lot of vim stuff. Considering the above, and playing with the new maplist() function, I did the following experiment because I was trying to figure out where nox came from for the Q mapping. I'm sure whatever drives having [on]unmap also implicitly do sunmap gives a better user experience.

map X xyzzy     [' ']
nunmap X        ['ov']
map X xyzzy     [' ']
xunmap X        ['nos']
map X xyzzy     [' ']
sunmap X        ['nox']
map X xyzzy     [' ']
ounmap X        ['nv']
map X xyzzy     [' ']
sunmap X        ['nox']
smap X xyzzy    ['s', 'nox']

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

You are receiving this because you commented.

errael

unread,

Apr 25, 2022, 1:35:24 PM4/25/22

to vim/vim, vim-dev ML, Comment

@errael commented on this pull request.

In runtime/doc/builtin.txt:

> @@ -3570,6 +3571,17 @@ getloclist({nr} [, {what}])				*getloclist()*
 			:echo getloclist(5, {'filewinid': 0})
 
 
+getmaps()						*getmaps()*
+		Returns a |List| of all mappings. Each List item is a |Dict|
+		as defined by |maparg()|. For a given mapping, the Dict from
+		getmaps() is identical to the dict from |maparg()|.

If overloading functions is philosophically undesirable, mapset(null, null, dict) is a possibility.

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

You are receiving this because you commented.

Bram Moolenaar

unread,

Apr 25, 2022, 2:30:21 PM4/25/22

to vim...@googlegroups.com, errael

> @errael commented on this pull request.
>

> > @@ -3570,6 +3571,17 @@ getloclist({nr} [, {what}]) *getloclist()*
> :echo getloclist(5, {'filewinid': 0})
>
>
> +getmaps() *getmaps()*
> + Returns a |List| of all mappings. Each List item is a |Dict|
> + as defined by |maparg()|. For a given mapping, the Dict from
> + getmaps() is identical to the dict from |maparg()|.
>
> > I would think the {mode} argument to mapset() could be a special
> > value and then the "mode" from the dict is uesd.
>
> I would suggest `mapset({dict})` means take abbr and mode from dict,
> that's easy for a user to understand.

> It's unclear to me, whether or not `mapcheck()` would want a similar change.
>
> Either way, is there a reason not to include "abbr: true/false" in the
> mapping-dict? I think it could be done right now, without any negative
> consequences. I'm guessing `mapset()` wouldn't care today.

Yes, that should work nicely. Then maplist() or maparg() can be used to
get an existing mapping, and restore it later. And it would also work
for an abbreviation, without having to specify that separately.

> I'm a casual vim user, so I'm mostly clueless about a lot of vim
> stuff. Considering the above, and playing with the new `maplist()`
> function, I did the following experiment because I was trying to
> figure out where `nox` came from for the `Q` mapping. I'm sure
> whatever drives having `[on]unmap` also implicitly do `sunmap` gives a
> better user experience.
> ```
> map X xyzzy [' ']
> nunmap X ['ov']
> map X xyzzy [' ']
> xunmap X ['nos']
> map X xyzzy [' ']
> sunmap X ['nox']
> map X xyzzy [' ']
> ounmap X ['nv']
> map X xyzzy [' ']
> sunmap X ['nox']
> smap X xyzzy ['s', 'nox']

> ```

There is a lot of history. Starting with the original Vi only having
"map" and "map!". Visual and Select modes were added later, so mappings
for different modes were made possible.

--
Q: What is a patch 22?
A: A patch you need to include to make it possible to include patches.

/// Bram Moolenaar -- Br...@Moolenaar.net -- http://www.Moolenaar.net \\\
/// \\\
\\\ sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///

Bram Moolenaar

unread,

Apr 25, 2022, 2:30:21 PM4/25/22

to vim...@googlegroups.com, errael

> @errael commented on this pull request.
>

> > @@ -3570,6 +3571,17 @@ getloclist({nr} [, {what}]) *getloclist()*
> :echo getloclist(5, {'filewinid': 0})
>
>
> +getmaps() *getmaps()*
> + Returns a |List| of all mappings. Each List item is a |Dict|
> + as defined by |maparg()|. For a given mapping, the Dict from
> + getmaps() is identical to the dict from |maparg()|.
>
> If overloading functions is philosophically undesirable, `mapset(null,
> null, dict)` is a possibility.

Using different types for the same argument might be unusual, but we
already have several functions do that.

What if one uses "mapset(null, true, dict)", would the abbreviation flag
in "dict" overrule the {abbr} argument? For backwards compatibility we
should. For the first argument {mode} this is already documented.

I think using "mapset(dict)" is the simplest, and it might very well
become the most common use.

--
"I can't complain, but sometimes I still do." (Joe Walsh)

Reply all

Reply to author

Forward

[vim/vim] Add getmaps() builtin (PR #10273)

errael

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

Commit Summary

File Changes

Patch Links:

errael

codecov[bot]

Codecov Report

Yegappan Lakshmanan

Yegappan Lakshmanan

Bram Moolenaar

Bram Moolenaar

Bram Moolenaar

Bram Moolenaar

LemonBoy

Yegappan Lakshmanan

vim-dev ML

errael

errael

errael

errael

Bram Moolenaar

Bram Moolenaar