Closed FelipeLema closed 5 years ago
Hi:
For me it is fine what ever you decide that could make the code simpler/smaller/better maintanable.
There is only a detail that should remain in my opinion is: In case the cursor is already in a symbol/word it could be better (simpler to implement and for the user interaction) if the initial input (in some commands) will be the symbol/word at point (even if it is not a valid global's simbol, this can be improved latter). I mean like when using ffap and counsel-find-file. Elisp already has some internal api for that in thingatpt.el and there is also ivy-thing-at-point.
Maybe we could ask for help to @abo-abo if there is a bigger issue, he is very kind with collaborators, pull requests and issues fixing.
There is also another option: in the ggtags package they deal with these issue pretty fine in tramp and locally, so maybe we can use their code, even if it creates a dependency with ggtags, It may not be a big issue and it will prevent you to reinvent the wheel and to have more code to maintain yourself.
Very thanks for supporting this, Ergus
Ok, will update the code to make it more maintainable.
For the "pick up what's under cursor" as initial input, it'd be best that we do it on a per-case basis. So open an issue. It's not complicated to do, but I don't want to work on something that nobody's requesting. Each is really quick to do, so no external help is needed. It's just that there are many functions and things-at-point to support.
The code update I mentioned is agnostic to Tramp (filtering will be done by global
on remote machine, no need for rg
or grep
to filter). On the same matter, ggtags has different goals that get in the way of adding counsel
support, see https://github.com/leoliu/ggtags/issues/175#issuecomment-428986369
Hi Felipe:
I am testing the latest changes you did, but I only get this candidates repeated many times: With find definition I only get:
Usage: global [-adEFGilMnNqrstTvx][-S dir][-e] pattern
global -c[dFiIMoOPrsT] prefix
global -f[adlnqrstvx][-L file-list][-S dir] files
global -g[aEGilMnoOqtvVx][-L file-list][-S dir][-e] pattern [files]
global -I[ailMnqtvx][-S dir][-e] pattern
global -P[aEGilMnoOqtvVx][-S dir][-e] pattern
global -p[qrv]
global -u[qv]
./src/dispextern.h:2217:struct it
If I try to open the "valid" file candidate I get:
ivy-call: Symbol’s value as variable is void: line
in the message buffer. I am not trying with tramp, this is localy only. But also in find-definition the tagname before counsel-gtags--select-file is empty.
I just found that the problem is that I was looking for a very small character (it in the src/xdisp.c) I forgot that ivy-more-chars-alist only has 2 for counsel-grep.
But the other error (void: line) is still there.
It would help a lot if you could attach complete stack trace.
Thanks for reporting.
Hi felipe I can do it tomorrow, but now I see that the problems are that :
counsel-gtags--select-file gets always an empty candidate form counsel-gtags--collect-candidates because global-args (in the last one) looks like: --result=grep --path-style=through ./src/buffer.h:483:struct buffer. when it was supposed to be: --result=grep buffer --path-style=through ./src/buffer.h:483
This is because the tagname when we call: (counsel-gtags--select-file 'definition tagname) is something like: ./src/buffer.h:483:struct buffer
I don't know why in this point it is needed to do another call to global if there is already a valid candidate selected that was already passed to select-file. counsel-gtags--select-file already receives a candidate. with file and line number.
I see a second prompt for a pattern that is not needed, or I don't understant.
Maybe find-definition should call find-file instead of select-file? Or counsel-gtags--build-command-to-collect-candidates needs to add a -c and pass throw process-line:
Debugger entered--Lisp error: (void-variable line) counsel-gtags--find-file("") ivy-call() ivy-read("Pattern: " nil :action counsel-gtags--find-file :caller counsel-gtags--select-file) counsel-gtags--select-file(definition "./src/buffer.h:483:struct buffer") counsel-gtags-find-definition("./src/buffer.h:483:struct buffer") funcall-interactively(counsel-gtags-find-definition "./src/buffer.h:483:struct buffer") call-interactively(counsel-gtags-find-definition nil nil) command-execute(counsel-gtags-find-definition)
The PR #4 solved the local issues for me, but for some reason they don't pass the github tests. In tram mode there is still the issue that the file path doesn't contain the /ssh:server: part. This is easy to solve, but I don't have time right now.
From https://github.com/FelipeLema/emacs-counsel-gtags/issues/1#issuecomment-480073233
@Ergus, I was using
grep
to filter the results, but now that I have a better knowledge of Ivy, I understand this is not the encouraged way to go.The correct way to approach this is to do "X more chars to go", translating the input as regex and let
global
do the filtering, but I first wanted to confirm with someone else using this package (you) first.Are you (and anyone reading this) OK with changing behaviour and force user input for tags?