ivy TRAMP navigation from arbitrary directory #283

Closed
ghost opened this Issue Nov 5, 2015 · 9 comments

Comments

Projects
None yet
2 participants
@ghost

ghost commented Nov 5, 2015

When using find-file or counsel-find-file to navigate through TRAMP-supported protocols, ivy doesn't seem to bring up any completion matches for filesystem navigation, unless I start at / on my local machine.

To reproduce (with /etc as current working directory): C-x C-f <backspace> sudo::/ <tab>
Additionally (any other directory than / as cwd): C-x C-f //sudo::/ <tab>

The above displays no matches. An additional <tab> will result in navigation to the dired buffer for / (as root@machine). C-j and C-m do the same.

The following works (with / as the current working directory): C-x C-f sudo::/ <tab>

I see issue #241, which seems to be similar, but I don't see a solution that works for me there. I do get completion by using the single / or three slash prefix /// mentioned there, but it comes up in a separate Completions buffer instead of through ivy for some reason.

abo-abo added a commit that referenced this issue Nov 5, 2015

Fix /ssh: and /sudo:: broken in 71695df
* ivy.el (ivy-alt-done): `file-directory-p' errors when given "/ssh:" or
  "/sudo::".

Re #283
@abo-abo

This comment has been minimized.

Show comment
Hide comment
@abo-abo

abo-abo Nov 5, 2015

Owner

Thanks for noticing, the behavior broke 2 weeks ago. You can get into TRAMP from any directory with /ssh: followed by RET or C-m. You also can get into sudo from any directory with /sudo: or /sudo:: followed by RET or C-m. I think the behavior for /sudo:: can be improved to move you to the current directory instead of root's home directory. I'll make that change shortly.

Owner

abo-abo commented Nov 5, 2015

Thanks for noticing, the behavior broke 2 weeks ago. You can get into TRAMP from any directory with /ssh: followed by RET or C-m. You also can get into sudo from any directory with /sudo: or /sudo:: followed by RET or C-m. I think the behavior for /sudo:: can be improved to move you to the current directory instead of root's home directory. I'll make that change shortly.

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Nov 5, 2015

Aha, I'm starting to see what it is doing. The additional slash in ivy is required to indicate a TRAMP path. Your solution does work from any directory.

This works for me, but there's possibly two things that might make it better:

C-x C-f /sudo::/ <tab> shouldn't pop up a Completions window. (I was doing that from habit)

Backspacing to the root directory / or navigating with // shouldn't list "sudo:" and others as possible completions. That made it confusing seeing as how normal Emacs works that way, and then ivy was listing them as possibilities, but not having the expected behavior.

ghost commented Nov 5, 2015

Aha, I'm starting to see what it is doing. The additional slash in ivy is required to indicate a TRAMP path. Your solution does work from any directory.

This works for me, but there's possibly two things that might make it better:

C-x C-f /sudo::/ <tab> shouldn't pop up a Completions window. (I was doing that from habit)

Backspacing to the root directory / or navigating with // shouldn't list "sudo:" and others as possible completions. That made it confusing seeing as how normal Emacs works that way, and then ivy was listing them as possibilities, but not having the expected behavior.

abo-abo added a commit that referenced this issue Nov 5, 2015

@abo-abo

This comment has been minimized.

Show comment
Hide comment
@abo-abo

abo-abo Nov 5, 2015

Owner

Backspacing to the root directory / or navigating with // shouldn't list "sudo:" and others as possible completions.

They are listed by Emacs itself, I don't want to interfere. The problem is that Ivy doesn't have proper handlers for every item of // yet. They can be added in the future, eventually.

Owner

abo-abo commented Nov 5, 2015

Backspacing to the root directory / or navigating with // shouldn't list "sudo:" and others as possible completions.

They are listed by Emacs itself, I don't want to interfere. The problem is that Ivy doesn't have proper handlers for every item of // yet. They can be added in the future, eventually.

abo-abo added a commit that referenced this issue Nov 5, 2015

Fix /ssh: and /sudo:: broken in 71695df
* ivy.el (ivy-alt-done): `file-directory-p' errors when given "/ssh:" or
  "/sudo::".

Re #283

abo-abo added a commit that referenced this issue Nov 5, 2015

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Nov 5, 2015

The problem is that Ivy doesn't have proper handlers for every item of // yet. They can be added in the future, eventually.

