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

camel case motion text object #460

Closed
muellan opened this issue Jun 23, 2014 · 34 comments
Closed

camel case motion text object #460

muellan opened this issue Jun 23, 2014 · 34 comments
Assignees

Comments

@muellan
Copy link

muellan commented Jun 23, 2014

It would be nice to have text objects for CamelCase editing ('dc,w' : 'CamelCase' -> 'Case') like seen here https://github.com/bkad/CamelCaseMotion.
Having some objects for underscore-separated words would also be great ('dc_w' : 'abc_def' -> 'def')

@albertdev
Copy link
Member

I've marked this as a feature request.

I'm not sure yet what we would map this to seeing how its an extension to Vim.

@keforbes
Copy link
Contributor

@muellan, you're requesting this to be a new vrapper plugin, right?

@muellan
Copy link
Author

muellan commented Jun 24, 2014

Yes, like the surround plugin or the one for the function argument text object, so a "subword text object plugin".
BTW: Considering how abundant camel case or underscore separated identifiers are in the programming world I find it surprising that vi din't already have text objects for that.
Or is there any clever way for let's say deleting one camel cased subword (especially if the cursor is not at the beginning of the subword) that I'm not aware of?

@keforbes
Copy link
Contributor

Vim has an iskeyword setting which defines the boundaries of w motions. You can tell vim to use _ as a word boundary by setting :set iskeyword-=_ which means "remove _ from the list of word characters". Supporting camelCase is a bit more complicated because it would require regex. Technically, you could tell it that a capital letter is a word boundary by removing A-Z from iskeyword but then all w motions would trip over every single capital letter. I'm guessing that would be more annoying than useful.

You asked if there was a way for vim to support underscore or camelCase boundaries. That's how vim does it. Otherwise, you'd have to resort to mappings (http://stackoverflow.com/a/8949552).

Anyway, that's my long-winded way of saying that I think a plugin is the right solution for this and I don't think you can really achieve what you want by simply customizing vim or vrapper.

@mmbradle
Copy link

mmbradle commented Jun 5, 2015

+1 for a CamelCaseMotion plugin

keforbes added a commit that referenced this issue Apr 15, 2016
This creates a plugin which defines camelCase and snake_case navigation.
It's still a work-in-progress but I think I'm ready for some outside
opinions to see if I got the use case right.
@keforbes
Copy link
Contributor

Hey @mmbradle and @muellan, I finally had time to create a rough implementation of this and I wanted to get your thoughts.

I looked at https://github.com/bkad/CamelCaseMotion and http://www.vim.org/scripts/script.php?script_id=1905 and tried coming up with something similar that made sense. So it doesn't exactly mimic either one and I wanted to see if I altered too many things or if it can still meet your needs.

Here's what it does right now (or will when I get the key bindings working correctly):
snake_case_word
camelCaseWord
_b ,b - move back to beginning of current snake_case or camelCase word
_e ,e - move to end of current snake_case or camelCase word
_w ,w - move to next snake_case or camelCase word

i, a, - select text object for camelCase word. Since there is no camelCase delimiter these two operations are the same.
i_ a_ - select text object for snake_case word. i_ selects up to first _ character, a_ selects current word and all following _ characters up to the next word.

Vimscript 1905 used , for their camelCase operator so I used that. bkad's implementation just re-defined the standard b e w operations and I wasn't sure if I wanted to go down that road. So I added _ and , as the keys to initiate things. This means that bkad's implementation allowed for v3ib to select current word and two previous words. My implementation is closer to the SentenceTextObject I recently implemented where v3i_ or v3i, will select the current word plus the two following. So my implementation doesn't support moving back with a count.

I guess I figured that if you didn't want to do any sub-word navigation, you'd still want w to jump over the entire word like normal but if you wanted to change a sub-word you would then use _ or ,. Is that a safe assumption or would you guys prefer if this plugin just overrode the default "word" behavior? Also, is it ok to have _ and , as separate keys or would you rather have one key and Vrapper "just figures out" whether it is snake_case or camelCase?

And if neither of you respond because this request is almost 2 years old, I'll just delete all this code. 😆

@muellan
Copy link
Author

muellan commented Apr 16, 2016

Great news! I definitely still want this. I created a command that lets me map vrapper normal mode keys to eclipses built-in subword navigation. But my 'text object' mapping hacks based on this are of course far from perfect.
Personally I would prefer just one set of "subword" mappings and let vrapper figure out if it is camel case or snake case. I strongly dislike the idea of overriding the standard word motions. They should stay untouched.
Do you plan to include this in the next unstable update? (please!)

@keforbes
Copy link
Contributor

There's one thing preventing me from including it in the next unstable update and it's that I can't get the ,w or _w motions (and therefore d_w operations) to work. They're both performing the default , and _ motions first and then my camelCase or snake_case motion. But the text objects work perfectly (since there are no existing , or _ text objects). If I picked some other key that didn't already exist as a motion then it would probably work fine. If you have an idea for other keys (that aren't already used for motions) then I could probably do a build for you to play with.

