emacs-helm / helm-system-packages

A Helm interface to the package manager of your operating system
GNU General Public License v3.0
106 stars 11 forks source link

Remove default-directory prefix when running find-files on remote #18

Closed Ambrevar closed 6 years ago

Ambrevar commented 6 years ago

Mentioned in #15:

Find files works fine on the local machine, but on the remote one the current directory is prepending to the paths e.g. /ssh:user@host:/home/user//usr/bin/etc. Ideally we need to strip /home/user from the path.

It comes from the compatibility change with helm-ff--create-tramp-name for Emacs<26, but I'm not sure what's the ideal way to fix it.

@@ -383,15 +383,7 @@ Otherwise display in `helm-system-packages-buffer'."
   "Prefix FILE with path to remote connection.
 If local, return FILE unmodified."
   (if (tramp-tramp-file-p default-directory)
-      (let ((v (tramp-dissect-file-name default-directory)))
-        (tramp-make-tramp-file-name
-         (tramp-file-name-method v)
-         (tramp-file-name-user v)
-         (tramp-file-name-domain v)
-         (tramp-file-name-host v)
-         (tramp-file-name-port v)
-         file
-         (tramp-file-name-hop v)))
+      (helm-ff--create-tramp-name (concat default-directory file))
     file))

Before the patch, it used to work for Emacs 26 but not on Emacs<=25. With the patch, the default-directory file path is now used in the result, while we only want the default-directory protocol + user + host + etc., which we prepent to the file. @thierryvolpiatto?

thierryvolpiatto commented 6 years ago

Pierre Neidhardt notifications@github.com writes:

Mentioned in #15:

Find files works fine on the local machine, but on the remote one the current
directory is prepending to the paths
e.g. /ssh:user@host:/home/user//usr/bin/etc. Ideally we need to strip
/home/user from the path.

Yes see below.

It comes from the compatibility change with helm-ff--create-tramp-name for Emacs<26, but I'm not sure what's the ideal way to fix it.

[...]

  • (helm-ff--create-tramp-name (concat default-directory file))

Not verified but I guess this expand to something like:

(helm-ff--create-tramp-name
  "/ssh:host:/home/you/somewhere//somewhere_else/file")

Perhaps you should use only file or (expand-file-name file default-directory)

NOTE: I am not sure how work helm-system-packages on remote files, but helm-ff--create-tramp-name is an internal function that have been written for HFF, perhaps you need a variation of it adapted to your needs, this is just a thought, not verified, perhaps it just works fine for you.

-- Thierry

Ambrevar commented 6 years ago

e6ce15c5c83148279549c4d9aa16848c1b96cbb4 should fix this.

I got rid of helm-ff--create-tramp-name which does not allow for separating the default-directory from the protocol.

@thierryvolpiatto: Your initial suggestion of using helm-ff--create-tramp-name solves a different compatibility issue that arose with Emacs 25.

For Helm System Packages, the issue is with Emacs 26 where the signature of tramp-make-tramp-file-name has changed. This probably means that helm-ff--create-tramp-name does not work properly on Emacs 26.

thierryvolpiatto commented 6 years ago

Pierre Neidhardt notifications@github.com writes:

e6ce15c should fix this.

I got rid of helm-ff--create-tramp-name which does not allow for separating the default-directory from the protocol.

@thierryvolpiatto: Your initial suggestion of using helm-ff--create-tramp-name solves a different compatibility issue that arose with Emacs 25.

For Helm System Packages, the issue is with Emacs 26 where the signature of tramp-make-tramp-file-name has changed. This probably means that helm-ff--create-tramp-name does not work properly on Emacs 26.

It does, the purpose of this function is to work with both signatures. As I said this function is an internal function for helm-find-files, if it doesn't work for you please write one adapted to your needs.

-- Thierry

Ambrevar commented 6 years ago

I understand: the documentation of `tramp-dissect-file-name' is wrong on Emacs 26. I'll report upstream if not already fixed.

Ambrevar commented 6 years ago

Fixed upstream: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=30904