Store maps in a table, and add description support

[Corey-Keller]

This adds a global table NestMapsTable that holds all nest mappings (providing a base that future integrations can build off of), and adds very minimal support for descriptions (they can be added as a 3rd field and it won't cause an error, but no code makes actual use of it yet).

It's a decent first step to closing #4 I think.

[LionC]

Hey Corey. Thanks for the PR. I will start to review it once 1.1 is done - I am working on that right now.

[Corey-Keller]

EDIT: this can be seen in my wk_min branch
I'm currently testing a super barebones which-key 'integration' in the form of require('which-key').register({ [mapConfig.lhs] = { mapConfig.description } }, { mode = sanitizedMode }) being included in the mode in string.gmatch(mergedPresets.mode, '.') for loop

[Corey-Keller]

I rebased my code off of (and changed the merge target to) master now that 1.1 is out, but I'll switch it to the 1.2 branch as soon as it's created.

[LionC]

@Corey-Keller I created a branch for 1.2 - lets rebase and PR targeting that branch for this feature. I will try to find some time this week to put some thought and feedback into it :-)

[Corey-Keller]

@LionC have you had a chance to look at/think about this yet? Not rushing/pushing you or anything (life always comes first), just wondering, in case it slipped your mind.

[LionC]

Looking at it now ;-)

[LionC]

So I looked at it and played around with it a bit. I took the liberty to modify the branch - I hope that is fine, as you marked it to be editable for maintainers.

Points behind the way I implemented it:

• We do not need to rewrite the option merging logic - as we already have mergdPresets, we can just save those
• The whole feature should probably be optional, so I extracted it into a separate module that is set up in a way that makes it very easy to load it only on some .setup {} call. We should include such a call for a releasable version of the feature
• That module should also (optionally) offer to register vim commands to open some UI to inspect the keymaps. I think this is also part of a minimal feature set to be releasable
• I am unsure about the API. The current version just has a name which does not cascade and needs to be referenced explicitly. I do not know if I like the anonymous third string argument style yet, nor if I want names to cascade as a name prefix to allow DRYing up repetitive names (although I am not too sure how often that actually happens)

[Corey-Keller]

@LionC I'm getting errors like:

packer.nvim: Error running config for neogen: vim/shared.lua:205: invalid "behavior": table: 0x7ffbc971cf40                                                                                                  
Error executing vim.schedule lua callback: vim/shared.lua:205: invalid "behavior": table: 0x7ffbc9b52258

Line 205 of $VIMRUNTIME/lua/vim/shared.lua is right before the vim.tbl_deep_extend function is defined, and I'm 99.9% sure it's due to the lack of a behavior (error/keep/force) in the vim.tbl_extend in inspect.saveMapping

[Corey-Keller]

@LionC I don't know about the 'name' cascading, since it's really just a descriptor for an individual mapping. I would however suggest cascading labels (like the "category" setting in nvim-mapper, but allowing for multiple, so that a mapping for some LSP functionality that takes a motion (like 3j) could have both the LSP and Motion labels).

I agree on not being sure about the API. You mention not being sure about using the 3rd anonymous argument, but I was just going along with the other 2 (which were also anonymous). The current method of 2 anonymous arguments, and 1 named seems really weird. Might be a good idea to make them all named explicitly.

[LionC]

The reason for the two anonymous arguments is that they are eht eonly required ones - the most minimal mapping is

{ 'S', 's<CR><Esc>' }

with no named optional fields.

As name / description (I think I prefer name) is optional, it would follow that.

[LionC]

@LionC I'm getting errors like:

packer.nvim: Error running config for neogen: vim/shared.lua:205: invalid "behavior": table: 0x7ffbc971cf40                                                                                                  
Error executing vim.schedule lua callback: vim/shared.lua:205: invalid "behavior": table: 0x7ffbc9b52258

Line 205 of $VIMRUNTIME/lua/vim/shared.lua is right before the vim.tbl_deep_extend function is defined, and I'm 99.9% sure it's due to the lack of a behavior (error/keep/force) in the vim.tbl_extend in inspect.saveMapping

I will take a look - but I thought I fixed that (had a version with that problem in between).

[LionC]

@LionC I'm getting errors like:

packer.nvim: Error running config for neogen: vim/shared.lua:205: invalid "behavior": table: 0x7ffbc971cf40                                                                                                  
Error executing vim.schedule lua callback: vim/shared.lua:205: invalid "behavior": table: 0x7ffbc9b52258

Line 205 of $VIMRUNTIME/lua/vim/shared.lua is right before the vim.tbl_deep_extend function is defined, and I'm 99.9% sure it's due to the lack of a behavior (error/keep/force) in the vim.tbl_extend in inspect.saveMapping

I will take a look - but I thought I fixed that (had a version with that problem in between).

Fixed - never code when tired ;-)

[LionC]

If you cannot find any bugs with this, I would be fine to merge it into v1.2 as a starting point for the other related features.

[Corey-Keller]

Works for me. Although I'd personally argue that f320a93 is a non-positive change (I wanted to say regression, but that wouldn't be technically accurate, would it?) since the field is a description of the mapping as opposed to a name, isn't it? name is less clear (to me personally) as to the exact purpose of the key.

[LionC]

I have no strong feelings about that - name makes clear that it is a short-ish string and both nam and description work for me, as they both are some human readable string trying to describe something shortly. Tbh, the main point is that name is shorter and I expect users that want to use it to type / copy it a lot.

[Corey-Keller]

That's true. If users are going to have to include it in each mapping, then shorter is definitely better.

[Corey-Keller]

I'm good with it being merged into 1.2 by the way. If you have any thoughts on design/scope/style etc. (so that I can do something more like 3c5d573 than 013d45c for example) please let me know.