FelipeLema / emacs-counsel-gtags

GNU Global with ivy completion
22 stars 5 forks source link

Tramp issues #1

Closed Ergus closed 5 years ago

Ergus commented 5 years ago

Hi: The package is not working with tramp. I just tried ggtags and it is working perfectly, so I think you can fix the issue with their code approach to retrieve and call remote processes.

Right now when I try to find a definition I get this in the Message buffer:

Tramp: Opening connection for mn2 using ssh...
Tramp: Sending command ‘exec ssh -q    -e none mn2’
Tramp: Waiting for prompts from remote shell...failed
Tramp: Opening connection for mn2 using ssh...failed
ivy-done: Wrong type argument: char-or-string-p, nil

(mn2 is the server I am connected to.)

Actually it prompts to ask the definition with the right completion but after pressing enter I just get the prompt again, but empty.

FelipeLema commented 5 years ago

I know I may be asking for too much, but a self contained test would help (it'll probably involve something like ssh localhost for simplicity).

See tests/unit-tests.el

Ergus commented 5 years ago

Hi: I think that the issue actually will be not exposed by a connection to localhost because the issue is the remote execution paths (default-directory).

Actually I am pretty sure that the everything happens because the function counsel-gtags--collect-candidates uses process-lines which internally uses call-process instead of process-file (bad name election which could produce confution). So the commands in tramp for remote files will be executed in the user's local home directory instead of the remote host.

I was looking for an alternative to process-lines (like process-file is for call-process) but I can't find any, so basically it will be needed to copy process-lines and implement a version in your code.

Ergus commented 5 years ago

Hi Felipe:

Very thanks for the fix it is better now, but still not working right for me.

Now I only get one ivy candidate:

sh: /usr/bin/rg: No such file or directory

This is because remotely the rg program is in a different place, but such a place is in the PATH because I set it in ~/.profile (I use counsel-rg constantly and it works fine).

I made a test: As I know that only grep is in the standard location /usr/bin/grep; I moved "grep" before the other candidates in line 182 and everything worked.

So in this case it looks like rg was detected by executable-find because it was in the remote PATH, but then, for some reason, the execution appended something else and tried to find it in the remote /usr/bin/ instead of its real path.

So there are 2 things to fix:

1) The issue with the path

2) The output when there is an error shouldn't be shown as an ivy candidate, but as a message.

The first one may be probably simple, but the second I have no idea because I don't undertand the ivy API.

Once again very thanks for all this. Ergus

Ergus commented 5 years ago

Hi Felipe: Very very sorry for bothering so much, but this issue (where everything started) is still open. In tramp files the files are open in the local machine, so in case of ssh it creates a new directory in the local machine.

the function counsel-gtags--find-file should add a condition with buffer-file-name like:

(and buffer-file-name
        (tramp-tramp-file-p buffer-file-name))

to test tthe filename associated to the visiting buffer (because the global candidates look like local paths) and then, if the buffer-file-name is a tramp's path, add the tramp remote part if it to the file that is gonna be open.

FelipeLema commented 5 years ago

no problem in re-opening issues, but do mind that it may take me a while to address certain issues.

Also, in case it isn't clear. I will accept PRs if you add a unit test for added code and travis shows tests are clean

Ergus commented 5 years ago

Hi Felipe:

For me it is working pretty fine!!. Very thanks.

I am reading the changes In the latest commit and in this function:

(defun counsel-gtags-find-file (&optional filename)
  "Search/narrow for FILENAME among tagged files."
  (interactive)
  (apply 'ivy-read candidates
     (plist-put
      (counsel-gtags--find-file-ivy-parameters filename)
      :caller 'counsel-gtags-find-file-name)
     :initial-input default-file))

doesn't work.

candidates and default-file are not declared.

FelipeLema commented 5 years ago

That was the sole function I didn't test. Sorry about that

Ergus commented 5 years ago

You don't have to say sorry, I should for bothering so much. :P

The byte compiler made me figure out that:

(defun counsel-gtags--select-file-ivy-parameters (type tagname extra-options auto-select-only-candidate)
  "Get `counsel-gtags--select-file' the parameters from TYPE to call `ivy-read'."
  (if (string-empty-p tagname)
      (message "No candidate tags")
    (let* ((root (counsel-gtags--default-directory))
           (encoding buffer-file-coding-system)
           (default-directory root)
           (collection (counsel-gtags--collect-candidates
            type tagname encoding extra-options))
           (ivy-auto-select-single-candidate t) ;; see issue #7
           )
      `("Pattern: " ,collection
        :action counsel-gtags--find-file))))

The variable ivy-auto-select-single-candidate maybe should be set to auto-select-only-candidate?

FelipeLema commented 5 years ago

auto-select-only-candidate shouldn't be forwarded, and ivy-auto-select-single-candidate should be set in counsel-gtags--select-file instead of there

Ergus commented 5 years ago

But then the if in counsel-gtags--select-file will be not needed because ivy can do it out of the box right?

Ergus commented 5 years ago

Hi Felipe:

I just pushed a commit in my fork with a very small change. The problem was that ivy was not supposed to be called at all if the tagname was empty, and we were doing that because the ivy call was moved. I also deleted the uneded condition because ivy can do that for us, so the code gets a bit simpler there.

I can't run the test for localhost, but for normal remote servers it is working for me after my changes. But try the localhost test you wrote.

I made a direct call to ivy-read because with the current method I don't see why we need to make it more complicated using apply+plist-put. The ivy api is simple and flexible enough for most of code. And it is better if we can simplify things.

Finally I deleted the get-grep function we were not using, because it was still there with no use.

Hope you can use my changes, but I won't do a pull request, because I diden't run the last test.

So please give a look and if you find something useful impl

FelipeLema commented 5 years ago

I gave you some feedback.

If you can't run tests yourself, you can do all development in a specific branch, push your changes and watch travis build for that branch. If you can't see the travis logs, let me know.