-
Notifications
You must be signed in to change notification settings - Fork 280
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 support for relative path for gf #1917
base: master
Are you sure you want to change the base?
Conversation
Thanks for the PR @liuzhishan. I think there will be some work required before we can merge. A few observations:
I'll take a look at the code in more detail once these things are ironed out. I hope that helps. |
@tomdl89 thanks for the detailed reply. I'll fix the code and update the pr. |
Hi, I have update the code, please have a look if there are any other problem. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added a load more comments. If this is an enjoyable educational process, then by all means keep going. Equally though, if this is turning out to be more work than you had hoped for, feel free to drop this PR, make an issue, and I'll take a look when I get the time. Cheers
evil-commands.el
Outdated
(find-file file-path) | ||
|
||
(if (and (stringp file-path) | ||
(string-match-p "/" file-path)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not 100% sure on this, but have you checked if this check would work in windows? I believe they use backslashes over there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have add a check on system-type to distinguish windows and other system.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, I haven't tried your function yet (because it requires functions I don't have) but your diagram of accessing a/a.h
from b/b.h
doesn't seem to tally with the vim's documentation on path
(by default, anyway). So I'm not sure if your diagram is wrong and your functionality is right, or the other way round, but that's worth checking too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please specify which part of vim's documentation on path ? I checked vim doc, but found several entries on path.
I have tested the function on mac and linux, it worked. I don't have a windows os, but as far as I know, the different of windows and linux path is just the '/' and '' seperator. So after I add a logic to distinguish os type, it should work on windows too.
Hi, I think this is a enjoyable learning process. I use doom emacs, and got used to use some doom function. I'll rm the doom related code, and fix other problems. |
Hi, thanks for the detailed comments. I have fix the code. The main change are as follows:
|
Hey @liuzhishan thanks for getting it to the point where I can test it. It seems to work as you want, but the same functionality doesn't work in vim for me. I don't have a C codebase to hand, so vim may behave differently there, but with a structure of text files as you laid out in your description, vim says |
Hi, @tomdl89 I known where the problem is. It's not a vim functionality of When I do So it's a a new functionality for |
Usually, but not always. Evil can improve upon vim in some cases. This is especially true when dealing with something on the interface between emacs and vim. Because there is no direct equivalent of I'm happy to proceed with this PR, but I think the right thing to do is to implement the least surprising hard-coded approach, and then we can look at adding customizability later. For me, if I'm visiting |
Ok, let me add some comment to document the limitation. As for the least surprising approach, I think changing the traverse order would be a solution, from parent of |
Hi, @tomdl89 already add comments of limitation and update the search order, please check the code. |
Hi, @tomdl89 It's been a while since I fix the code last time. If you have time please take a look at the code. |
Why not contribute this to ffap upstream instead? I do not see the point of adding yet another bespoke implementation to Evil that does not even attempt to be bug-for-bug compatible with Vim, when a standard, Emacs-native counterpart to |
In evil mode,
gf
doest work when path under cursor has common ancstor with current buffer file. This seems to be a common situation. For example, suppose I have the following files:and in b.h, the file
a/a.h
is included:The
gf
command was mapped tofind-file-at-point
. If I want to opena/a.h
file usinggf
inb.h
file, it doest not work.To solve this problem, I add a new function
evil-open-file-under-cursor
:find-file
.evil-find-file-at-point-with-line
as the origin logic.And also I changed the
gf
key mapping inevil-maps.el
: