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

Add gd and gD to go to declaration and type definition #257

Closed

Conversation

aioutecism
Copy link
Owner

@aioutecism aioutecism commented Jan 20, 2020

Recreating this PR.

@alisonatwork @jackfranklin I just realized this PR breaks gg and any potential g started motions.

Here is what's happening:

  1. g d and g D are registered at the root: root > g > d and D
  2. g g is registered under {motion}: root > {motion} > g > g
  3. Normal mapping will take priority of special mapping such as {motion}, so when g is pressed, this branch is activated: root > g
  4. The next g will fail to match.

Vim is treating gd and gD as motions and won't jump to other files, so this won't be a problem. In our case, we probably can't do that because we may jump to another file.

In order to make both g d and g g work, we need to rewrite some logic of GenericMapper.match so when normal mapping failed, we go back and try special mappings. This can be a bit tricky and may have performance issue. What's your thought?

@alisonatwork
Copy link
Collaborator

Interesting! I don't use g g so i didn't notice this. I'll take a closer look this week and see if i can come up with a nice way to support both.

@alisonatwork
Copy link
Collaborator

Just adding some thoughts here so I don't forget: I had a look at how vim handles this, and it does indeed treat these g commands as motions. This means that for gd and gD you can do stuff like dgd to delete back to the definition, although it's questionable what value that provides. There is even a precedent for jumping to another file - the gf command, which will try to find the filename under the cursor and open it. Doing dgf has interesting results.

I think if possible it might make sense to implement gd, gD and possibly gf in amVim as "motions" too, insofar as in the same matcher as the motion... but then just ignore the prefix (numeric, d, y etc) and jump straight to the destination. For people who really care about doing stuff like dgd that could be a feature added later for maximum vim compatibility.

That said, i haven't had time to have a deep look at the code yet. I hope to get some time on the weekend or during the Lunar New Year holiday next week 🐀

@aioutecism
Copy link
Owner Author

@alisonatwork Happy Lunar New Year! If we implement them as motions, we probably need to change all "actions" that handle motions to ignore just this one special case. This is not very clean IMO.

Maybe we can implement a special mapper(just like SpecialKeyMotion) for this rare case, give it higher priority than SpecialKeyMotion so it's always matched first. I haven't look deep into the current implementation(I forgot some details…), so I'm not sure if this can cause other problems.

@alisonatwork
Copy link
Collaborator

alisonatwork commented Jan 28, 2020

@aioutecism I have opened #258 to try to address this in a different way. It does succeed in working the same as vim for dgd and ygd as long as the definition is in the same file, which is pretty neat, but there are still a few quirks i am trying to figure out. Feedback welcome 😊

@alisonatwork
Copy link
Collaborator

This PR superseded by #258

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

Successfully merging this pull request may close these issues.

2 participants