Operator pending #152

Closed
wants to merge 5 commits into
from

Projects

None yet

4 participants

@jroes

This is a rough implementation of operator-pending mode, with some explanation and discussion in #58 and #129.

Based on discussion in #129, this doesn't seem like it's the correct long-term solution to the problem.

All tests pass in this PR except for two new tests that came after I put in the original work for this mode that I haven't had a chance to look at --

Operators
the y keybinding
when selected lines in visual linewise mode
it saves the lines to the default register
@bhuga bhuga commented on the diff Mar 9, 2014
keymaps/vim-mode.cson
@@ -119,3 +119,47 @@
'ctrl-w v': 'pane:split-right'
'ctrl-w ctrl-s': 'pane:split-down'
'ctrl-w s': 'pane:split-down'
+
+'.vim-mode.operator-pending-mode:not(.mini)':
@bhuga
bhuga Mar 9, 2014

There's a lot of duplication here...could we rephrase the regular vim-mode list as not(.operator-pending)?

@mcolyer
mcolyer Mar 10, 2014

I like this idea.

@jroes
jroes Mar 14, 2014

Yeah, I hate the duplication too. I'm not sure if I can do that though. If by regular vim-mode you mean command-mode, once I start doing that things disappear from command-mode, because they aren't necessarily exclusive. They kind of mean something different in each mode. Maybe I can do two comma-separated selectors though...

@jroes
jroes Mar 14, 2014

Tried doing something like this:

'.vim-mode.command-mode:not(.mini), .vim-mode.operator-pending-mode:not(.mini)':
     'w': 'vim-mode:move-to-next-word'
    'e': 'vim-mode:move-to-end-of-word'
    'b': 'vim-mode:move-to-previous-word'
    'x': 'vim-mode:delete'
    'd': 'vim-mode:delete'
    'c': 'vim-mode:change'
...

But I don't think it's working. Is it possible this multiple selector format isn't supported by atom core?

@mcolyer mcolyer commented on the diff Mar 10, 2014
lib/operators.coffee
@@ -120,6 +133,11 @@ class Yank extends Operator
else
@editor.clearSelections()
+ if @motion instanceof textObjects.InnerWord
@mcolyer
mcolyer Mar 10, 2014

I think this needs to be done in a more generic way on the motion. Otherwise we're going to start including a check for many different text objects.

@jroes
jroes Mar 14, 2014

Agreed. Total hack here. I think we have to change the interface a bit to support an after callback.

@mcolyer mcolyer commented on the diff Mar 10, 2014
lib/vim-state.coffee
@@ -148,20 +152,20 @@ class VimState
_.each commands, (fn, commandName) =>
eventName = "vim-mode:#{commandName}"
@editorView.command eventName, (e) =>
- possibleOperators = fn(e)
- possibleOperators = if _.isArray(possibleOperators) then possibleOperators else [possibleOperators]
- for possibleOperator in possibleOperators
+ operations = fn(e)
@mcolyer
mcolyer Mar 10, 2014

👍 on the rename here, definitely clarifies.

@mcolyer

@jroes I think we're on the right track here. There's a bit more refactoring to do but nice work so far.

@mcolyer mcolyer added the enhancement label Mar 10, 2014
@jroes

@mcolyer @bhuga based on the discussion in #129 do you guys think it's worthwhile to see this through to its conclusion?

@mcolyer

@jroes I still think this is useful.

@nathansobo

I think I may take a stab at reimplementing this rather than catching this up with master. I may change my mind. Either way I'll be sure to credit you in the change log and reference this PR.

@jroes

@nathansobo totally good with me. sorry I haven't had a chance to devote more time to it!

@nathansobo nathansobo referenced this pull request Mar 19, 2014
Merged

Implement operator pending mode and text objects #183

1 of 7 tasks complete
@mcolyer

@nathansobo @jroes are you cool with closing this out in favor of #183?

@nathansobo nathansobo closed this Mar 19, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment