Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mapping interferes with regular commands #415

Closed
albertdev opened this issue Apr 14, 2014 · 7 comments
Closed

Mapping interferes with regular commands #415

albertdev opened this issue Apr 14, 2014 · 7 comments
Labels

Comments

@albertdev
Copy link
Member

As mentioned in commit d089258, certain mappings can interfere with a normal command. For example:

noremap ]] :nextmember<CR>

When then pressing di] the "operator pending" caret remains active and the delete action won't finish.

Instead, the ]] sequence should only be matched with the command buffer when the current keymap was just (re-)entered.

Optionally, a timer might need to be implemented which aborts the remap matching after a while and just proceeds with the command as entered (though this might be hard to do with the current code).

This bug was mitigated by commits d089258 and fa74be5, but fixing #411 by reverting these two commits has brought this bug back.

@albertdev
Copy link
Member Author

@igrekster
I just thought about something: by fixing #411, Vrapper does differentiate between :onoremap and :nnoremap when using an operator, so your use case should still work.

If we ever implement an "eclipse motion action", things might become more dire as you might want to use it as an operator.

@igrekster
Copy link
Contributor

@albertdev
It seems the only sensible way of adding an "eclipse motion" is to assign it to a character:

eclipsemotion Q org.eclipse.cdt.ui.edit.text.c.select.enclosing

This way we would put a motion command into the normal and visual maps and a motion text object into the operator map. So for the example above dQ would delete a C++ enclosing element, and Q in the normal mode would probably move past that element, etc.

@keforbes
Copy link
Contributor

keforbes commented May 4, 2014

@albertdev, you mentioned in #425 that we really should fix this defect. I'd also like to see this defect fixed mostly because it's another regression from our last release. I just wanted to add my vote in case you were debating which defect to work on next. :)

Also, thank you for spending so much time with Vrapper and contributing so much to this project. I really appreciate the work you're doing and I'm amazed at the things you're able to fix. I feel like I'm not pulling my own weight anymore but I'll try to help you out as much as I can. Thank you for continuing to move this project forward!

@albertdev
Copy link
Member Author

@keforbes Actually, it's not a regression in the usual sense: Vrapper 0.32.0 and before also suffered from this problem, so I had to make the trade-off of fixing #409 or reintroducing this bug from long ago. After fixing #425, most corner cases are handled through the magic of NO_KEYMAP.

Still, it would be nice to have more complete support for maps, we're still a while removed from Vim in that regard. I'll look at it in the coming days.

As for the help, you're most welcome. I steal all the challenging work while you are left with the chores of releasing. ;)
So I must thank you in turn for keeping this project alive, as I know that by my own volition I would let it to whither once most challenges would be solved.

albertdev added a commit that referenced this issue May 12, 2014
Mapping 's' to something in normal mode would have weird side effects:
since KeyMapResolver would keep returning "NormalMode", pressing 'fs'
used to trigger the 's' mapping even though 's' is not a prefix of 'fs'.

Since the resolver now returns null when in the middle of a command,
mappings will no longer be triggered in that state.

One side-effect is that all KeyMapResolver instances can not return null
after a CountConsumingState is traversed or they would break counts
before remaps.

Also added helper methods in ConstructorWrappers to help with operator
keymaps (those need to return omap after the operator or when any count
for the following motion was entered).

Work for #415.
albertdev added a commit that referenced this issue May 12, 2014
@albertdev
Copy link
Member Author

Just an update: I think I've made some good progress on this issue.
In the end it was mostly a question of returning null when a mapping cannot be started anew and returning the current mode when a count gets passed.

All tests work, but I'm going to do some more tests and fixes as I think that most of our plugins could be broken with respect to counts before a text object.

albertdev added a commit that referenced this issue May 14, 2014
Plugins needed a bit of extra work to support counts before operator
mappings.

Work for #415.
@albertdev
Copy link
Member Author

I think this issue has been resolved, no more need for workarounds.

@keforbes
Copy link
Contributor

I've updated the unstable update site with a new build (0.43.20140515) which includes this fix in case anyone else is interested in it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants