bug#31355: 26.1; [PATCH] Fix remote-host/tramp directory tracking for shells in `term' buffers

bug#31355: 26.1; [PATCH] Fix remote-host/tramp directory tracking for shells in `term' buffers

[Phil Sainty]

[-- Attachment #1: Type: text/plain, Size: 3649 bytes --]

With tramp methods now being mandatory, the term.el directory tracking
was generating invalid tramp paths on remote hosts.

I've rolled a patch for this (along with some very minor improvements
to the related commentary) in which I'm hard-coding the "ssh" method.
I'm not sure whether that's the ideal fix, but I suspect it's likely
to do the trick for most people, whereas the current code won't work
at all.


-Phil




In GNU Emacs 26.1 (build 1, x86_64-pc-linux-gnu, X toolkit, Xaw3d scroll 
bars)
  of 2018-04-15 built on shodan
Windowing system distributor 'The X.Org Foundation', version 
11.0.11804000
System Description:	Ubuntu 16.04.4 LTS

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
uncompressing term.el.gz...done
C-c i is undefined
Quit [3 times]
Making completion list... [3 times]
Quit [4 times]
GNU Emacs 26.1 (build 1, x86_64-pc-linux-gnu, X toolkit, Xaw3d scroll 
bars) of 2018-04-15

Configured using:
  'configure --prefix=/home/phil/emacs/26.1rc1/usr/local
  --with-x-toolkit=lucid --without-sound'

Configured features:
XAW3D XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK DBUS GSETTINGS NOTIFY
GNUTLS LIBXML2 FREETYPE XFT ZLIB TOOLKIT_SCROLL_BARS LUCID X11 THREADS
LCMS2

Important settings:
   value of $LANG: en_NZ.UTF-8
   locale-coding-system: utf-8

Major mode: Dired by name

Minor modes in effect:
   show-paren-mode: t
   minibuffer-depth-indicate-mode: t
   winner-mode: t
   global-hl-line-mode: t
   tooltip-mode: t
   global-eldoc-mode: t
   electric-indent-mode: t
   mouse-wheel-mode: t
   menu-bar-mode: t
   file-name-shadow-mode: t
   global-font-lock-mode: t
   font-lock-mode: t
   auto-composition-mode: t
   auto-encryption-mode: t
   auto-compression-mode: t
   buffer-read-only: t
   line-number-mode: t
   transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr imenu misearch multi-isearch jka-compr thingatpt
find-func emacsbug message rmc puny seq byte-opt gv bytecomp
byte-compile cconv cl-loaddefs cl-lib format-spec rfc822 mml mml-sec
password-cache epa derived epg epg-config gnus-util rmail rmail-loaddefs
mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils
mailheader sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr
mail-utils dired-x easymenu paren mb-depth winner ring hl-line dired
dired-loaddefs advice elec-pair time-date mule-util tooltip eldoc
electric uniquify ediff-hook vc-hooks lisp-float-type mwheel term/x-win
x-win term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode elisp-mode lisp-mode
prog-mode register page menu-bar rfn-eshadow isearch timer select
scroll-bar mouse jit-lock font-lock syntax facemenu font-core
term/tty-colors frame cl-generic cham georgian utf-8-lang misc-lang
vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms cp51932
hebrew greek romanian slovak czech european ethiopic indian cyrillic
chinese composite charscript charprop case-table epa-hook jka-cmpr-hook
help simple abbrev obarray minibuffer cl-preloaded nadvice loaddefs
button faces cus-face macroexp files text-properties overlay sha1 md5
base64 format env code-pages mule custom widget hashtable-print-readable
backquote dbusbind inotify lcms2 dynamic-setting system-font-setting
font-render-setting x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 16 108228 20488)
  (symbols 48 20930 1)
  (miscs 40 423 129)
  (strings 32 31412 1231)
  (string-bytes 1 820395)
  (vectors 16 15792)
  (vector-slots 8 508466 10704)
  (floats 8 60 351)
  (intervals 56 1335 16)
  (buffers 992 17)
  (heap 1024 17853 1684))

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-remote-host-directory-tracking-for-shells-in-ter.patch --]
[-- Type: text/x-diff; name=0001-Fix-remote-host-directory-tracking-for-shells-in-ter.patch, Size: 2878 bytes --]

From 4ae811e050902280f6eff19541db4b32f4848190 Mon Sep 17 00:00:00 2001
From: Phil Sainty <psainty@orcon.net.nz>
Date: Fri, 4 May 2018 01:29:42 +1200
Subject: [PATCH] Fix remote-host directory tracking for shells in `term'
 buffers

* lisp/term.el (term-handle-ansi-terminal-messages): Use explicit
"ssh" tramp method when constructing the tramp path for a non-local
host, as a method is now mandatory.

Minor related improvements to the commentary.
---
 lisp/term.el | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/lisp/term.el b/lisp/term.el
index 75c2779..a970570 100644
--- a/lisp/term.el
+++ b/lisp/term.el
@@ -233,31 +233,33 @@
 ;;
 ;;  Notice: for directory/host/user tracking you need to have something
 ;; like this in your shell startup script (this is for a POSIXish shell
-;; like Bash but should be quite easy to port to other shells)
+;; like Bash but should be quite easy to port to other shells).
+;;
+;; For troubleshooting in Bash, you can check the definition of the
+;; custom functions with the "type" command.  e.g. "type cd".  If you
+;; do not see the expected definition from the config below, then the
+;; directory tracking will not work.
 ;;
 ;;             ----------------------------------------
 ;;
-;;  # Set HOSTNAME if not already set.
+;;	# Set HOSTNAME if not already set.
 ;;	: ${HOSTNAME=$(uname -n)}
 ;;
-;;  # su does not change this but I'd like it to
-;;
+;;	# su does not change this but I'd like it to
 ;;	USER=$(whoami)
 ;;
-;;  # ...
+;;	# ...
 ;;
 ;;	case $TERM in
 ;;	    eterm*)
 ;;
 ;;		printf '%s\n' \
 ;;		 -------------------------------------------------------------- \
-;;		 "Hello $user" \
+;;		 "Hello $USER" \
 ;;		 "Today is $(date)" \
 ;;		 "We are on $HOSTNAME running $(uname) under Emacs term mode" \
 ;;		 --------------------------------------------------------------
 ;;
-;;		export EDITOR=emacsclient
-;;
 ;;		# The \033 stands for ESC.
 ;;		# There is a space between "AnSiT?" and $whatever.
 ;;
@@ -269,10 +271,11 @@
 ;;		printf '\033AnSiTh %s\n' "$HOSTNAME"
 ;;		printf '\033AnSiTu %s\n' "$USER"
 ;;
-;;		eval $(dircolors $HOME/.emacs_dircolors)
+;;		# Use custom dircolors in term buffers.
+;;		# eval $(dircolors $HOME/.emacs_dircolors)
 ;;	esac
 ;;
-;;  # ...
+;;	# ...
 ;;
 ;;
 
@@ -2753,8 +2756,8 @@ term-handle-ansi-terminal-messages
 					(string= term-ansi-at-user (user-real-login-name)))
 		   (expand-file-name term-ansi-at-dir)
 		 (if (string= term-ansi-at-user (user-real-login-name))
-		     (concat "/" term-ansi-at-host ":" term-ansi-at-dir)
-		   (concat "/" term-ansi-at-user "@" term-ansi-at-host ":"
+                     (concat "/ssh:" term-ansi-at-host ":" term-ansi-at-dir)
+                   (concat "/ssh:" term-ansi-at-user "@" term-ansi-at-host ":"
 			   term-ansi-at-dir)))))
 
 	;; I'm not sure this is necessary,
-- 
2.8.3

bug#31355: 26.1; [PATCH] Fix remote-host/tramp directory tracking for shells in `term' buffers