@albertdev recommended that I use the <Plug>(word-snake-forward) syntax like in bkad's implementation and require users to map the motion to any key of their choosing. Of course, that requires us to implement the <Plug> feature first. I'm not sure what all is involved in that.

@mmbradle
Copy link

@keforbes

That all sounds great to me. And I still want this as well. I'm used to using comma but could switch to something else if needs be. Underscore works as well, it is a little more intuitive than comma (In my opinion) but it is harder to get to, so I'd say the pros/cons balance each other out.

I also agree that I do not want the original behavior of w overridden.

I'd slightly prefer with one key for both snake and camel case.

I really like the v3i_ idea.

The main use case for me is renaming one part of a descriptive function or variable name, such as renaming moveFooToBaz() what would be really cool is to be able to get my cursor over Foo and then do something like:

  1. vi,w --Selects Foo
  2. _* --Searches for the word (I don't think this is currently supported by anything, I'd probably add my own mapping to support it)
  3. cgnEntity<esc> --Changes word to something else
    Now, moveFooToBaz() is moveEntityToBaz() and I can hit . for as many additional replaces as I'd like.

BTW, if you make this too good, then I'll want your implementation in vanilla VIM.

keforbes added a commit that referenced this issue Apr 18, 2016
keforbes added a commit that referenced this issue Apr 18, 2016
collide with other commands.  Handles more scenarios now.
@keforbes
Copy link
Contributor

I've updated the unstable update site with a new build (0.67.20160418) which includes this subword plugin so you guys can try it out. It now detects camelCase and snake_case implicitly so it only uses one key, but I changed that one key to \ (backslash) so it doesn't collide with any existing features. I figured if this plugin ends up using <Leader> then that's typically \ to begin with. You can probably remap it though if you want (I haven't tried that).

So currently, the plugin does the following:
\b - jump back to beginning of camelCase or snake_case word
\e - jump to end of current camelCase or snake_case word
\w - jump to next camelCase or snake_case word
i\ - select camelCase or snake_case text object
a\ - select camelCase or snake_case text object (including _ after snake_case word)

The \b \e \w motions work fine, but d\w y\w c\w operations don't work. However, di\ da\ ci\ ca\ all work fine.

Of course, it's only after pushing out this update that I find defects. I've been testing this with a camelCase word and snake_case word each on their own line. After installing this plugin, I learned that my regex doesn't quite handle .someCamelCase(). I thought I'd be clever and claim any non-capital letter matches camelCase, which means the . and ( characters are being included. I'll change that to only look for alphanumeric characters tomorrow.

keforbes added a commit that referenced this issue Apr 19, 2016
I'm still having issues with '\e' not jumping to the next (normal) word
when the cursor is already on the last letter of the last (snake/camel)
word.
@muellan
Copy link
Author

muellan commented Apr 19, 2016

Just tried the plugin - great work so far!
I discovered two issues:

  1. Consider this code: void foo(InputRange in) with the cursor on "a". I think di\ should delete "Range" but not the space after it (which it does).
  2. Since \ is almost useless on German keyboards I did omap i, i\ and omap a, a\ in my vrapperrc. d and c mappings work just fine but I can't get visual mode mappings to work, so vi, or va, will do strange things. This behavior thays the same if I use _ instead of ,.

@muellan
Copy link
Author

muellan commented Apr 19, 2016

One more thing: If CamelCase and snake_case are mixed like this:
memberVariable_ or this camelCase_snake the motions will only consider the _.
Could you also make it so that the plugin jumps to the nearest subword boundary, so if you are at the first "c" of camelCase_snake then repeatedly pressing \w will jump to "C" and "s"?

keforbes added a commit that referenced this issue Apr 19, 2016
handle the scenario myself.  This fixes some corner cases I wasn't
handling.
@keforbes
Copy link
Contributor

Ok, I've updated the unstable update site again with a new build (0.67.20160419) with some of those issues fixed. You're right, I'm currently looking for the existence of a _ character to determine whether to use snake_case regex or camelCase regex. I'll spend some time tomorrow trying to come up with a clever regex that supports both.

keforbes added a commit that referenced this issue Apr 20, 2016
…ord.

I kept telling myself I'd come up with a clever regex that would handle every possible scenario but this solution is more readable and works as far as I can tell.
@keforbes
Copy link
Contributor

I've updated the unstable update site with some more fixes, please try it again. Also, you said you defined a custom omap but did you also define a vmap? I'm surprised visual mode isn't working for you since that's how I've been doing most of my testing.

@albertdev
Copy link
Member

Talking about mapping, have you tried :vnoremap i, i\ ?

@muellan
Copy link
Author

muellan commented Apr 21, 2016

Remapping is working fine now. I forgot to remove one of my old hacks to emulate subword navigation.

@keforbes keforbes self-assigned this Apr 26, 2016
@keforbes
Copy link
Contributor

I finally figured out the <Plug> thing. Everything seems to be working as expected now. I'll update this issue again once I have a build ready for you guys to test.

Here's what I was using in my .vrapperrc to test this feature:

map \b <Plug>(subword-back)
map \e <Plug>(subword-end)
map \w <Plug>(subword-word)

map i\ <Plug>(subword-inner)
map a\ <Plug>(subword-outer)

@albertdev
Copy link
Member

albertdev commented Apr 26, 2016

Meanwhile I've been experimenting with support for <Leader> so I'll see if I can add a custom command to the Sub-word-motion plugin to setup default remaps using that <Leader> key. I'll submit a pull request in the coming days.

@keforbes Let me know if you discover anything odd about those <Plug> mappings, I think I fixed some problems in a (private) prototype branch and I might or might not have ported those fixes to our master branch (though I was using them in a special manner there).

@keforbes
Copy link
Contributor

Yeah, I was playing with the idea of adding this to ConstructorWrappers.java:

} else if(key.equals("LEADER")) {
     KeyStroke leader = new SimpleKeyStroke(vim.getConfiguration().get(Options.MAP_LEADER).charAt(0));
     result.add(leader);

but then I'd need access to the EditorAdaptor in that method, which I'd rather not do.

For my use case, the <Plug> feature seemed to work fine, no problems in any of the scenarios I tested.

@keforbes
Copy link
Contributor

I've updated the unstable update site with a new build (0.67.20160426) which includes this fix. Please give it a try and let me know how it goes. Also, make sure you define your <Plug> mappings in .vrapperrc like I mentioned above.

albertdev added a commit that referenced this issue Apr 29, 2016
Users could bind everything themselves but it's just boilerplate. This
command will automate the bindings for them.

It works in 4 or 5 ways:

- running `:subwordmappings` with no parameters will require you to
press `<Leader>b` for the back motion, `i<Leader>` for the textobj.
- running `:subwordmappings ,` will assign the back motion to `,b`. The
textobject is triggered by `i,`.
- running `:subwordmappings , _` will assign the back motion to `,b` but
the textobject to `i_`.
- finally, the first parameter also accepts `''` to indicate that no
mapping prefix should be used, hence binding subword-back to `b`.
The (optional) second parameter works like before, if not present it
is set to `<Leader>`.

Related to #460.
@albertdev
Copy link
Member

I've added a special command to set up those mappings:

  • running :subwordmappings with no parameters will require you to
    press <Leader>b for the back motion, i<Leader> for the textobj (you can run :let mapleader="," to use a comma instead of the default backslash).
  • running :subwordmappings , will assign the back motion to ,b. The
    textobject is triggered by i,.
  • running :subwordmappings , _ will assign the back motion to ,b but
    the textobject to i_.
  • finally, the first parameter also accepts '' to indicate that no
    mapping prefix should be used, hence binding subword-back to b.
    The (optional) second parameter works like before, but if not present it
    is set to <Leader>.

Now that I've committed it I'm wondering if I shouldn't reverse the parameters, but I'll wait for you guys' feedback before doing so. Also note that you can pass more than one character (e.g. ,,) or special keys.

@keforbes
Mind you, I'm not sure if this is the best final solution. I was seriously thinking of implementing the :let g:variable="" command and some way for a plugin to detect that the configuration has been loaded. This would make it so that the plugin automatically adds the default mappings unless the user has put something like :let g:subword_default_mappings="0" in his .vrapperrc, after which he can call this helper command or explicitly bind every plug.

The possibilities dazzled me though - I'm wondering if we need plugins to explicitly declare variables or if it's essentially free-form like Vim, and when or how it would need to be set (Vrapper has no concept of after / before or even a single config load at startup so concepts are really different from Vim).

@keforbes
Copy link
Contributor

I have mixed feelings about this. I agree it's nice to provide default key bindings so users don't have to add them to their .vrapperrc themselves, but if we're just replacing boilerplate mappings with some other custom commands, what are we gaining? At least the boilerplate mappings are standard vim commands. Plus, the original implementation I based this on only defined <Plug> operations.

@muellan, @mmbradle, what are your thoughts? Do you like having a single command subwordmappings to add to your .vrapperrc which will setup all the default mappings for you or would you rather define the mappings yourself?

@muellan
Copy link
Author

muellan commented Apr 29, 2016

I like the Plug solution. Either that or default leader mappings and the possibility to define the leader key.

@albertdev
Copy link
Member

@keforbes You forgot the third option: let the plugin bind default mappings unless you set some special variable (or somehow detect that the user hasn't mapped these <Plug>s), in which case you can still map things by hand.

Do note that the forked version of the plugin also had something like this. From its documentation:

To use the default mappings, add the following to your vimrc:

call camelcasemotion#CreateMotionMappings('<leader>')

The original plugin on the other hand simply bound everything to ,b / w / e unless it detected an existing mapping by means of the hasmapto() function.

Advantage for the latter is that I don't need to invent a "Variable API", downside is that I need to search through all current mappings.

@muellan Your reply seems cut short?

@albertdev
Copy link
Member

Before I forget: these options are not mutually exclusive. The custom <Plug> maps should always work, that :subwordmappings command just maps it for you.

@keforbes
Copy link
Contributor

That's true, you can either use <Plug> or :subwordmappings. I'll leave it alone then. 😄

@keforbes
Copy link
Contributor

I've updated the unstable update site with a new build (0.67.20160429) which includes support for <Leader> and :subwordmappings if you're interested.

@muellan
Copy link
Author

muellan commented Jun 3, 2016

One little annoyance / defect:
if the cursor is inside aaa in obj.aaa_bbb_ccc() or if( aaa_bbb and I delete the inner subword then the character before the first word (the dot in the first case, the space in the latter) gets also deleted.

@keforbes
Copy link
Contributor

keforbes commented Jun 6, 2016

@muellan, I just released version 0.68.0 over the weekend. Can you install that version and try it again? I think that issue has already been fixed.

@muellan
Copy link
Author

muellan commented Jun 6, 2016

Just installed version 0.68.0; it is fixed.
Also very nice work on the cycle plugin!

@muellan
Copy link
Author

muellan commented Jun 26, 2016

There is still one issue:
Suppose you have the text using boundary_iterator = and the cursor is at the first character of a leading subword ("b" in this case). If you do di\ or ci\ the word before ("using " in this example) gets also deleted.

@keforbes
Copy link
Contributor

@muellan, I've fixed the issue with text objects when the cursor is on the first character of a word. I also fixed the corresponding issue when the cursor is on the last character of a word. And also if the cursor was on the last character of a subword. I'll let you know when I've updated the unstable update site so you can test it out.

@keforbes
Copy link
Contributor

@muellan, ok, I've updated the unstable update site with 0.69.20160629 so you can test out my changes. Please let me know how it goes.

@muellan
Copy link
Author

muellan commented Jul 1, 2016

works fine now - thanks!

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

No branches or pull requests

4 participants