Friday, May 26, 2017

Re: Bug/non-determinism in output of maparg() and map commands

On Fri, May 26, 2017 at 5:24 PM, Nikolay Aleksandrovich Pavlov
<zyx.vim@gmail.com> wrote:
> 2017-05-27 0:32 GMT+03:00 Brett Stahlman <brettstahlman@gmail.com>:
>> On Fri, May 26, 2017 at 4:12 PM, Nikolay Aleksandrovich Pavlov
>> <zyx.vim@gmail.com> wrote:
>>> 2017-05-26 20:43 GMT+03:00 Bram Moolenaar <Bram@moolenaar.net>:
>>>>
>>>> Brett Stahlman wrote:
>>>>
>>>>> >> On Tuesday, May 23, 2017 at 8:25:33 AM UTC-5, Brett Stahlman wrote:
>>>>> >> > On Tue, May 23, 2017 at 4:35 AM, Bram Moolenaar <Bram@moolenaar.net> wrote:
>>>>> >> > >
>>>>> >> > > Brett Stahlman wrote:
>>>>> >> > >
>>>>> >> %--snip--%
>>>>> >> > >
>>>>> >> > > The best solution is probably to also add the raw rhs, with the terminal
>>>>> >> > > codes replaced. This won't work when changing the terminal type, but
>>>>> >> > > that is very unlikely to happen.
>>>>> >> >
>>>>> >> > You mean adding a key such as "raw_rhs" to the dictionary returned by
>>>>> >> > maparg()? If so, then yes this would help, but there would still need to
>>>>> >> > be a way to determine lhs, which is currently even more ambiguous than
>>>>> >> > rhs. While it's true that I probably already have lhs if I'm calling
>>>>> >> > maparg(), I need a way to determine which lhs(s) is/are ambiguous with a
>>>>> >> > given lhs. Mapcheck() gives me only the rhs of the conflicting map. To
>>>>> >> > save and restore, I'd need to know the lhs in canonical form as well.
>>>>> >>
>>>>> >> Perhaps mapcheck() could take an optional arg requesting something more than a simple boolean return. When called with this extra arg, mapcheck() could return a conflicting/ambiguous lhs (or list thereof) in some canonical format (possibly determined by the value of the extra arg itself). As long as the format returned could be fed to maparg(), it would be possible to find conflicting mappings, remove them temporarily, and subsequently restore them...
>>>>> >
>>>>> > If you define a mapping you will want to know whether the mapping
>>>>> > already exists and needs to be restored. For that you can use maparg(),
>>>>> > no need to use mapcheck().
>>>>> >
>>>>> > Not sure why you would want to remove "conflicting" mappings. Perhaps
>>>>> > when you map the ; key, and the user has ;x mapped? Then you would need
>>>>> > a list. Adding a maplist() function would be better than adding
>>>>> > arguments to mapcheck().
>>>>>
>>>>> Yes. Very much like that. I'm implementing a sort of transient mode, in
>>>>> which I'll "shadow" existing maps with very short (generally single
>>>>> character) mappings, which are expected to be ambiguous/conflicting with
>>>>> existing maps, and even builtin operators. Of course, when I exit the
>>>>> transient mode, I'd need to restore the mappings that were shadowed.
>>>>>
>>>>> The global and builtin maps are not a problem, since the transient maps use
>>>>> <buffer> and <nowait>; however, without parsing the output of one of the :map
>>>>> functions, I have no way of knowing which buf-local mappings will be ambiguous
>>>>> with the transient maps I'm defining. And parsing the :map output is
>>>>> problematic for the reasons already mentioned: e.g., no way to tell the
>>>>> difference between function key <F8> and the corresponding 4 characters. I'd
>>>>> actually considered taking some sort of iterative approach: e.g., trying all
>>>>> possible permutations of lhs as input to maparg() and testing the results, in
>>>>> an attempt to deduce the canonical form, but this would be extremely messy,
>>>>> and I don't even know whether it would be deterministic... The maplist()
>>>>> function you mentioned, if it returned all ambiguous left hand sides in
>>>>> canonical form, or even a list of the corresponding maparg()-style
>>>>> dictionaries, would be perfect. Of course, there would also need to be a way
>>>>> to get the rhs's canonical form: e.g., the extra "raw_rhs" key in the maparg()
>>>>> or maplist() dictionary.
>>>>
>>>> OK, so for this you would use maplist() to get the list of mappings to
>>>> disable, use maparg() to get the current mapping, clear the mapping, do
>>>> your stuff, then restore the cleared mappings. You then need to make
>>>> sure you restore the mappings exactly as they were, even when your
>>>> "stuff" fails miserably.
>>>>
>>>> It's a lot easier if we would have a way to temporarily disable
>>>> mappings. It's mostly the same as above, but you won't need to use
>>>> maparg() to get the current mapping and the restore operation. Instead
>>>> you would disable instead of clear, and later re-enable instead of
>>>> restore. Still need to make sure the re-enbling does happen, no change
>>>> in that part.
>>>
>>> Not sure I understood what exactly you suggest to disable/restore. All
>>> mappings at once with one command? I would actually disagree here: I
>>> need something similar for translit3, but it only remaps
>>> single-character mappings, leaving most of other user mappings alone.
>>> One mapping at a time? It would be good, but given that request is
>>> temporary remapping naming the functionality enable/disable looks
>>> strange. And there are still issues with determining {lhs}.
>>
>> No. Not all maps at once. As I understood it, the maplist() function
>> would return a list of map "handles", each corresponding to a specific
>> map. It would probably have optional args that let you request handles
>> for all maps, or all maps ambiguous/conflicting with a specific lhs,
>> or even all maps satisfying certain criteria: e.g., buf-local or
>> global. The enable/disable functions (and any other such interface
>> functions) would accept a map handle and would operate only on that
>> map. In addition to the enable/disable functions, I proposed adding
>> some sort of get_map_info() function, which would return the
>> information currently returned in the maparg() dict. Also useful would
>> be an "execute" function, which would allow you to invoke a map
>> programmatically through its handle, even if the map is currently
>> disabled or shadowed (e.g., by a buf-local map).
>
> This looks like being too complex for no real reason. Approach
> suggested by me simplifies C code and allows imitating features
> possible with your suggestion in pure VimL: disable is just `:unmap`,
> enable is `mappings_load()` (`add` is not a good verb), handle is one
> of the dictionaries, execute can be emulated by "define two temporary
> mappings: one to enter target mode, second is the original handle with
> lhs replaced, then do `:execute 'normal'
> "\<Plug>(EnterMode)\<Plug>(OriginalMap)"`. (`<Plug>(EnterMode)` is
> there to avoid problems due to user possibly remapping sequences to
> enter target mode, it should normally be something like `nnoremap
> <Plug>(EnterNormalMode) <Nop>`, `nnoremap <Plug>(EnterInsertMode) i`,
> etc.)

Hmmm... If the goal is to provide the missing functionality
with minimal additions to the current C source, perhaps your
way makes the most sense. Though I wouldn't go so far as to
qualify a clearer, cleaner and safer user interface as "no
real reason". As I see it, the advantage of the approach I
outlined is that you would specify the lhs and rhs key
sequences exactly once, in one of the formats currently
supported by Vim, and from that point on, you could manage the
map throughout its life-cycle without ever having to deal
with the key sequences again. With no need to pass the
sequences between functions, there would be no need for
intermediate/canonical formats, special escaping for
normal/execute, etc...

The <Plug>(EnterMode)<Plug>(OriginalMode) strategy for
executing maps feels a bit fragile, albeit more in keeping
with the original vi paradigm, in which scripts operate just
like a normal user who happens to type really quickly. But
over the course of the last few versions of Vim, VimL has come
a long way towards providing programmatic (functional) access
to the Vim internals. The hybrid approach you describe can
feel a bit kludgy at times: passing key sequences in and out
of functions, then having to escape them specially for use
with commands like :execute and :normal. I mean, I've grown
used to it, but I think you'd be hard pressed to make the case
that it's as clean and intuitive as a more functional
approach.

At any rate, I'm ok with either approach, provided that the
current limitations are addressed: namely, inability to
determine all conflicting/ambiguous lhs(s), and inability to
get lhs/rhs in deterministic/canonical form.

