Implement gD

[irrationalistic]

gd and gD both are mapped to find the definition of a variable. Support for variables starting with _, @, and $. Doesn't follow scope, so will find the first defining version of a variable in the file. Uses a loose regular expression to find common variable definition pattern.

[irrationalistic]

I'm not sure why the travis build errored out! All the tests passed when I ran them in-editor. Any thoughts?

[irrationalistic]

I should also note that this implementation doesn't use ctags unlike the built-in "go to declaration"!

[winstliu]

I'm not sure why the travis build errored out!

I've convinced it to reconsider 😉.
All the tests are passing but you have 2 lint errors - mind fixing those? 😄

[irrationalistic]

Thanks! Fixed!

[bronson]

You actually implemented gD (find starting at beginning of file), right? Or am I misreading the code?

I guess a sane gd implementation would lean heavily on the [[ motion, which vim-mode doesn't have yet.

[irrationalistic]

Yeah, technically this is gD! I think the ideal would be to allow certain languages to register what scope looks like so that vim-mode knows where certain components can be found. Or we could just build out a set of pre-determined language scopes, which would lead into [[ and gd. Does that seem doable?

[irrationalistic]

I was actually going to work on some of the bracket motions next. Would love some thoughts on ideal implementation!

[bronson]

I have the same thought but no idea how doable it would be.

Maybe vim-mode could lean on grammars to provide its motions? Then every language would be supported. I imagine this can't be as easy as it sounds. :)

For now it might make sense to provide gD and leave gd unimplemented? Merge the stuff that works!

[irrationalistic]

I actually explored grammars pretty heavily when i created the tasks plugin, but for the most part there isn't any normalization! You can name things whatever you want and class things however you like. We might be able to make use of indentation (which i believe is how the folds are detected), but that's about all we can rely on.

I can agree with removing the gd keybinding and just merging in the bit that works! I'll add another commit to clear the binding out for now.

[irrationalistic]

I found this bit from the atom goto package goto symbol generator that essentially uses a big ol' list of grammar scope-name matches. This is another option, but still requires vim-mode to keep track of all the languages.

[bronson]

Yeah, that makes sense. Nothing is ever easy. I hope the goto package can solve this (filing a ton of PRs that normalize names? that would be huge!)

Given how simplistic stock Vim's [[ and [] keybindings are implemented (just searches for { or } in the first column), maybe do that for now? Worry about hooking other languages once that works.

And I definitely vote for ignoring nroff paragraph/sections when implementing these motions!

[irrationalistic]

I'm not familiar with nroff! Hopefully that means I already ignore it :)

Eventually it would be nice to support languages like coffeescript or python where {} are hard to come by.

[bronson]

Well, Vim's docs say:

]]                      [count] sections forward or to the next '{' in the
                        first column.  When used after an operator, then also
                        stops below a '}' in the first column.  exclusive
                        Note that exclusive-linewise often applies.

(and run :help section to see why you want to ignore roff...)

Vim's language plugins remap the motion key bindings to be more useful.

It might make sense to follow Vim's lead here: have vim-mode implement whatever core Vim does, then attach language-specific functionality using something like per-grammar keymaps (except that might not be so easy). Or figure out some other technique to isolate the language-specific features. Scattering language knowledge through vim-mode's code will get out of hand pretty quick!

[bronson]

To be more clear, I 100% agree with supporting coffee/python/ruby/etc, but I'm not sure language knowledge belongs in vim-mode's core. Maybe it would go in a new package (vim-mode-ruby), or a new directory in vim-mode (vim-mode/lib/languages/ruby)? Something like that.

[irrationalistic]

Totally with you! That makes complete sense. I think it makes sense to have additional packages that augment the core, but since vim-mode is essentially vim in atom, and if vim doesn't have the functionality, there's no need to interrupt the "native" experience.

[bronson]

This works great. If you change the name of the issue to 'implement gD' then I think this PR is ready to go. (until the per-language stuff makes things interesting of course).

I added it to vim-mode-next.

[irrationalistic]

Done! Thanks! Going to start exploring gd and the other bracket-style motions too.

[jacekkopecky]

maybe remove lines 2, 3, 5? They seem unused.

[jacekkopecky]

@irrationalistic ☝️ ? 😎

[irrationalistic]

Sorry! Currently traveling, will do this asap!

[irrationalistic]

Done! Awaiting CI...

[jacekkopecky]

$ is added in the default keyword regex, but if the user has iskeyword set, theirs will be the same as for SearchCurrentWord then. I'd suggest that, until this regex depends on the language, we just rely on iskeyword for the right behaviour.

If you agree, this constructor could go.

[irrationalistic]

I'm not sure I follow. Maybe I don't fully understand the iskeyword setting? The intent here was to focus on variable definitions and, for at least php and javascript, the $ is part of a valid variable name. Without that included in the regex, it would be ignored and the word search would seek the first occurrence of the word without the extra character, right?

[irrationalistic]

Related to this and your note about replacing the scan with super:

I've taken out the majority of the code, bringing it down to just the call to super and added in an additional parameter to allow the search to halt on the first result. This works great for almost all items, so thanks for the great suggestions! (this will probably get much more complicated for scope-level gd, so I think it makes sense to keep it in a separate file until that's built)

However, I still am not sure about the right approach for supporting variables that start with $. All but those type of variable pass the test! I can get it to work if I keep the constructor and the custom getSearchTerm to include the $ and escape it properly, but I'm not sure if there is a good way to make this work without those two functions! Thoughts?

[jacekkopecky]

Very good, I can answer both questions together: I would suggest that we treat, for now, languages with variables starting with $ as a user setting: in the tests with $, do a

beforeEach -> 
  # a user may choose to include `$` as part of names:
  atom.config.set('vim-mode.iskeyword', "---the regex with $ in it---")

For example, I don't like having @ as part of * searches, so I have, in my init.coffee, the command atom.config.set('vim-mode.iskeyword', "[a-zA-Z0-9_]+")

Yeah, it's very likely I think that [[ and gd may mean gD will change, but for now, having a basic gD would be good.

[irrationalistic]

Great! Makes sense. I'll set that up and commit the changes ASAP.

[irrationalistic]

I made some big changes to this pull request to re-use more code. I'm going to try to rebase it with the current code to fix up the conflicts.

[irrationalistic]

Alright! Should be back on track :)

[jacekkopecky]

This would prevent the use of $ to mean the end of the line in normal searches.
I would actually suggest to override getSearchTerm in SearchCurrentWord so that it simply returns new RegExp(_.escapeRegExp(term), 'g') - or with 'gi' instead of 'g' if case insensitivity is desired for SearchCurrentWord and its kin.

[Zundrium]

Installed this PR and it works perfectly for me with the last master-branch, thanks @irrationalistic!

[irrationalistic]

Awesome!

[lee-dohm]

As stated in the README, this package is no longer maintained and is deprecated. We recommend that people use the vim-mode-plus package instead. Because of this, we are archiving this repository and closing all issues and pull requests. Thanks very much for your support and contributions!