robe
robe copied to clipboard
Add capability to jump to MRI C method sources
Human-readable explanation of the scope of changes is below:
Overview
This PR adds the ability to (usually!) jump to the C sources of Ruby methods.
It only works if the user has GNU Global installed and sets a few Elisp variables to the right values.
If the user doesn't set the variable robe-use-global-p
to true,
the behavior of Robe stays exactly as it is now -- none of
the "jump to C source" machinery is engaged.
Summary of code changes
- Add functions
robe-list-c-method-files
,robe-determine-c-file-for
,robe-jump-to-c-source
- Add variables
robe-use-global-p
,robe-ruby-source-directory
,robe-global-command
- Change the interface of
robe-jump-to
to add optional arguments that allow it to pass in the value ofthing-at-point
for the "jump to C source" machinery. Because the arguments are optional, we don't need to change any ofrobe-jump-to
's callers - Modify
robe-jump-to
to check the value ofrobe-use-global-p
to decide whether to try to jump to C method sources. For users who haven't set this variable, Robe works exactly as it does today.
Remaining bugs/weird UX
- Finding the C source of the method is not 100% accurate - some of this is due to GLOBAL, some due to the regex search's incomplete coverage
- When jumping back from the C source, you have to hit
M-,
twice for some reason (possibly aggtags-mode
thing) - Sometimes you have to enter the module name in the minibuffer twice to jump to C-based definitions, I'm not sure why yet
Hi!
Sorry about the wait. This is an interesting idea, but I don't like some implementation details. Namely, the added optional arguments (could we split it into a pluggable feature that touches the existing code very litte?) and a bit awkward integration with ggtags
(ggtags-find-tag
doesn't seem like it's doing too much, maybe we could inline the definition, like with robe-list-c-method-files
).
Further:
-
xref-push-marker-stack
is only available in Emacs 25. If we were to only target that version, maybe it could be a good idea to use Xref and a separate backend for this feature. Unfortunately, the fallback mechanism there doesn't take into account whether the first tried backend returned any results. So something would have to change for that, here or there. - It would be cool if instead of hardcoding the sources directory we took into account the currently used Ruby version (via rbenv or rvm). Unfortunately, both of them by default delete the sources after installation, so it would need a fallback.
This doesn't seem like a frequently-requested feature, so I think we can take our time improving it.
Thanks for this thoughtful feedback. Sorry about my late response.
I agree with you 100% - this is a pretty hacky approach.
I'm not sure when/if I will be able to work more on this PR. Please feel free to reject/close if you like.
Hopefully someone gets interested in hacking on this type of feature in the future. I agree with you that it's not really necessary for day-to-day Ruby programming though. :-)
This might be relevant: https://github.com/pry/pry-doc/pull/82
Once it gets into a release, delegating some of this logic to pry-doc seems like a better choice (and it only requires find plus etags).