Sincerely,
Brett S.

>
>>
>> While this approach could be made completely generic, I can also see
>> that the <push>/<pop> mechanism could simplify certain use cases... Of
>> course, just making the lhs/rhs values deterministic (e.g., by using
>> the code used for mkvimrc) would definitely be an improvement,
>> especially if some sort of maplist() functionality permitted the
>> programmer to determine which maps conflict with a given lhs.
>> (Currently, mapcheck gives the rhs only, and there's no easy way to
>> determine the corresponding lhs.)
>>
>> Sincerely,
>> Brett S.
>>
>>>
>>> One of the logical variants would be `:map <push> {lhs}
>>> {new-rhs}`/`:unmap <push> {lhs}`+`:map <pop> {lhs}`, but this is hard
>>> to implement and is rather limited, though less limited then
>>> enable/disable everything variant.
>>>
>>> I would instead suggest a function mappings_dump()/mappings_add():
>>> first is similar to `nvim[_buf]_get_keymap` and should dump all
>>> mappings as a list of maparg()-like dictionaries. Second should define
>>> mappings being given a list of them. Of course, this means that
>>> dictionaries need to be fixed to allow correctly saving/restoring.
>>>
>>> The advantages:
>>>
>>> 1. Easier to implement. Code for creating a maparg() dictionary is
>>> already there, iterating over all mappings is not a problem. Results
>>> needs to be incompatible with maparg() or use additional keys though:
>>> e.g. Neovim altered the contents of `noremap` and `buffer` keys: first
>>> is now 0, 1 or 2 (you can't correctly restore a mapping if you can't
>>> distinguish `map <script>` and `noremap`) and second is a buffer
>>> number or zero.
>>> 2. More flexible: you can save and restore everything, push or pop
>>> individual mappings, create a temporary mapping which is just like
>>> mapping X, but has `<Plug>(Translit3TemporaryMap)` lhs instead (to be
>>> returned from `<expr>` mappings in order to select either plugin
>>> behaviour or fall back to previously present user mapping instead).
>>>
>>> I can imagine other usages enable/disable or push/pop could not
>>> achieve: generating configuration with mappings like :mkvimrc, but
>>> allows doing adjustments (parsing `:mkvimrc` output is not fun,
>>> especially if you want to be forward compatible), creating a plugin
>>> which analyses how often different mappings are used (need to copy all
>>> mappings to temporary then replace existing mappings with plugin
>>> ones).
>>> 3. This is also forward compatible: just need to state in the
>>> documentation that new significant keys may be added in the future to
>>> the dictionaries so they should be preserved.
>>>
>>>>
>>>> Big advantage is that if we evern add functionality to mappings, it will
>>>> keep working, while your save/restore pair probably fails.
>>>>
>>>> Ah, your later post goes in the same direction.
>>>>
>>>> --
>>>> DENNIS: Look, strange women lying on their backs in ponds handing out
>>>> swords ... that's no basis for a system of government. Supreme
>>>> executive power derives from a mandate from the masses, not from some
>>>> farcical aquatic ceremony.
>>>> "Monty Python and the Holy Grail" PYTHON (MONTY) PICTURES LTD
>>>>
>>>> /// Bram Moolenaar -- Bram@Moolenaar.net -- http://www.Moolenaar.net \\\
>>>> /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
>>>> \\\ an exciting new programming language -- http://www.Zimbu.org ///
>>>> \\\ help me help AIDS victims -- http://ICCF-Holland.org ///
>>>>
>>>> --
>>>> --
>>>> You received this message from the "vim_use" maillist.
>>>> Do not top-post! Type your reply below the text you are replying to.
>>>> For more information, visit http://www.vim.org/maillist.php
>>>>
>>>> ---
>>>> You received this message because you are subscribed to the Google Groups "vim_use" group.
>>>> To unsubscribe from this group and stop receiving emails from it, send an email to vim_use+unsubscribe@googlegroups.com.
>>>> For more options, visit https://groups.google.com/d/optout.

--
--
You received this message from the "vim_use" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php

---
You received this message because you are subscribed to the Google Groups "vim_use" group.
To unsubscribe from this group and stop receiving emails from it, send an email to vim_use+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

No comments: