Re: INSIDE_EMACS and Tramp

[Federico Tedin <federicotedin@gmail.com>]

Hey Michael,

> You're right, `run-hooks' was a stupid idea. I have adapted my proposal
> and prepared a patch which works for me as expected. See appended.

Thanks for creating this patch! I have a couple of
comments/observations. Note that this is my first time reviewing a patch
by mail so formatting may not be very tidy.

> diff --git a/lisp/comint.el b/lisp/comint.el
> index ea06f8af87..1045d02641 100644
> --- a/lisp/comint.el
> +++ b/lisp/comint.el
> @@ -831,7 +831,7 @@ comint-exec-1
>    (let ((process-environment
>  	 (nconc
>            (comint-term-environment)
> -	  (list (format "INSIDE_EMACS=%s,comint" emacs-version))
> +	  (list (inside-emacs-envvar "comint"))
>  	  process-environment))
>  	(default-directory
>  	  (if (file-accessible-directory-p default-directory)
> diff --git a/lisp/epg.el b/lisp/epg.el
> index 222fd913e1..a5c06c9ae8 100644
> --- a/lisp/epg.el
> +++ b/lisp/epg.el
> @@ -606,8 +606,7 @@ epg--start
>  	    (cons (concat "GPG_TTY=" terminal-name)
>  		  (cons "TERM=xterm" process-environment))))
>      (setq process-environment
> -	  (cons (format "INSIDE_EMACS=%s,epg" emacs-version)
> -		process-environment))
> +	  (cons (inside-emacs-envvar "epg") process-environment))
>      ;; Record modified time of gpg-agent socket to restore the Emacs
>      ;; frame on text terminal in `epg-wait-for-completion'.
>      ;; See
> diff --git a/lisp/eshell/em-dirs.el b/lisp/eshell/em-dirs.el
> index 51df6fa1d5..7218279e08 100644
> --- a/lisp/eshell/em-dirs.el
> +++ b/lisp/eshell/em-dirs.el
> @@ -168,9 +168,6 @@ eshell-dirstack
>  (defvar eshell-last-dir-ring nil
>    "The last directory that Eshell was in.")
>
> -(defconst eshell-inside-emacs (format "%s,eshell" emacs-version)
> -  "Value for the `INSIDE_EMACS' environment variable.")
> -
>  ;;; Functions:
>
>  (defun eshell-dirs-initialize ()    ;Called from `eshell-mode' via intern-soft!
> @@ -195,7 +192,8 @@ eshell-dirs-initialize
>  			  (expand-file-name
>  			   (ring-ref eshell-last-dir-ring 0))))
>              t)
> -           ("INSIDE_EMACS" eshell-inside-emacs
> +           ("INSIDE_EMACS" ,(lambda (_indices)
> +                              (inside-emacs "eshell"))
>              t))))
>
>    (when eshell-cd-on-directory
> diff --git a/lisp/net/tramp-sh.el b/lisp/net/tramp-sh.el
> index a39d503e22..414fca10cc 100644
> --- a/lisp/net/tramp-sh.el
> +++ b/lisp/net/tramp-sh.el
> @@ -4166,10 +4166,10 @@ tramp-open-shell
>         vec (format
>  	    (eval-when-compile
>  	      (concat
> -	       "exec env TERM='%s' INSIDE_EMACS='%s,tramp:%s' "
> +	       "exec env TERM='%s' INSIDE_EMACS='%s' "
>  	       "ENV=%s %s PROMPT_COMMAND='' PS1=%s PS2='' PS3='' %s %s"))
>              tramp-terminal-type
> -            emacs-version tramp-version  ; INSIDE_EMACS
> +            (inside-emacs)
>              (or (getenv-internal "ENV" tramp-remote-process-environment) "")
>  	    (if (stringp tramp-histfile-override)
>  		(format "HISTFILE=%s"
> diff --git a/lisp/net/trampver.el b/lisp/net/trampver.el
> index 8d21133b3b..2351867129 100644
> --- a/lisp/net/trampver.el
> +++ b/lisp/net/trampver.el
> @@ -76,6 +76,13 @@ tramp-repository-version
>              (replace-regexp-in-string "\n" "" (emacs-version))))))
>    (unless (string-equal "ok" x) (error "%s" x)))
>
> +;; `inside-emacs-functions' has been introduces with Emacs 28.1.
                                        ^^^^^^^^^^
                                           typo

> +(when (symbolp inside-emacs-functions)
> +  (add-hook
> +   'inside-emacs-functions
> +   (lambda ()
> +     (when (file-remote-p default-directory) "tramp:2.5.0-pre"))))
                                                ^^^^^^^^^^^^^^^^^

Should this be `tramp-version' instead?

> +
>  ;; Tramp versions integrated into Emacs.  If a user option declares a
>  ;; `:package-version' which doesn't belong to an integrated Tramp
>  ;; version, it must be added here as well (see `tramp-syntax', for
> diff --git a/lisp/progmodes/compile.el b/lisp/progmodes/compile.el
> index a76a3c44a3..65c5257d93 100644
> --- a/lisp/progmodes/compile.el
> +++ b/lisp/progmodes/compile.el
> @@ -1823,7 +1823,7 @@ compilation-start
>  	     (append
>  	      compilation-environment
>                (comint-term-environment)
> -	      (list (format "INSIDE_EMACS=%s,compile" emacs-version))
> +	      (list (inside-emacs-envvar "compile"))
>  	      (copy-sequence process-environment))))
>  	(set (make-local-variable 'compilation-arguments)
>  	     (list command mode name-function highlight-regexp))
> diff --git a/lisp/simple.el b/lisp/simple.el
> index b5ba05426f..70621225ec 100644
> --- a/lisp/simple.el
> +++ b/lisp/simple.el
> @@ -4295,6 +4295,29 @@ list-processes
>      (tabulated-list-print))
>    (display-buffer buffer)
>    nil)
> +
> +(defvar inside-emacs-functions nil
> +  "List of functions to compose the environment variable INSIDE_EMACS.
> +Every package which needs to set a string in that envirenmont
                                                     ^^^^^^^^^^^
                                                        typo

> +variable shall add a function without arguments, which returns
> +the respective string, or nil.")
> +
> +(defun inside-emacs (&optional context)
> +  "Return the string to be set in environment variable INSIDE_EMACS.
> +CONTEXT could be a string which is added."
> +  (mapconcat
> +   #'identity
> +   (delq nil (append `(,emacs-version
> + 	               ,(and (stringp context) context))
> + 	             (mapcar #'funcall inside-emacs-functions)))
> +   ","))
> +
> +(defun inside-emacs-envvar (&optional context)
> +  "Return the \"INSIDE_EMACS=...\" string.
> +This can be used to modify the `process-environment'.  See
> +`inside-emacs' for the CONTEXT parameter."
> +  (concat "INSIDE_EMACS=" (inside-emacs context)))
> +
>  \f
>  ;;;; Prefix commands
>
> diff --git a/lisp/term.el b/lisp/term.el
> index b990c83cfc..d6a1bdfd1f 100644
> --- a/lisp/term.el
> +++ b/lisp/term.el
> @@ -1535,7 +1535,7 @@ term-exec-1
>  	   (format term-termcap-format "TERMCAP="
>  		   term-term-name term-height term-width)
>
> -	   (format "INSIDE_EMACS=%s,term:%s" emacs-version term-protocol-version)
> +	   (inside-emacs-envvar (format "term:%s" term-protocol-version))
>  	   (format "LINES=%d" term-height)
>  	   (format "COLUMNS=%d" term-width))
>  	  process-environment))

- Fede