feat(pick): make key query process respect the 'iminsert' option

[yehorb]

The 'getcharstr' returns a character from the input as is, and it is not affected by the 'iminsert' option.

It is possible, however, to construct a lookup table for ':lmap' mappings and apply it to the returned character on the fly.

This successfully emulates the 'iminsert' option in the 'mini.pick' input field.

However, the 'iminsert' toggle has to be defined in 'mini.pick' configuration as a keybinding, as the standard '<C-^>' has no effect by default.

The lookup table is created only once and then cached. In my non-scientific testing, thie feature has no impact on input latency.

The feature has no effect if the 'keymap' option is not set.

I understand that 'iminsert' is a relatively niche feature to cover, and it was never mentioned in issues or discussions. While you can always change the keyboard layout, and achieve the desired effect, it feels unnatural to me. I write quite a lot in the 'iminsert' mode, and find it easier to invoke than the keyboard layout change.

I thought to raise the issue beforehand, but the implementation is quite trivial, so I decided to raise the PR outright.

I intentionally did not update the documentation or anything. If you want to move forward with the feature, I am ready and willing to follow your lead and bring the change up to standard.

• I have read CONTRIBUTING.md
• I have read CODE_OF_CONDUCT.md

[echasnovski]

Thanks for the PR!

I have several issues with supporting this kind of functionality. As I've never used the 'iminsert' option and language mappings, could you please clarify if I understand correctly?

1. From usability side, does it indeed require having a dedicated mapping for toggling 'iminsert' value? Or is it reasonably okay to suggest having it manually enabled/disabled before calling picker?

   If having a dedicated 'mini.pick' mapping is crucial for comfortable usability, then it has to be built-in. Which is a not trivial amount of code/docs/tests.

2. Caching all language mappings once is not the best approach here. Because mappings can be created after the first 'mini.pick' call. I think the best approach here is to compute it once per picker invocation. The actual computation can depend on the 'keymap' and 'iminsert' option values (for performance).

3. vim.iter is not present on all versions that 'mini.nvim' aims to support, as it was introduced in Neovim=0.10. Besides, the pure Lua code is probably easier to understand and more performant in this particular case.

4. While you can always change the keyboard layout, and achieve the desired effect, it feels unnatural to me. I write quite a lot in the 'iminsert' mode, and find it easier to invoke than the keyboard layout change.

   This is probably the most important aspect here. What is the advantage of not preferring to switch the keyboard layout? It already works without any extra code in 'mini.pick' or a dedicated <C-^> mapping.

I thought to raise the issue beforehand, but the implementation is quite trivial, so I decided to raise the PR outright.

Please, always first create an issue for a new feature request (after searching that this was not already suggested earlier). This usually saves time for both parties involved.

[yehorb]

First of all, I am sorry for rushing this PR. I was upset by the absence of the 'iminsert' handling and very elated after adding it. You are right, I should have started with an issue.

Would you like me to close the PR and create an issue for a new feature request first?

Now, I will try to make the best of the bad situation. The PR explicitly shows what my suggestion is. Let me try to be more rational, address why I think it is important, and answer your questions. And in the end I hope you can help me understand how we can move forward with implementation.

I am sorry in advance for the wall of text below, and I am thankful for your time to review it.

So, what is 'iminsert' and why is it important? Well, the 'iminsert' option is a part of the broader 'keymap' option. To think of it, the better name for the PR would be: allow input with a keymap in a picker window

Essentially, the 'keymap' option is one of the ways Neovim supports multiple input languages. You have a 'langmap' option set in your Neovim setup, so I will make the comparison between 'langmap' and 'keymap'.

The 'langmap' allows executing Normal commands while keeping the non-English keyboard mode. ролд get translated to hjkl, and you can navigate a buffer just fine while keeping the non-English keyboard mode.

Well, 'keymap' is the opposite to 'langmap'. It even says so in the manual (albeit it says that 'langmap' is the opposite of 'keymap'):

This is the opposite of the 'keymap' option, where characters are mapped in Insert mode.

- help 'langmap'

When 'keymap=ukrainian-jcuken' is set, and 'iminsert=1', typing ghbdsn will translate to привіт. This translation works in multiple typing modes:

":lmap" defines a mapping that applies to:

• Insert mode
• Command-line mode
• when entering a search pattern
• the argument of the commands that accept a text character, such as "r" and "f"
• for the input() line

- help language-mapping

It allows the user to input non-English characters while keeping the English keyboard mode.

This (typing characters with a 'keymap' set and 'iminsert=1') will translate one or more (English) characters to another (non-English) character.

- help mbyte-keymap

The 'iminsert' option specifically controls whether or not such mappings are applied. If 'keymap' is not set, it essentially does nothing.

'CTRL-^' toggles the 'iminsert' option:

Toggle the use of typing language characters.

When language |:lmap| mappings are defined:

• If 'iminsert' is 1 (langmap mappings used) it becomes 0 (no langmap mappings used).
• If 'iminsert' has another value it becomes 1, thus langmap mappings are enabled.

- help i_CTRL-^

'CTRL-^' is not a user-defined mapping, but a standard control character, like 'hjkl' are.

I hope I was able to explain the mechanics of the 'keymap' function clearly enough. If you want, you can easily try it out:

• set keymap=ukrainian-jcuken
  • This will also set 'iminsert=1' by default
• Enter Insert mode
  • You should see -- INSERT (uk) -- as the current mode
  • Typing will produce Ukrainian characters
• Now you can use 'CTRL-^' to toggle between -- INSERT (uk) -- and -- INSERT -- (English) modes
• Neovim ships with quite a few 'keymaps' predefined, so it is possible to try out multiple 'keymap' values

Now to address your questions:

From usability side, does it indeed require having a dedicated mapping for toggling 'iminsert' value?

I would advocate for having a dedicated mapping for toggling 'iminsert' value in 'mini.pick' and setting it to specifically to '<C-^>'. However, as this is only for 'keymap' emulation, it does not need to toggle the 'iminsert' specifically. The 'iminsert' has no effect on getcharstr() anyway, so any other arbitrary flag can be used. I used the 'iminsert' in my example just because it is a standard.

The best approach here is to compute it once per picker invocation

I do agree that mappings can be created after the first 'mini.pick' call, and that caching them globally may be suboptimal. However, the "l"-type mapping are created using the ":lmap" command, and one can reasonably expect that ":lmap" commands will only be invoked when loading a 'keymap'.

The simplest way to load a set of related language mappings is by using the 'keymap' option. See |45.5|.

- help language-mapping

So the "optimal" cache invalidation strategy may be to invalidate the cache on "OptionSet" autocommand with the "keymap" pattern. But, realistically, any strategy may be adopted, as lookup table creation is quite cheap.

The pure Lua code is probably easier to understand and more performant

Sure, I will rewrite the lookup table creation in imperative style. The performance difference is negligible, though. But technically yes, imperative is better, having smaller mean and variance.

1000 executions	Mean	Variance
Imperative	0.003362400000000006	5.611514074074387e-07
vim.iter	0.0034688370000000067	8.711525790099662e-07

What is the advantage of not preferring to switch the keyboard layout?

Regarding the advantage of using the 'keymap' option.

I use the 'keymap' option all the time. I do a lot of note taking, and I prefer my notes to be in my native tongue. However, almost every one of my notes contains English characters - code blocks, formulas, etc.

The wonderful thing about 'keymap' is the fact that it is scriptable. For example, I have the following mapping defined: inoremap $ <Cmd>set iminsert=0<CR>$ for Markdown and LaTeX, as 99.99% of the time the '$' will be followed by a LaTeX formula. It makes bilingual work in Neovim effortless and intuitive.

So I prefer 'keymap' option to switching the keyboard layout. It allows to achieve what essentially is an automatic layout switching, fully self-contained in my Neovim setup.

In standard Neovim, 'keymap' option works transparently and everywhere. I am very used to being able to type non-English characters in every Insert mode buffer after pressing 'CTRL-^'.

So when I first started using 'mini.pick', I was quite disappointed to discover that it does not work. At first I thought this must be a bug! Other pickers implemented in Lua - specifically "Telescope" and "Snacks.picker" - do create the standard Insert mode buffer, and the 'keymap' option has the expected effect while typing there.

Later I learned about the custom key query process, and tried to emulate the 'keymap' effect, and that is where we are now.

But it will be fair to admit that it is up to preference. "What is the advantage of using Neovim over VSCode?" - the answer will be different for everyone.

So why do I think it is a good feature to add?

The advantage is making a picker buffer behave more like a standard Insert mode buffer. While it is powered by a custom key query process, it is reasonable to assume that it is an Insert mode buffer. I see value in making the behavior more in line with the standard set by the core Neovim feature.

[echasnovski]

Thanks for such a detailed explanation (especially since it looks like not written by AI :) ).

I'll be perfectly honest here. To me personally right now using built-in Neovim 'keymap' instead of OS's layout change does look like a matter of taste. And since the latter is more ubiquitous, more flexible (allows more naturally switching between more than two layouts), and already works, I'd close this as not planned. Mostly due to concerns of adding complexity and lines to already the biggest and most complex 'mini.nvim' module.

But as you are a fellow Ukrainian and seem to be very passionate about this, I'd be willing to compromise here. Adding this with close to zero performance penalty for the common case of no 'keymap' with as few lines as possible should be doable. For that:

• No built-in mapping for <C-^>. Let users add it themselves and design implementation for it to work.
• No global one-time caching or dependency on 'keymap' for caching because language mappings is a more general idea. They can be created/updated after the first getcharstr() call and can be used outside of 'keymap' option.

In particular:

• Make an H.get_lmap() helper that returns language map without caching.
• Update H.getcharstr() to take a language map as a second table argument lmap and always try to use it first (basically return lmap[char] or char).
• Update H.picker_advance() to cache language mapping before all iterations. This compromises in favor of performance over allowing to change language mappings within a single picker session.
• Use vim.o.iminsert == 0 and {} or H.get_lmap() for H.getcharstr() when advancing picker. This adds a single vim.o.iminsert check for every input character in a common case, but this should be negligible.
• Use {} when getting character for paste action (which is how it work in Insert mode, I believe).

If this okay with you, we can go one of two routes:

1. I finish this myself. It will need implementation, tests to cover all mentioned use cases (like basic use case with 'keymap' and manual language mapping, changing 'iminsert' within a picker session, working in paste action) and changelog update. I'll credit you as a co-author in a commit.
2. We'll work together to finish this up (you make code and test updates) until the state where maybe only nit picks remain. You get credit as a commit author and in the changelog.

Both options are completely fine by me. The 1 will require less work for both of us, the 2 will get you full credit and me - another person who has experience with 'mini.nvim' development.

[yehorb]

it looks like not written by AI :)

I would not dare pushing AI content in this situation. Nevertheless, thank you for acknowledging my writing skills :)

Reading through your feedback and learning more about your values, it seems that you are also very passionate about 'mini.nvim', keeping it as minimal as possible, and adhering to things that already work. And it seems that you are quite reluctant to adopt the feature in a way that I presented it. It's okay, and I respect that.

I checked the issues and discussions once again, and it seems that there are very few issues and discussions regarding natural language and keyboard layouts. And it is fare to say that most (all?) of the 'mini.pick' uses do not really care about the 'keymap' option. So by moving forward even if the performance penalty for the common case is close to zero we would solve my very specific issue.

However, as you correctly noted, I would very much like for this improvement to re made. However, I do not want the solution to be forced. And, I think, there might be yet another way, that is less of a compromise and more of an actual improvement/feature.

The root cause of my problem is the fact that 'mini.pick' relies on custom key query process, and this process does not follow the Insert mode rules. Instead, the query is essentially built character by character.

My initial proposal hooked into this process in the most straightforward way possible - I intercepted the character and changed it, by injecting use case specific code into a general flow. And while we can formalize it slightly according to your suggestion, the core issue with my proposal remains the same.

How about we lean into formalizing the character interception instead? In the form of a callback that will fire after a character is retrieved and before it is added to a query. This approach is more general, can be used for more than one thing and is easier to test.

Future maintainer will not need to ask themselves "What are these 'keymap' and 'iminsert' options, and why are they here?" The 'keymap' use case can be added to the docs as a new example. And I will be able to solve my issue in my config, not in the 'mini.pick' codebase - by implementing a proper callback.

There are multiple possible places to place such a callback - in H.getcharstr itself (current) example, in the loop in H.picker_advance, in H.picker_query_add. I like the idea of H.picker_advance and H.picker_query_add as both char and picker are in context, and a callback can look like H.get_config().callbacks.on_char(picker, callback). Having picker available allows for caching state in picker itself, and aligns well with your suggestion to compute lmap once per picker invocation.

In the minimal case, it will require 1 new config option, and 1 conditional function call. No built-in mappings.

If you like the idea - I can prototype the implementation and we will move on from there.

Regarding the implementation - I 100% prefer the route where we work on this together. I want to learn more about the Neovim plugin development lifecycle, specifically setting up testing environment and adding tests. This looks like the great opportunity to do so. And I don't want to miss the opportunity to work on one of the most popular Neovim plugins, with one of the most popular Neovim plugin authors :)

[echasnovski]