[Phil Sainty]

Or if I'm reading the NEWS correctly:

> *** The new pseudo method "-" is a marker for the default method.
> "/-::" is the shortest remote file name then.

I should actually have made the following change?

-                     (concat "/" term-ansi-at-host ":" 
term-ansi-at-dir)
-                   (concat "/" term-ansi-at-user "@" term-ansi-at-host 
":"
+                     (concat "/-:" term-ansi-at-host ":" 
term-ansi-at-dir)
+                   (concat "/-:" term-ansi-at-user "@" 
term-ansi-at-host ":"

bug#31355: 26.1; [PATCH] Fix remote-host/tramp directory tracking for shells in `term' buffers

[Michael Albinus]

Phil Sainty <psainty@orcon.net.nz> writes:

Hi Phil,

> With tramp methods now being mandatory, the term.el directory tracking
> was generating invalid tramp paths on remote hosts.
>
> I've rolled a patch for this (along with some very minor improvements
> to the related commentary) in which I'm hard-coding the "ssh" method.
> I'm not sure whether that's the ideal fix, but I suspect it's likely
> to do the trick for most people, whereas the current code won't work
> at all.

Thanks for this. I haven't tested, just a short comment:

> @@ -2753,8 +2756,8 @@ term-handle-ansi-terminal-messages
>  					(string= term-ansi-at-user (user-real-login-name)))
>  		   (expand-file-name term-ansi-at-dir)
>  		 (if (string= term-ansi-at-user (user-real-login-name))
> -		     (concat "/" term-ansi-at-host ":" term-ansi-at-dir)
> -		   (concat "/" term-ansi-at-user "@" term-ansi-at-host ":"
> +                     (concat "/ssh:" term-ansi-at-host ":" term-ansi-at-dir)
> +                   (concat "/ssh:" term-ansi-at-user "@" term-ansi-at-host ":"

I believe it is always save to use term-ansi-at-user. There's no need to
check, whether it is equal to (user-real-login-name).

Furthermore, I wouldn't hard-code "ssh". People might have other
preferences, and on some systems it doesn't work. Think about Emacs
running on MS WIndows, where "plink" is used instead of "ssh". Just take
"-" as method, this is a notation to apply user's default (Tramp) method.

> -Phil

Best regards, Michael.

bug#31355: 26.1; [PATCH] Fix remote-host/tramp directory tracking for shells in `term' buffers

[Phil Sainty]

[-- Attachment #1: Type: text/plain, Size: 1065 bytes --]

On 2018-05-04 02:04, Michael Albinus wrote:
> Furthermore, I wouldn't hard-code "ssh". People might have other
> preferences, and on some systems it doesn't work. Think about Emacs
> running on MS WIndows, where "plink" is used instead of "ssh". Just 
> take
> "-" as method, this is a notation to apply user's default (Tramp) 
> method.

Revised patch attached to use the "-" pseudo method.


> I believe it is always save to use term-ansi-at-user. There's no need
> to check, whether it is equal to (user-real-login-name).

I've not changed this.  The code is evidentially deciding whether the
user/host pair being tracked in the shell in the term buffer is a match
for the user/host pair known locally to Emacs.  Unless both of those
components match, a tramp path is created.  I've not tested the case
of a different user on the local host, but it seems like reasonable
logic to me, at first look.

(if (and (string= term-ansi-at-host (system-name))
          (string= term-ansi-at-user (user-real-login-name)))
     ;; Use a local path
   ;; Use a tramp path


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-remote-host-directory-tracking-for-shells-in-ter.patch --]
[-- Type: text/x-diff; name=0001-Fix-remote-host-directory-tracking-for-shells-in-ter.patch, Size: 2930 bytes --]

From 4c3620d3a420b3ed617e95d127ce5e5333ceda61 Mon Sep 17 00:00:00 2001
From: Phil Sainty <psainty@orcon.net.nz>
Date: Fri, 4 May 2018 01:29:42 +1200
Subject: [PATCH] Fix remote-host directory tracking for shells in `term'
 buffers

* lisp/term.el (term-handle-ansi-terminal-messages): Use an explicit
tramp method when constructing the tramp path for a non-local host,
as this is now mandatory.  "-" is a pseudo-method for the user's
`tramp-default-method'.

Minor related improvements to the commentary.
---
 lisp/term.el | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/lisp/term.el b/lisp/term.el
index 75c2779..2fbc796 100644
--- a/lisp/term.el
+++ b/lisp/term.el
@@ -233,31 +233,33 @@
 ;;
 ;;  Notice: for directory/host/user tracking you need to have something
 ;; like this in your shell startup script (this is for a POSIXish shell
-;; like Bash but should be quite easy to port to other shells)
+;; like Bash but should be quite easy to port to other shells).
+;;
+;; For troubleshooting in Bash, you can check the definition of the
+;; custom functions with the "type" command.  e.g. "type cd".  If you
+;; do not see the expected definition from the config below, then the
+;; directory tracking will not work.
 ;;
 ;;             ----------------------------------------
 ;;
-;;  # Set HOSTNAME if not already set.
+;;	# Set HOSTNAME if not already set.
 ;;	: ${HOSTNAME=$(uname -n)}
 ;;
-;;  # su does not change this but I'd like it to
-;;
+;;	# su does not change this but I'd like it to
 ;;	USER=$(whoami)
 ;;
-;;  # ...
+;;	# ...
 ;;
 ;;	case $TERM in
 ;;	    eterm*)
 ;;
 ;;		printf '%s\n' \
 ;;		 -------------------------------------------------------------- \
-;;		 "Hello $user" \
+;;		 "Hello $USER" \
 ;;		 "Today is $(date)" \
 ;;		 "We are on $HOSTNAME running $(uname) under Emacs term mode" \
 ;;		 --------------------------------------------------------------
 ;;
-;;		export EDITOR=emacsclient
-;;
 ;;		# The \033 stands for ESC.
 ;;		# There is a space between "AnSiT?" and $whatever.
 ;;
@@ -269,10 +271,11 @@
 ;;		printf '\033AnSiTh %s\n' "$HOSTNAME"
 ;;		printf '\033AnSiTu %s\n' "$USER"
 ;;
-;;		eval $(dircolors $HOME/.emacs_dircolors)
+;;		# Use custom dircolors in term buffers.
+;;		# eval $(dircolors $HOME/.emacs_dircolors)
 ;;	esac
 ;;
-;;  # ...
+;;	# ...
 ;;
 ;;
 
@@ -2753,8 +2756,8 @@ term-handle-ansi-terminal-messages
 					(string= term-ansi-at-user (user-real-login-name)))
 		   (expand-file-name term-ansi-at-dir)
 		 (if (string= term-ansi-at-user (user-real-login-name))
-		     (concat "/" term-ansi-at-host ":" term-ansi-at-dir)
-		   (concat "/" term-ansi-at-user "@" term-ansi-at-host ":"
+                     (concat "/-:" term-ansi-at-host ":" term-ansi-at-dir)
+                   (concat "/-:" term-ansi-at-user "@" term-ansi-at-host ":"
 			   term-ansi-at-dir)))))
 
 	;; I'm not sure this is necessary,
-- 
2.8.3

bug#31355: 26.1; [PATCH] Fix remote-host/tramp directory tracking for shells in `term' buffers

[Michael Albinus]

Phil Sainty <psainty@orcon.net.nz> writes:

Hi Phil,

>> I believe it is always save to use term-ansi-at-user. There's no need
>> to check, whether it is equal to (user-real-login-name).
>
> I've not changed this.  The code is evidentially deciding whether the
> user/host pair being tracked in the shell in the term buffer is a match
> for the user/host pair known locally to Emacs.  Unless both of those
> components match, a tramp path is created.  I've not tested the case
> of a different user on the local host, but it seems like reasonable
> logic to me, at first look.

Of course.

> (if (and (string= term-ansi-at-host (system-name))
>          (string= term-ansi-at-user (user-real-login-name)))
>     ;; Use a local path

OK.

>   ;; Use a tramp path

And here I believe you could always apply

(format "/-:%s@%s:%s" term-ansi-at-user term-ansi-at-host term-ansi-at-dir)

Best regards, Michael.

bug#31355: 26.1; [PATCH] Fix remote-host/tramp directory tracking for shells in `term' buffers

[Phil Sainty]

On 2018-05-04 02:59, Michael Albinus wrote:
> And here I believe you could always apply
> 
> (format "/-:%s@%s:%s" term-ansi-at-user term-ansi-at-host 
> term-ansi-at-dir)

Right, although I believe the intent is simply to produce the least-
verbose path by excluding the username if possible (as indeed the user
would very likely be doing if entering the tramp path manually); so
I'd be inclined to leave that part alone as well.

bug#31355: 26.1; [PATCH] Fix remote-host/tramp directory tracking for shells in `term' buffers

[Michael Albinus]

Phil Sainty <psainty@orcon.net.nz> writes:

Hi Phil,

>> And here I believe you could always apply
>>
>> (format "/-:%s@%s:%s" term-ansi-at-user term-ansi-at-host
>> term-ansi-at-dir)
>
> Right, although I believe the intent is simply to produce the least-
> verbose path by excluding the username if possible (as indeed the user
> would very likely be doing if entering the tramp path manually); so
> I'd be inclined to leave that part alone as well.

I don't understand the benefit of the "least-verbose path". The user
hasn't to type anything, so there's no profit to have a shorter file name.

And you cannot expect, that for all Tramp methods the default user is
equal to `user-login-name'. This happened to ange-ftp, but it isn't the
case in Tramp. See variables `tramp-default-user' and
`tramp-default-user-alist'.

Best regards, Michael.

bug#31355: 26.1; [PATCH] Fix remote-host/tramp directory tracking for shells in `term' buffers

[Phil Sainty]

[-- Attachment #1: Type: text/plain, Size: 384 bytes --]

On 2018-05-04 19:27, Michael Albinus wrote:
> And you cannot expect, that for all Tramp methods the default user is
> equal to `user-login-name'. This happened to ange-ftp, but it isn't the
> case in Tramp. See variables `tramp-default-user' and
> `tramp-default-user-alist'.

You're right, the explicit username is more robust.

Attaching a new patch with that fix included.


-Phil

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-remote-host-directory-tracking-for-shells-in-ter.patch --]
[-- Type: text/x-diff; name=0001-Fix-remote-host-directory-tracking-for-shells-in-ter.patch, Size: 3292 bytes --]

From 806ab6f4ba730b26c05f3469a64da3a2223b32c5 Mon Sep 17 00:00:00 2001
From: Phil Sainty <psainty@orcon.net.nz>
Date: Fri, 4 May 2018 01:29:42 +1200
Subject: [PATCH] Fix remote-host directory tracking for shells in `term'
 buffers

* lisp/term.el (term-handle-ansi-terminal-messages): Use an explicit
tramp method when constructing the tramp path for a non-local host,
as this is now mandatory.  "-" is a pseudo-method for the user's
`tramp-default-method'.

Specify the remote username explicitly in all cases, as
`tramp-default-user' and `tramp-default-user-alist' could cause the
previous logic to fail.

Minor related improvements to the commentary.
---
 lisp/term.el | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/lisp/term.el b/lisp/term.el
index 75c2779..419ddb2 100644
--- a/lisp/term.el
+++ b/lisp/term.el
@@ -233,31 +233,33 @@
 ;;
 ;;  Notice: for directory/host/user tracking you need to have something
 ;; like this in your shell startup script (this is for a POSIXish shell
-;; like Bash but should be quite easy to port to other shells)
+;; like Bash but should be quite easy to port to other shells).
+;;
+;; For troubleshooting in Bash, you can check the definition of the
+;; custom functions with the "type" command.  e.g. "type cd".  If you
+;; do not see the expected definition from the config below, then the
+;; directory tracking will not work.
 ;;
 ;;             ----------------------------------------
 ;;
-;;  # Set HOSTNAME if not already set.
+;;	# Set HOSTNAME if not already set.
 ;;	: ${HOSTNAME=$(uname -n)}
 ;;
-;;  # su does not change this but I'd like it to
-;;
+;;	# su does not change this but I'd like it to
 ;;	USER=$(whoami)
 ;;
-;;  # ...
+;;	# ...
 ;;
 ;;	case $TERM in
 ;;	    eterm*)
 ;;
 ;;		printf '%s\n' \
 ;;		 -------------------------------------------------------------- \
-;;		 "Hello $user" \
+;;		 "Hello $USER" \
 ;;		 "Today is $(date)" \
 ;;		 "We are on $HOSTNAME running $(uname) under Emacs term mode" \
 ;;		 --------------------------------------------------------------
 ;;
-;;		export EDITOR=emacsclient
-;;
 ;;		# The \033 stands for ESC.
 ;;		# There is a space between "AnSiT?" and $whatever.
 ;;
@@ -269,10 +271,11 @@
 ;;		printf '\033AnSiTh %s\n' "$HOSTNAME"
 ;;		printf '\033AnSiTu %s\n' "$USER"
 ;;
-;;		eval $(dircolors $HOME/.emacs_dircolors)
+;;		# Use custom dircolors in term buffers.
+;;		# eval $(dircolors $HOME/.emacs_dircolors)
 ;;	esac
 ;;
-;;  # ...
+;;	# ...
 ;;
 ;;
 
@@ -2750,12 +2753,10 @@ term-handle-ansi-terminal-messages
 	(setq default-directory
 	      (file-name-as-directory
 	       (if (and (string= term-ansi-at-host (system-name))
-					(string= term-ansi-at-user (user-real-login-name)))
+                        (string= term-ansi-at-user (user-real-login-name)))
 		   (expand-file-name term-ansi-at-dir)
-		 (if (string= term-ansi-at-user (user-real-login-name))
-		     (concat "/" term-ansi-at-host ":" term-ansi-at-dir)
-		   (concat "/" term-ansi-at-user "@" term-ansi-at-host ":"
-			   term-ansi-at-dir)))))
+                 (concat "/-:" term-ansi-at-user "@" term-ansi-at-host ":"
+                         term-ansi-at-dir))))
 
 	;; I'm not sure this is necessary,
 	;; but it's best to be on the safe side.
-- 
2.8.3

bug#31355: 26.1; [PATCH] Fix remote-host/tramp directory tracking for shells in `term' buffers

[Michael Albinus]

Phil Sainty <psainty@orcon.net.nz> writes:

Hi Phil,

> Attaching a new patch with that fix included.

LGTM, could go into the emacs-26 branch after release of 26.1. Do you
have git push permissions, or shall I do it for you?

> -Phil

Best regards, Michael.

bug#31355: 26.1; [PATCH] Fix remote-host/tramp directory tracking for shells in `term' buffers

[Phil Sainty]

On 2018-05-09 05:43, Michael Albinus wrote:
> LGTM, could go into the emacs-26 branch after release of 26.1.
> Do you have git push permissions, or shall I do it for you?

I do.  For the moment I've pushed it to origin/fix/bug-31355.

cheers,
-Phil

bug#31355: 26.1; [PATCH] Fix remote-host/tramp directory tracking for shells in `term' buffers

[Phil Sainty]

On 2018-05-09 05:43, Michael Albinus wrote:
> LGTM, could go into the emacs-26 branch after release of 26.1.

Pushed to emacs-26 in commit 03697e648c.

-Phil