Hmm.. I'm not seeing why those two cases should be treated differently though. ivy should pass the same value in both cases.

Bad (from /etc): C-x C-f <backspace> sudo::/ <tab> <tab>
Good (from /): C-x C-f sudo::/ <tab> <tab>

I looked through the code and this might be incorrect. Based on the next expression, it looks like file-directory-p was supposed to wrap around (I'm thinking it could be a paredit-style typo?):

(and
 (not (equal ivy-text ""))
 (file-directory-p ivy-text)
 (ignore-errors
   (file-directory-p ivy-text))
 (setq dir (expand-file-name
            ivy-text ivy--directory)))

And what is actually wanted is:

(and
 (not (equal ivy-text ""))
 (file-directory-p
  (setq dir (expand-file-name
             ivy-text ivy--directory))))

Otherwise file-directory-p is just based on the CWD and ivy-text instead of ivy--directory -- which explains why it only works when / is the CWD.

Not sure if I've misevaluated or if it causes other bugs, since I'm not very familiar with the codebase, but basic testing seems to work for both the above cases. // to jump to root and then use tramp also works now based on the suggested completions (without any special cases needed for //). -- I believe this is also what the OP of the previous issue was wanting for the behavior. @PythonNut, can you confirm?

ghost commented Nov 5, 2015

The problem is that Ivy doesn't have proper handlers for every item of // yet. They can be added in the future, eventually.

Hmm.. I'm not seeing why those two cases should be treated differently though. ivy should pass the same value in both cases.

Bad (from /etc): C-x C-f <backspace> sudo::/ <tab> <tab>
Good (from /): C-x C-f sudo::/ <tab> <tab>

I looked through the code and this might be incorrect. Based on the next expression, it looks like file-directory-p was supposed to wrap around (I'm thinking it could be a paredit-style typo?):

(and
 (not (equal ivy-text ""))
 (file-directory-p ivy-text)
 (ignore-errors
   (file-directory-p ivy-text))
 (setq dir (expand-file-name
            ivy-text ivy--directory)))

And what is actually wanted is:

(and
 (not (equal ivy-text ""))
 (file-directory-p
  (setq dir (expand-file-name
             ivy-text ivy--directory))))

Otherwise file-directory-p is just based on the CWD and ivy-text instead of ivy--directory -- which explains why it only works when / is the CWD.

Not sure if I've misevaluated or if it causes other bugs, since I'm not very familiar with the codebase, but basic testing seems to work for both the above cases. // to jump to root and then use tramp also works now based on the suggested completions (without any special cases needed for //). -- I believe this is also what the OP of the previous issue was wanting for the behavior. @PythonNut, can you confirm?

@abo-abo

This comment has been minimized.

Show comment
Hide comment
@abo-abo

abo-abo Nov 5, 2015

Owner

Bad (from /etc): C-x C-f sudo::/

You're using the compatibility bindings here, left just in case the recommended method breaks.

The recommended method is

  • for sudo: C-x C-f /sudo:: C-j.
  • for ssh: C-x C-f /ssh: C-j.

What I meant by // not working is e.g. C-x C-f // sud C-j.

In any case, if there's something still missing in your opinion, send a PR, it's easier to look at the code.

Owner

abo-abo commented Nov 5, 2015

Bad (from /etc): C-x C-f sudo::/

You're using the compatibility bindings here, left just in case the recommended method breaks.

The recommended method is

  • for sudo: C-x C-f /sudo:: C-j.
  • for ssh: C-x C-f /ssh: C-j.

What I meant by // not working is e.g. C-x C-f // sud C-j.

In any case, if there's something still missing in your opinion, send a PR, it's easier to look at the code.

@ghost ghost referenced this issue Nov 5, 2015

Closed

Fix directory validity check #284

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Nov 5, 2015

Bad (from /etc): C-x C-f sudo::/

Not sure if that's a mistyping, but I don't mean sudo::/ in the /etc directory. I mean accessing the qualified "/sudo::/" while the current working directory is /etc. It happens that you can construct this path quickly with // in ivy though. Or a bunch of backspaces. I'm not sure if it's a compatibility mode, but it's the one I always ran before.

What I meant by // not working is e.g. C-x C-f // sud C-j.

This does work at the moment, but your current working directory has to be /. -- Just to add, by current working directory I don't mean anything to do with the ivy prompt. Rather the CWD of the buffer that you're coming from.

Went ahead and submitted a PR with the proposed change.

ghost commented Nov 5, 2015

Bad (from /etc): C-x C-f sudo::/

Not sure if that's a mistyping, but I don't mean sudo::/ in the /etc directory. I mean accessing the qualified "/sudo::/" while the current working directory is /etc. It happens that you can construct this path quickly with // in ivy though. Or a bunch of backspaces. I'm not sure if it's a compatibility mode, but it's the one I always ran before.

What I meant by // not working is e.g. C-x C-f // sud C-j.

This does work at the moment, but your current working directory has to be /. -- Just to add, by current working directory I don't mean anything to do with the ivy prompt. Rather the CWD of the buffer that you're coming from.

Went ahead and submitted a PR with the proposed change.

@PythonNut

This comment has been minimized.

Show comment
Hide comment
@PythonNut

PythonNut Nov 5, 2015

Contributor

I believe this is also what the OP of the previous issue was wanting for the behavior. @PythonNut, can you confirm?

Hi all. I'm was basically asking about the following:

C-x C-f //ssh:user@host: C-j seems to prompt for the password and open a dired buffer on the remote host (which is what vanilla emacs would do). From the description, I expected it to prompt for the password and continue completion.

From any other directory, it works as expected.

Contributor

PythonNut commented Nov 5, 2015

I believe this is also what the OP of the previous issue was wanting for the behavior. @PythonNut, can you confirm?

Hi all. I'm was basically asking about the following:

C-x C-f //ssh:user@host: C-j seems to prompt for the password and open a dired buffer on the remote host (which is what vanilla emacs would do). From the description, I expected it to prompt for the password and continue completion.

From any other directory, it works as expected.

@abo-abo abo-abo closed this in 6666cf8 Nov 5, 2015

@abo-abo

This comment has been minimized.

Show comment
Hide comment
@abo-abo

abo-abo Nov 5, 2015

Owner

@PythonNut If I understood correctly, what you want to do is not to open a dired buffer, but continue with completion by pressing C-j.
This is the default behavior, or at least it should be; if you have any other behavior report a bug.

What you need to type is C-x C-f /ssh:user@host: C-j. Note the absence of the leading slash before ssh. What I usually do is just C-x C-f /ssh: C-j, since then I get host completion.

Owner

abo-abo commented Nov 5, 2015

@PythonNut If I understood correctly, what you want to do is not to open a dired buffer, but continue with completion by pressing C-j.
This is the default behavior, or at least it should be; if you have any other behavior report a bug.

What you need to type is C-x C-f /ssh:user@host: C-j. Note the absence of the leading slash before ssh. What I usually do is just C-x C-f /ssh: C-j, since then I get host completion.

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Nov 6, 2015

Hey, submitted a PR (#285) with better TRAMP support for /protocol: as well as /path/to//protocol:. From what I can tell, they would support all same features, including host completion (minus the quick //sudo:: going to the current directory, since that is probably not wanted for /sudo::).

C-x C-f //ssh:user@host: C-j seems to prompt for the password and open a dired buffer on the remote host (which is what vanilla emacs would do). From the description, I expected it to prompt for the password and continue completion.

That should be fixed in 6666cf8 above. The problem was that it was checking the directory relative to the CWD, instead of what you had in the minibuffer. This was causing it to fail a conditional and go to the catch-all (ivy-done) at the bottom.

I can perform your key sequence and get the correct result with the version in MELPA. Does your sequence now work as expected for you?

ghost commented Nov 6, 2015

Hey, submitted a PR (#285) with better TRAMP support for /protocol: as well as /path/to//protocol:. From what I can tell, they would support all same features, including host completion (minus the quick //sudo:: going to the current directory, since that is probably not wanted for /sudo::).

C-x C-f //ssh:user@host: C-j seems to prompt for the password and open a dired buffer on the remote host (which is what vanilla emacs would do). From the description, I expected it to prompt for the password and continue completion.

That should be fixed in 6666cf8 above. The problem was that it was checking the directory relative to the CWD, instead of what you had in the minibuffer. This was causing it to fail a conditional and go to the catch-all (ivy-done) at the bottom.

I can perform your key sequence and get the correct result with the version in MELPA. Does your sequence now work as expected for you?

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