How about we lean into formalizing the character interception instead? In the form of a callback that will fire after a character is retrieved and before it is added to a query. This approach is more general, can be used for more than one thing and is easier to test.

I don't think I like this idea. It is more general, but it is also relatively complicated to actually use (no "out of the box value"). While I'd imagine it is pretty niche, especially since there are already a mechanism of custom mappings.

Future maintainer will not need to ask themselves "What are these 'keymap' and 'iminsert' options, and why are they here?" The 'keymap' use case can be added to the docs as a new example. And I will be able to solve my issue in my config, not in the 'mini.pick' codebase - by implementing a proper callback.

Knowing about Neovim's options and capabilities is a good thing for a Neovim plugin maintainer to have. Even/especially if it is not widely used.

Regarding the implementation - I 100% prefer the route where we work on this together. I want to learn more about the Neovim plugin development lifecycle, specifically setting up testing environment and adding tests. This looks like the great opportunity to do so.

Sounds good. But to be fair, I don't think working on 'mini.nvim' will extrapolate well on other plugins: not enough plugins have testing at all while coding practice/design can differ significantly.

Start with implementation outlined in this comment and try to look into how to write and run tests. Useful sources:

• 'tests/text_pick.lua' for 'mini.pick' tests (the file is quite big). Usually adjusting close enough test case or copy-pasting a similar looking case is enough. It should be enough here.
• TESTING.md with more in-depth details about how 'mini.test' tests work.
• I'd recommend (at least temporarily) create a dedicated mappings to run 'mini.test' tests (mentioned here). Running test at cursor saves a lot of time when writing a test. From CLI the file can be run with make test_pick.

[echasnovski]

Do not check 'keymap' option because language mappings can be present even if 'keymap' is not used.

[echasnovski]

The change looks okay.

Do you need help with writing and/or understanding tests?

Edit: Accidentally closed PR, sorry.

[yehorb]

Hi. I am working on tests, yes. I think I more or less got it. However, I stumbled upon the next layer of issues. While the feature as expected and as we discussed with simple keymaps - like the ukrainian-jcuken I mentioned before, - the behavior can be unexpected for more complex ones. Both lhs and rhs can be anything, as it is the same type of map command, just different type. There are built in keymaps, where all lhs are pairs of symbols. In the $VIMRUNTIME/keymap/accents.vim every entry is like this:

`A	À
'A	Á
´A	Á
^A	Â
~A	Ã
:A	Ä

The behavior is expectedly underwhelming - 'iminsert' does nothing, as every key in lmap is a string of 2 symbols, but we only ever index it using a single symbol.
There are keymaps for more complex alphabets. For example, the $VIMRUNTIME/keymap/arabic_utf-8.vim maps keys directly to unicode characters by code:

q	<char-0x0636>			" (1590)	- DAD
w	<char-0x0635>			" (1589)	- SAD
e	<char-0x062b>			" (1579)	- THEH
r	<char-0x0642>			" (1602)	- QAF
t	<char-0x0641>			" (1601)	- FEH
y	<char-0x063a>			" (1594)	- GHAIN

And in such a case the behavior was unexpected for me - with 'iminsert' enabled input was consumed, but there was no output. lmap is correctly populated, but rhs are multibyte characters and they get filtered out in H.picker_query_add.
There is also no rule that prevents rhs of the :lmap from being an expression or a function, and I have no idea how to handle that. No built in keymaps have an expression as rhs, but it is possible.
So I don't really know how to proceed. I don't think it is a good idea to proceed as planned - 'keymaps' can be more complex, that I initially anticipated, and without changes the behavior can be unexpected.
Implementing proper :map behavior handling, including functions is expressions is not really feasible or even possible.
Limiting the feature to only manually selected keymap names is not an option - it is brittle and 'keymap' file can have an arbitrary name.
Trying to cover cases "in-between" by introducing 2 or 3 characters buffer, or trying to work around edge cases will drastically increase complexity. And it will defeat the main benefit of the custom key query process, if I understand correctly.
The ideal way to make the behavior predictable would be to ask the user to provide lmap table by themselves - and check if it is a table of specifically single character to single character. But this runs into the issue you previously outlined - no "out of the box value".

[echasnovski]

And in such a case the behavior was unexpected for me - with 'iminsert' enabled input was consumed, but there was no output. lmap is correctly populated, but rhs are multibyte characters and they get filtered out in H.picker_query_add.

I don't quite follow here. If you are comfortable writing tests, please write it as a separate failing test (in a comment). If not - as a step for me to reproduce the issue.

And if reasonable changes are needed in H.picker_query_add(), then it can be looked into. But only if necessary.

There is also no rule that prevents rhs of the :lmap from being an expression or a function, and I have no idea how to handle that. No built in keymaps have an expression as rhs, but it is possible.

If that's the case, then I think it is reasonable to filter them out in H.get_lmap().

So I don't really know how to proceed. I don't think it is a good idea to proceed as planned - 'keymaps' can be more complex, that I initially anticipated, and without changes the behavior can be unexpected.

The planned approach of what is currently in the PR looks like a strict improvement already. It should make 'mini.pick' work in situations it currently can not while preserving to work where it works now. The fact that implementation doesn't cover all new use cases is a shame, but should not block strict improvements.

I'd add (concise!) comments about the limitations to the tests with possibly commented out example of where the test should ideally pass but it currently fails.

---

I'll also ask to not overly cover this functionality with tests. I.e. no need to test all possible and impossible situations. Start with the ones outlined at the end of this comment.

[echasnovski]

Please don't create separate set and files for such small content. All this can be a single test case under T['start'] (you can put it after 'allows overriding built-in mappings').

Probably something like this:

diff --git a/tests/test_pick.lua b/tests/test_pick.lua
index 54bf95b8..2e800a0d 100644
--- a/tests/test_pick.lua
+++ b/tests/test_pick.lua
@@ -893,6 +893,38 @@ T['start()']['allows overriding built-in mappings'] = function()
   eq(get_picker_state().caret, 2)
 end
 
+T['start()']['works with language mappings'] = function()
+  child.o.keymap = 'ukrainian-jcuken'
+  eq(child.o.iminsert, 1)
+
+  start_with_items({})
+  type_keys('g', 'h')
+  eq(get_picker_query(), { 'п', 'р' })
+  type_keys('<C-u>')
+
+  -- Should allow changing 'iminsert' while picker is active
+  child.o.iminsert = 0
+  type_keys('g', 'h')
+  eq(get_picker_query(), { 'g', 'h' })
+  type_keys('<C-c>')
+
+  -- Should work with custom "good" language mappings
+  child.o.keymap = ''
+  child.cmd('lmap a 1')
+  child.cmd('lmap b <char-0x1f171>')
+  child.cmd('lmap cc C')
+
+  start_with_items({})
+  type_keys('a', 'b', 'c', 'c')
+  eq(get_picker_query(), { '1', 'b', 'c', 'c' })
+  type_keys('<C-u>')
+
+  -- Should cache language mappings per picker session
+  child.cmd('lmap d 4')
+  type_keys('d')
+  eq(get_picker_query(), { 'd' })
+end
+
 T['start()']['respects `window.config`'] = function()
   -- As table
   start({ source = { items = { 'a', 'b', 'c' } }, window = { config = { border = 'double' } } })

[yehorb]

Sure, I can minify the test set. I was following the example of other test sets, but it makes sense to view this feature scope as "small" and include it in the "start" set. Should I create multiple test functions, or condense it all into a single test, as in your example?

[echasnovski]

The above test case is big enough already (and looks like covering all cases of language mappings, right?). If the tested behavior fits into "work with languge mappings", then add to it. If not - add new test case.

I think only testing of "paste" action is left, which is better done as a separate case in T['Paste'] set.

[echasnovski]

@yehorb, this takes a bit longer than initially expected. So I merged into a temporary branch to polish this up myself and later merge into main.

[echasnovski]

This is now part of the main branch with 14145d3 commit. Apart from mentioned changes to tests, I also did two small tweaks:

• Moved the vim.o.iminsert check directly into H.getcharstr() to have its change effect immediately. Previously it was computed before H.getcharstr() (during computing its arguments), so changing it directly had effect only on the next character.
• Account for the fact that maplist() Vimscript function became available only on Neovim>=0.10.

Otherwise, thank you for your time and efforts when making this! Lovely to see more Ukrainians using Neovim and contributing to its ecosystem :)

[yehorb]

Oh my, you are too kind. I just carved out some time to polish the feature later today and tomorrow. Now I am a bit embarrassed for leaving the feature unattended so you felt the need to step in. Oh well, looks like the perfect is the enemy of good after all.

Thank you for the kind words and the opportunity to contribute.

[echasnovski]

Now I am a bit embarrassed for leaving the feature unattended so you felt the need to step in. Oh well, looks like the perfect is the enemy of good after all.

Please, don't be. You've done more than most users 🙏

It's just that this was already nearly finished for almost two weeks and I don't like seeing PR number go up :)