vrapper / vrapper

Vim-like editing in Eclipse
http://vrapper.sourceforge.net/
GNU General Public License v3.0
1.11k stars 150 forks source link

f/t should not use omap mappings #411

Closed Konfekt closed 10 years ago

Konfekt commented 10 years ago

Perhaps someone can reproduce this. Using f to find L in a line, vrapper jumps to all occurrences of $, too.

keforbes commented 10 years ago

I don't understand. I just tried fL on a line with:

L   $   L   $   L   $

And it correctly jumped to the L each time. What am I missing?

Konfekt commented 10 years ago

This might be the problem: The line

noremap L $

in .vrapperc.

albertdev commented 10 years ago

Ah, that's it.

Vrapper 0.42.0 now includes pull request #397 where noremap also affects the operator-pending keymap, causing fL to be translated to f$.

This is what the Vim help has to say about your use case (see |omap-info|):

To enter a mapping for Normal and Visual mode, but not Operator-pending mode, first define it for all three modes, then unmap it for Operator-pending mode: :map xx something-difficult :ounmap xx

So changing your .vrapperrc to the following should fix your problem:

noremap L $
ounmap L
Konfekt commented 10 years ago

Ok. That noremap also applies to omode is what we would expect from vim.

But in vim, search commands such as f, t and / are imode but not omode pending.

Le 10 avril 2014 12:14:02 Bert Jacobs notifications@github.com a écrit :

Ah, that's it.

Vrapper 0.42.0 now includes pull request #397 where noremap also affects the operator-pending keymap, causing fL to be translated to f$.

This is what the Vim help has to say about your use case (see |omap-info|):

To enter a mapping for Normal and Visual mode, but not Operator-pending mode, first define it for all three modes, then unmap it for Operator-pending mode: :map xx something-difficult :ounmap xx

So changing your .vrapperrc to the following should fix your problem:

noremap L $
ounmap L

Reply to this email directly or view it on GitHub: https://github.com/vrapper/vrapper/issues/411#issuecomment-40062335

keforbes commented 10 years ago

This defect seems to affect any command that takes an arbitrary character as an argument. With map L $ I just tried qL and Vrapper said "recording: $". I really want to fix this defect but I don't know enough about omap to know how to fix it.

EDIT: It seems resolveKeyMap in NormalMode.java uses omap if we're in a ConvertingState. I wonder if we can define a "non-operator pending converting state". Otherwise, I'm afraid this if statement would have to list out every command to not use omap with.

ghost commented 10 years ago

This is it. There are two pending states, insert mode pending (for example when searching, marking or recording), and operator pending, to which omap should apply (movements such as b,e and w, and range operators such as i/aw = inner/all of word).

Why would it be that we can't define a non-operator pending state? For example, the search keys / and ? already are insert mode pending commands.

keforbes commented 10 years ago

All normal mode keys "transition" between states which eventually determine exactly what operation to run. It was on that transition (ConvertingState) where omap was implemented. So all transitions get omap mappings right now. / and ? are separate modes which use the provided string as an argument, not as a transition between operations. So we never really implemented these pending modes at all.

albertdev commented 10 years ago

@keforbes I was studying it today. I think I might have to revert the code to before igrekster's changes in d0892587c1974da3a9469683158d160f2ee4c324 and your changes in fa74be5d5eee35cc553540ff035fb84bc18d4c1c to get it working. Both commits are about an issue which actually reared its ugly head again.

In the end, I think the remap code will need a thorough screening. I've been writing some test code to catch a few cases, including the case in #409. So far, I haven't really started changing anything, but I'm trying to cover most cases.

Konfekt commented 10 years ago

A quick and dirty, but by all practical means suficient fix would be to ignore omaps for f/t.

The practical justification: These commands receive a single letter, and noone remaps a single letter, like a to b, in insert mode.

To the user, this would be the same as saying that f/t do not accept imaps.

Perhaps you could point out these transition functions to understand what's going on for f/t?

So there are now two Converting States for normal mode mappings, for imode and omode.

If that's too much tinkering for the very unlikely case that the user remapped single letters in insert mode, the above hack is a good compromise.

Le 13 avril 2014 22:03:57 keforbes notifications@github.com a écrit :

All normal mode keys "transition" between states which eventually determine exactly what operation to run. It was on that transition (ConvertingState) where omap was implemented. So all transitions get omap mappings right now. / and ? are separate modes which use the provided string as an argument, not as a transition between operations. So we never really implemented these pending modes at all.


Reply to this email directly or view it on GitHub: https://github.com/vrapper/vrapper/issues/411#issuecomment-40318049

albertdev commented 10 years ago

@EPNGH The code actually used to do that before omap got implemented, but due to a small error a lot of combinations suddenly started using omap when not called for. I think this went unnoticed as noremap wouldn't bind anything in omap before the 0.42.0 release, and likely few people were using omap or a Normal mode remap using an operator command.

Long story short: we're working on it.

keforbes commented 10 years ago

I've updated the unstable update site with a new build (0.43.20140415) which includes this fix. Give it a try and let us know if you find any problems with it. Thanks!

Konfekt commented 10 years ago

I installed, tried and can confirm that the issue that opened this thread is fixed. Thank you!

Konfekt commented 10 years ago

Curiously, Just found out that f does not accept imap either. lmap it is. (:help language-mapping)

Just as a sidenote.

albertdev commented 10 years ago

Interesting, I didn't know Vim had lmap. Anyway, we're only going to support that if somebody makes a clear feature request for it.

Konfekt commented 10 years ago

Reasonable. I think very few know about it. What's more it seems to be more consistent if imap applies to r,f and so on as well. I was confused about these not applying in Vim. That's how I noticed.