unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#25496: 25.1.91; INSIDE_EMACS env variable is not set in eshell
@ 2017-01-20 22:32 Alex Hutcheson
  2017-01-23 20:20 ` Glenn Morris
  2020-03-08 16:50 ` Federico Tedin
  0 siblings, 2 replies; 14+ messages in thread
From: Alex Hutcheson @ 2017-01-20 22:32 UTC (permalink / raw)
  To: 25496

The INSIDE_EMACS environmental variable is set for comint processes,
including M-x shell and ansi-term. However, it's not set for eshell or
for processes launched by eshell. This makes it difficult for scripts to
detect whether or not they are being run inside eshell.

The INSIDE_EMACS env variable should be set within eshell, or should at
least be set for processes launched by eshell.

In GNU Emacs 25.1.91.1 (x86_64-pc-linux-gnu, X toolkit, Xaw3d scroll bars),  
modified by Debian
Windowing system distributor 'The X.Org Foundation', version 11.0.11501000
System Description:	Ubuntu 14.04 LTS

Configured using:
  'configure --build x86_64-linux-gnu --build x86_64-linux-gnu
  --prefix=/usr --sharedstatedir=/var/lib --libexecdir=/usr/lib
  --localstatedir=/var/lib --infodir=/usr/share/info
  --mandir=/usr/share/man --with-pop=yes
   
--enable-locallisppath=/etc/google-emacs:/etc/emacs:/usr/local/share/emacs/25.1.91+gg1+1/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/25.1.91+gg1+1/site-lisp:/usr/share/emacs/site-lisp
  --with-crt-dir=/usr/lib/x86_64-linux-gnu --disable-build-details
  --disable-silent-rules --with-modules GOOGLE_VERSION=25.1.91+gg1+1
  --with-x=yes --with-x-toolkit=lucid --with-toolkit-scroll-bars
  --without-gconf --without-gsettings build_alias=x86_64-linux-gnu
  'CFLAGS=-g -O2 -fstack-protector --param=ssp-buffer-size=4 -Wformat
  -Werror=format-security -Wall' 'LDFLAGS=-Wl,-Bsymbolic-functions
  -Wl,-z,relro -Wl,-fuse-ld=gold,--export-dynamic-symbol=__google_auxv'
  'CPPFLAGS=-D_FORTIFY_SOURCE=2 -DGOOGLE_EMACS_DEFINE_AUXV''

Configured features:
XAW3D XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS NOTIFY
LIBSELINUX GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB
TOOLKIT_SCROLL_BARS LUCID X11 MODULES

Important settings:
   value of $LANG: en_US.UTF-8
   value of $XMODIFIERS: @im=ibus
   locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
   tooltip-mode: t
   global-eldoc-mode: t
   electric-indent-mode: t
   mouse-wheel-mode: t
   tool-bar-mode: t
   menu-bar-mode: t
   file-name-shadow-mode: t
   global-font-lock-mode: t
   font-lock-mode: t
   blink-cursor-mode: t
   auto-composition-mode: t
   auto-encryption-mode: t
   auto-compression-mode: t
   line-number-mode: t
   transient-mark-mode: t

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message dired format-spec rfc822 mml
mml-sec password-cache epg epg-config gnus-util mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail
rfc2047 rfc2045 ietf-drums mm-util help-fns help-mode easymenu
cl-loaddefs pcase cl-lib mail-prsvr mail-utils time-date mule-util
tooltip eldoc electric uniquify ediff-hook vc-hooks lisp-float-type
mwheel x-win term/common-win x-dnd tool-bar dnd fontset image regexp-opt
fringe tabulated-list newcomment elisp-mode lisp-mode prog-mode register
page menu-bar rfn-eshadow timer select scroll-bar mouse jit-lock
font-lock syntax facemenu font-core 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 charscript case-table epa-hook
jka-cmpr-hook help simple abbrev 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 dynamic-setting
font-render-setting x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 16 87104 6318)
  (symbols 48 19929 0)
  (miscs 40 51 110)
  (strings 32 15098 4711)
  (string-bytes 1 425436)
  (vectors 16 11832)
  (vector-slots 8 435700 4672)
  (floats 8 168 31)
  (intervals 56 262 0)
  (buffers 976 19)
  (heap 1024 12629 768))





^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#25496: 25.1.91; INSIDE_EMACS env variable is not set in eshell
  2017-01-20 22:32 bug#25496: 25.1.91; INSIDE_EMACS env variable is not set in eshell Alex Hutcheson
@ 2017-01-23 20:20 ` Glenn Morris
  2017-01-23 20:36   ` Alex Hutcheson
  2020-03-08 16:50 ` Federico Tedin
  1 sibling, 1 reply; 14+ messages in thread
From: Glenn Morris @ 2017-01-23 20:20 UTC (permalink / raw)
  To: Alex Hutcheson; +Cc: 25496

Alex Hutcheson wrote:

> The INSIDE_EMACS environmental variable is set for comint processes,
> including M-x shell and ansi-term. However, it's not set for eshell or
> for processes launched by eshell. This makes it difficult for scripts to
> detect whether or not they are being run inside eshell.

Could you give some examples of where this matters?
It never has been set for eshell, AFAIK.

When I think of INSIDE_EMACS usage, the only thing that comes to mind is
interactive bash, which obviously isn't relevant for eshell (as a bash
replacement).

> The INSIDE_EMACS env variable should be set within eshell, or should at
> least be set for processes launched by eshell.





^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#25496: 25.1.91; INSIDE_EMACS env variable is not set in eshell
  2017-01-23 20:20 ` Glenn Morris
@ 2017-01-23 20:36   ` Alex Hutcheson
  0 siblings, 0 replies; 14+ messages in thread
From: Alex Hutcheson @ 2017-01-23 20:36 UTC (permalink / raw)
  To: Glenn Morris; +Cc: 25496

> Could you give some examples of where this matters?
> It never has been set for eshell, AFAIK.

Here's the case I'm running into:
Some command line programs that I work with (version control systems, as
well as other scripts) allow the user to specify a merge tool that gets called
for merge conflicts. I prefer to use ediff as my merge tool, so I have a simple
bash script that launches emacsclient and calls ediff with the appropriate
arguments.

My desired behavior is this:
if INSIDE_EMACS:
  Run an ediff session within the existing Emacs instance
else:
  Create a new frame and launch the ediff session in that

I frequently work by SSHing into my workstation and using emacsclient to
connect to the Emacs instance running on the workstation. I run all my shell
commands in M-x shell or eshell. If I run `emacsclient -c' in one of
these shells,
it creates a graphical frame on my workstation's desktop, which I can't see over
SSH. Instead, I check for INSIDE_EMACS, and if I detect that my shell is running
within Emacs I call emacsclient without -c.


On Mon, Jan 23, 2017 at 3:20 PM, Glenn Morris <rgm@gnu.org> wrote:
> Alex Hutcheson wrote:
>
>> The INSIDE_EMACS environmental variable is set for comint processes,
>> including M-x shell and ansi-term. However, it's not set for eshell or
>> for processes launched by eshell. This makes it difficult for scripts to
>> detect whether or not they are being run inside eshell.
>
> Could you give some examples of where this matters?
> It never has been set for eshell, AFAIK.
>
> When I think of INSIDE_EMACS usage, the only thing that comes to mind is
> interactive bash, which obviously isn't relevant for eshell (as a bash
> replacement).
>
>> The INSIDE_EMACS env variable should be set within eshell, or should at
>> least be set for processes launched by eshell.



-- 
Alex Hutcheson
alexhutcheson@google.com





^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#25496: 25.1.91; INSIDE_EMACS env variable is not set in eshell
  2017-01-20 22:32 bug#25496: 25.1.91; INSIDE_EMACS env variable is not set in eshell Alex Hutcheson
  2017-01-23 20:20 ` Glenn Morris
@ 2020-03-08 16:50 ` Federico Tedin
  2020-03-28 15:05   ` Federico Tedin
  2020-03-29  0:21   ` Noam Postavsky
  1 sibling, 2 replies; 14+ messages in thread
From: Federico Tedin @ 2020-03-08 16:50 UTC (permalink / raw)
  To: 25496

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

Didn't realize that Bug#39596 was merged with this one, I'll attach my
patch here just in case.

- Fede


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Copy-INSIDE_EMACS-env-variable-to-subprocesses-in-Es.patch --]
[-- Type: text/x-patch, Size: 4158 bytes --]

From cf93960de7d3d92ccb7a5465851bbcb55a0f20d9 Mon Sep 17 00:00:00 2001
From: Federico Tedin <federicotedin@gmail.com>
Date: Wed, 19 Feb 2020 00:11:35 +0100
Subject: [PATCH] Copy INSIDE_EMACS env variable to subprocesses in Eshell

* lisp/eshell/em-dirs.el (eshell-dirs-initialize): Add INSIDE_EMACS
variable to buffer-local value of eshell-variable-aliases-alist.
* lisp/eshell/esh-var.el (eshell-variable-aliases-list): Update doc
string; remove mention of eshell-user-aliases-list and explain that
variables can optionally be copied to subprocesses' environments.
* test/lisp/eshell/eshell-tests.el (eshell-test/inside-emacs-var): Add
test for the INSIDE_EMACS variable.
* etc/NEWS: Announce changes.
---
 etc/NEWS                         |  6 ++++++
 lisp/eshell/em-dirs.el           |  3 +++
 lisp/eshell/esh-var.el           | 11 ++++++-----
 test/lisp/eshell/eshell-tests.el |  9 ++++++++-
 4 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 1f8e6049a8..238184df24 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -120,6 +120,12 @@ To revert to the previous behaviour,
 unconditionally aborts the current edebug instrumentation with the
 supplied error message.
 
+** Eshell
+
+---
+*** Environment variable INSIDE_EMACS is now copied to subprocesses.
+Its value equals the result of evaluating '(format "%s,eshell" emacs-version)'.
+
 ** Tramp
 
 +++
diff --git a/lisp/eshell/em-dirs.el b/lisp/eshell/em-dirs.el
index 1949e5dc8f..b478ee028a 100644
--- a/lisp/eshell/em-dirs.el
+++ b/lisp/eshell/em-dirs.el
@@ -191,6 +191,9 @@ eshell-dirs-initialize
 		        (unless (ring-empty-p eshell-last-dir-ring)
 			  (expand-file-name
 			   (ring-ref eshell-last-dir-ring 0))))
+            t)
+           ("INSIDE_EMACS" ,(lambda (_indices)
+                              (format "%s,eshell" emacs-version))
             t))))
 
   (when eshell-cd-on-directory
diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el
index 327a1def46..4231be92fb 100644
--- a/lisp/eshell/esh-var.el
+++ b/lisp/eshell/esh-var.el
@@ -179,10 +179,8 @@ eshell-variable-aliases-list
 	      (eshell-apply-indices eshell-command-arguments
 				    indices)))))
   "This list provides aliasing for variable references.
-It is very similar in concept to what `eshell-user-aliases-list' does
-for commands.  Each member of this defines the name of a command,
-and the Lisp value to return for that variable if it is accessed
-via the syntax `$NAME'.
+Each member of this defines the name of a command, and the Lisp value
+to return for that variable if it is accessed via the syntax `$NAME'.
 
 If the value is a function, that function will be called with two
 arguments: the list of the indices that was used in the reference, and
@@ -190,7 +188,10 @@ eshell-variable-aliases-list
 For example, a reference of `$NAME[10][20]' would result in the
 function for alias `NAME' being called (assuming it were aliased to a
 function), and the arguments passed to this function would be the list
-'(10 20)', and nil."
+'(10 20)', and nil.
+
+Additionally, each member may specify if it should be copied to the
+environment of created subprocesses."
   :type '(repeat (list string sexp
 		       (choice (const :tag "Copy to environment" t)
 			       (const :tag "Use only in Eshell" nil)))))
diff --git a/test/lisp/eshell/eshell-tests.el b/test/lisp/eshell/eshell-tests.el
index 026818ab06..f98e4ce92b 100644
--- a/test/lisp/eshell/eshell-tests.el
+++ b/test/lisp/eshell/eshell-tests.el
@@ -167,7 +167,14 @@ eshell-test/last-arg-var
   "Test using the \"last arg\" ($_) variable"
   (with-temp-eshell
    (eshell-command-result-p "+ 1 2; + $_ 4"
-                             "3\n6\n")))
+                            "3\n6\n")))
+
+(ert-deftest eshell-test/inside-emacs-var ()
+  "Test presence of \"INSIDE_EMACS\" in subprocesses"
+  (with-temp-eshell
+   (eshell-command-result-p "env"
+                            (format "INSIDE_EMACS=%s,eshell"
+                                    emacs-version))))
 
 (ert-deftest eshell-test/escape-nonspecial ()
   "Test that \"\\c\" and \"c\" are equivalent when \"c\" is not a
-- 
2.21.1 (Apple Git-122.3)


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* bug#25496: 25.1.91; INSIDE_EMACS env variable is not set in eshell
  2020-03-08 16:50 ` Federico Tedin
@ 2020-03-28 15:05   ` Federico Tedin
  2020-03-29  0:21   ` Noam Postavsky
  1 sibling, 0 replies; 14+ messages in thread
From: Federico Tedin @ 2020-03-28 15:05 UTC (permalink / raw)
  To: Federico Tedin; +Cc: 25496

Federico Tedin <federicotedin@gmail.com> writes:

> Didn't realize that Bug#39596 was merged with this one, I'll attach my
> patch here just in case.
>
> - Fede

Pinging this thread again in case a maintainer can take a look at the
patch I submitted on my last message.

Thanks!





^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#25496: 25.1.91; INSIDE_EMACS env variable is not set in eshell
  2020-03-08 16:50 ` Federico Tedin
  2020-03-28 15:05   ` Federico Tedin
@ 2020-03-29  0:21   ` Noam Postavsky
  2020-03-29 22:29     ` Federico Tedin
  1 sibling, 1 reply; 14+ messages in thread
From: Noam Postavsky @ 2020-03-29  0:21 UTC (permalink / raw)
  To: Federico Tedin; +Cc: 25496

Federico Tedin <federicotedin@gmail.com> writes:

> Pinging this thread again in case a maintainer can take a look at the
> patch I submitted on my last message.

Thanks for the ping, I had forgotten about this one.

> * lisp/eshell/em-dirs.el (eshell-dirs-initialize): Add INSIDE_EMACS
> variable to buffer-local value of eshell-variable-aliases-alist.
> * lisp/eshell/esh-var.el (eshell-variable-aliases-list): Update doc
> string; remove mention of eshell-user-aliases-list and explain that

I think eshell-user-aliases-list was meant to refer to
eshell-command-aliases-list (though maybe the reference isn't really
needed anyway).

> +           ("INSIDE_EMACS" ,(lambda (_indices)
> +                              (format "%s,eshell" emacs-version))

Since emacs-version generally doesn't change during an Emacs session, is
there any need for a lambda at all here?

>    "This list provides aliasing for variable references.
> -It is very similar in concept to what `eshell-user-aliases-list' does
> -for commands.  Each member of this defines the name of a command,
> -and the Lisp value to return for that variable if it is accessed
> -via the syntax `$NAME'.
> +Each member of this defines the name of a command, and the Lisp value
               ^^^^^^^                       ^^^^^^^
> +to return for that variable if it is accessed via the syntax `$NAME'.

(These problems weren't introduced by your patch, but) I think the "of
this" is redundant, and it should say "variable" instead of "command".





^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#25496: 25.1.91; INSIDE_EMACS env variable is not set in eshell
  2020-03-29  0:21   ` Noam Postavsky
@ 2020-03-29 22:29     ` Federico Tedin
  2020-03-30  1:44       ` Noam Postavsky
  0 siblings, 1 reply; 14+ messages in thread
From: Federico Tedin @ 2020-03-29 22:29 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: 25496

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

Thanks for the feedback!

I've removed the lambda, but I wasn't able to use a string directly, I
had to set the value elsewhere using a defconst. This is due to how
`eshell-get-variable' interprets string values associated to
variables. I'm attaching a new patch.

Thanks


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Copy-INSIDE_EMACS-env-variable-to-subprocesses-in-Es.patch --]
[-- Type: text/x-diff, Size: 4415 bytes --]

From bc6319ca49241c9e270ba0b6f939766dcf078e25 Mon Sep 17 00:00:00 2001
From: Federico Tedin <federicotedin@gmail.com>
Date: Mon, 30 Mar 2020 00:22:27 +0200
Subject: [PATCH] Copy INSIDE_EMACS env variable to subprocesses in Eshell

* lisp/eshell/em-dirs.el (eshell-dirs-initialize): Add INSIDE_EMACS
variable to buffer-local value of eshell-variable-aliases-alist.
(eshell-inside-emacs): Add new constant used for INSIDE_EMACS.
* lisp/eshell/esh-var.el (eshell-variable-aliases-list): Update doc
string; remove mention of eshell-user-aliases-list and explain that
variables can optionally be copied to subprocesses' environments.
* test/lisp/eshell/eshell-tests.el (eshell-test/inside-emacs-var): Add
test for the INSIDE_EMACS variable.
* etc/NEWS: Announce changes.
---
 etc/NEWS                         |  6 ++++++
 lisp/eshell/em-dirs.el           |  5 +++++
 lisp/eshell/esh-var.el           | 11 ++++++-----
 test/lisp/eshell/eshell-tests.el |  7 +++++++
 4 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 4b477e5def..9ac90ecabf 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -161,6 +161,12 @@ functions accessible to the user through the existing single function hook
 Modes should use the new hook instead of this user option to register
 their backends.
 
+** Eshell
+
+---
+*** Environment variable INSIDE_EMACS is now copied to subprocesses.
+Its value equals the result of evaluating '(format "%s,eshell" emacs-version)'.
+
 ** Tramp
 
 +++
diff --git a/lisp/eshell/em-dirs.el b/lisp/eshell/em-dirs.el
index 1949e5dc8f..51df6fa1d5 100644
--- a/lisp/eshell/em-dirs.el
+++ b/lisp/eshell/em-dirs.el
@@ -168,6 +168,9 @@ 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!
@@ -191,6 +194,8 @@ eshell-dirs-initialize
 		        (unless (ring-empty-p eshell-last-dir-ring)
 			  (expand-file-name
 			   (ring-ref eshell-last-dir-ring 0))))
+            t)
+           ("INSIDE_EMACS" eshell-inside-emacs
             t))))
 
   (when eshell-cd-on-directory
diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el
index 327a1def46..0f23e61e33 100644
--- a/lisp/eshell/esh-var.el
+++ b/lisp/eshell/esh-var.el
@@ -179,10 +179,8 @@ eshell-variable-aliases-list
 	      (eshell-apply-indices eshell-command-arguments
 				    indices)))))
   "This list provides aliasing for variable references.
-It is very similar in concept to what `eshell-user-aliases-list' does
-for commands.  Each member of this defines the name of a command,
-and the Lisp value to return for that variable if it is accessed
-via the syntax `$NAME'.
+Each member defines the name of a variable, and the Lisp value to
+return for that variable if it is accessed via the syntax `$NAME'.
 
 If the value is a function, that function will be called with two
 arguments: the list of the indices that was used in the reference, and
@@ -190,7 +188,10 @@ eshell-variable-aliases-list
 For example, a reference of `$NAME[10][20]' would result in the
 function for alias `NAME' being called (assuming it were aliased to a
 function), and the arguments passed to this function would be the list
-'(10 20)', and nil."
+'(10 20)', and nil.
+
+Additionally, each member may specify if it should be copied to the
+environment of created subprocesses."
   :type '(repeat (list string sexp
 		       (choice (const :tag "Copy to environment" t)
 			       (const :tag "Use only in Eshell" nil)))))
diff --git a/test/lisp/eshell/eshell-tests.el b/test/lisp/eshell/eshell-tests.el
index 026818ab06..ce8d728833 100644
--- a/test/lisp/eshell/eshell-tests.el
+++ b/test/lisp/eshell/eshell-tests.el
@@ -169,6 +169,13 @@ eshell-test/last-arg-var
    (eshell-command-result-p "+ 1 2; + $_ 4"
                              "3\n6\n")))
 
+(ert-deftest eshell-test/inside-emacs-var ()
+  "Test presence of \"INSIDE_EMACS\" in subprocesses"
+  (with-temp-eshell
+   (eshell-command-result-p "env"
+                            (format "INSIDE_EMACS=%s,eshell"
+                                    emacs-version))))
+
 (ert-deftest eshell-test/escape-nonspecial ()
   "Test that \"\\c\" and \"c\" are equivalent when \"c\" is not a
 special character."
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* bug#25496: 25.1.91; INSIDE_EMACS env variable is not set in eshell
  2020-03-29 22:29     ` Federico Tedin
@ 2020-03-30  1:44       ` Noam Postavsky
  2020-03-30 19:52         ` Federico Tedin
  0 siblings, 1 reply; 14+ messages in thread
From: Noam Postavsky @ 2020-03-30  1:44 UTC (permalink / raw)
  To: Federico Tedin; +Cc: 25496

Federico Tedin <federicotedin@gmail.com> writes:

> I've removed the lambda, but I wasn't able to use a string directly, I
> had to set the value elsewhere using a defconst. This is due to how
> `eshell-get-variable' interprets string values associated to
> variables.

Oh, a string is actually taken as an environment variable to alias.  I
guess that does justify the name.  I think we might actually be better
off with the lambda, what do you think?

>    "This list provides aliasing for variable references.
> -It is very similar in concept to what `eshell-user-aliases-list' does
> -for commands.  Each member of this defines the name of a command,
> -and the Lisp value to return for that variable if it is accessed
> -via the syntax `$NAME'.
> +Each member defines the name of a variable, and the Lisp value to
> +return for that variable if it is accessed via the syntax `$NAME'.

Again, not a problem introduced by your patch, but "Lisp value to
return" seem pretty inaccurate (if you don't feel like fixing this, it's
okay, I think describing it correctly is somewhat non-trivial).






^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#25496: 25.1.91; INSIDE_EMACS env variable is not set in eshell
  2020-03-30  1:44       ` Noam Postavsky
@ 2020-03-30 19:52         ` Federico Tedin
  2020-03-30 23:11           ` Noam Postavsky
  0 siblings, 1 reply; 14+ messages in thread
From: Federico Tedin @ 2020-03-30 19:52 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: 25496

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

>> I've removed the lambda, but I wasn't able to use a string directly, I
>> had to set the value elsewhere using a defconst. This is due to how
>> `eshell-get-variable' interprets string values associated to
>> variables.
>
> Oh, a string is actually taken as an environment variable to alias.  I
> guess that does justify the name.  I think we might actually be better
> off with the lambda, what do you think?

Hmm, actually now it's working fine - the value associated with
INSIDE_EMACS is the symbol `eshell-inside-emacs'.  Then
`eshell-get-variable' is returning that symbol's value, which is what we
want. The `$$', `$?' and `$0' variables are also set to symbols.

(But the lambda would also work, of course. No preferences here.)

>>    "This list provides aliasing for variable references.
>> -It is very similar in concept to what `eshell-user-aliases-list' does
>> -for commands.  Each member of this defines the name of a command,
>> -and the Lisp value to return for that variable if it is accessed
>> -via the syntax `$NAME'.
>> +Each member defines the name of a variable, and the Lisp value to
>> +return for that variable if it is accessed via the syntax `$NAME'.
>
> Again, not a problem introduced by your patch, but "Lisp value to
> return" seem pretty inaccurate (if you don't feel like fixing this, it's
> okay, I think describing it correctly is somewhat non-trivial).

Aaah sorry, I hadn't read your comment correctly. I've rewritten that
part and expanded a bit upon it. I think the most confusing part is that
when a user does `$foobar', if "foobar" is not in
eshell-variable-aliases-list, then "foobar" itself is used as a "value"
(which may or may not end up pointing to an env var, or a Lisp var). But
I don't think this is relevant when explaining what
eshell-variable-aliases-list should contain.

(I've attached a new patch!)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Copy-INSIDE_EMACS-env-variable-to-subprocesses-in-Es.patch --]
[-- Type: text/x-diff, Size: 4988 bytes --]

From 3f56b3641ba75430210325bcfd0c1bdff16f2a01 Mon Sep 17 00:00:00 2001
From: Federico Tedin <federicotedin@gmail.com>
Date: Mon, 30 Mar 2020 21:44:47 +0200
Subject: [PATCH] Copy INSIDE_EMACS env variable to subprocesses in Eshell

* lisp/eshell/em-dirs.el (eshell-dirs-initialize): Add INSIDE_EMACS
variable to buffer-local value of eshell-variable-aliases-alist.
(eshell-inside-emacs): Add new constant used for INSIDE_EMACS.
* lisp/eshell/esh-var.el (eshell-variable-aliases-list): Update doc
string; remove mention of eshell-user-aliases-list and explain that
variables can optionally be copied to subprocesses' environments.
* test/lisp/eshell/eshell-tests.el (eshell-test/inside-emacs-var): Add
test for the INSIDE_EMACS variable.
* etc/NEWS: Announce changes.
---
 etc/NEWS                         |  6 ++++++
 lisp/eshell/em-dirs.el           |  5 +++++
 lisp/eshell/esh-var.el           | 23 ++++++++++++++++++-----
 test/lisp/eshell/eshell-tests.el |  7 +++++++
 4 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 4b477e5def..9ac90ecabf 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -161,6 +161,12 @@ functions accessible to the user through the existing single function hook
 Modes should use the new hook instead of this user option to register
 their backends.
 
+** Eshell
+
+---
+*** Environment variable INSIDE_EMACS is now copied to subprocesses.
+Its value equals the result of evaluating '(format "%s,eshell" emacs-version)'.
+
 ** Tramp
 
 +++
diff --git a/lisp/eshell/em-dirs.el b/lisp/eshell/em-dirs.el
index 1949e5dc8f..51df6fa1d5 100644
--- a/lisp/eshell/em-dirs.el
+++ b/lisp/eshell/em-dirs.el
@@ -168,6 +168,9 @@ 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!
@@ -191,6 +194,8 @@ eshell-dirs-initialize
 		        (unless (ring-empty-p eshell-last-dir-ring)
 			  (expand-file-name
 			   (ring-ref eshell-last-dir-ring 0))))
+            t)
+           ("INSIDE_EMACS" eshell-inside-emacs
             t))))
 
   (when eshell-cd-on-directory
diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el
index 327a1def46..c12999ee0d 100644
--- a/lisp/eshell/esh-var.el
+++ b/lisp/eshell/esh-var.el
@@ -179,10 +179,9 @@ eshell-variable-aliases-list
 	      (eshell-apply-indices eshell-command-arguments
 				    indices)))))
   "This list provides aliasing for variable references.
-It is very similar in concept to what `eshell-user-aliases-list' does
-for commands.  Each member of this defines the name of a command,
-and the Lisp value to return for that variable if it is accessed
-via the syntax `$NAME'.
+Each member defines the name of a variable, and a Lisp value used to
+compute the string value that will be returned when the variable is
+accessed via the syntax `$NAME'.
 
 If the value is a function, that function will be called with two
 arguments: the list of the indices that was used in the reference, and
@@ -190,7 +189,21 @@ eshell-variable-aliases-list
 For example, a reference of `$NAME[10][20]' would result in the
 function for alias `NAME' being called (assuming it were aliased to a
 function), and the arguments passed to this function would be the list
-'(10 20)', and nil."
+'(10 20)', and nil.
+
+If the value is a string, the value for the variable with that name in
+the current environment will be returned.  If no variable with that
+name exists in the environment, but if a symbol with that same name
+exists and has a value bound to it, then that value will be used.  You
+can prioritize symbol values over environment values by setting
+`eshell-prefer-lisp-variables' to t.
+
+If the value is a symbol, the value bound to that symbol will be used.
+
+If the value has any other type, `error' will be signaled.
+
+Additionally, each member may specify if it should be copied to the
+environment of created subprocesses."
   :type '(repeat (list string sexp
 		       (choice (const :tag "Copy to environment" t)
 			       (const :tag "Use only in Eshell" nil)))))
diff --git a/test/lisp/eshell/eshell-tests.el b/test/lisp/eshell/eshell-tests.el
index 026818ab06..ce8d728833 100644
--- a/test/lisp/eshell/eshell-tests.el
+++ b/test/lisp/eshell/eshell-tests.el
@@ -169,6 +169,13 @@ eshell-test/last-arg-var
    (eshell-command-result-p "+ 1 2; + $_ 4"
                              "3\n6\n")))
 
+(ert-deftest eshell-test/inside-emacs-var ()
+  "Test presence of \"INSIDE_EMACS\" in subprocesses"
+  (with-temp-eshell
+   (eshell-command-result-p "env"
+                            (format "INSIDE_EMACS=%s,eshell"
+                                    emacs-version))))
+
 (ert-deftest eshell-test/escape-nonspecial ()
   "Test that \"\\c\" and \"c\" are equivalent when \"c\" is not a
 special character."
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* bug#25496: 25.1.91; INSIDE_EMACS env variable is not set in eshell
  2020-03-30 19:52         ` Federico Tedin
@ 2020-03-30 23:11           ` Noam Postavsky
  2020-04-02 23:06             ` Noam Postavsky
  2020-04-03 12:17             ` Eli Zaretskii
  0 siblings, 2 replies; 14+ messages in thread
From: Noam Postavsky @ 2020-03-30 23:11 UTC (permalink / raw)
  To: Federico Tedin; +Cc: 25496

Federico Tedin <federicotedin@gmail.com> writes:

>>> I've removed the lambda, but I wasn't able to use a string directly, I
>>> had to set the value elsewhere using a defconst. This is due to how
>>> `eshell-get-variable' interprets string values associated to
>>> variables.
>>
>> Oh, a string is actually taken as an environment variable to alias.  I
>> guess that does justify the name.  I think we might actually be better
>> off with the lambda, what do you think?
>
> Hmm, actually now it's working fine - the value associated with
> INSIDE_EMACS is the symbol `eshell-inside-emacs'.

Yeah, it works fine, I'm just not sure if having the extra
eshell-inside-emacs variable is better than a lambda.  But anyway, it
doesn't really matter that much.  I'll wait a couple of days (in case
there are any more suggested changes) and then push to master.

>> Again, not a problem introduced by your patch, but "Lisp value to
>> return" seem pretty inaccurate (if you don't feel like fixing this, it's
>> okay, I think describing it correctly is somewhat non-trivial).
>
> Aaah sorry, I hadn't read your comment correctly.

No, no, you read it right.  I just initially wrote the wrong thing: I
hadn't realized the docstring was wrong before (that's why I mistakenly
suggested using a string instead of a lambda).

> +If the value is a string, the value for the variable with that name in
> +the current environment will be returned.  If no variable with that
> +name exists in the environment, but if a symbol with that same name
> +exists and has a value bound to it, then that value will be used.  You
> +can prioritize symbol values over environment values by setting
> +`eshell-prefer-lisp-variables' to t.
> +
> +If the value is a symbol, the value bound to that symbol will be used.

Much better, thanks.






^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#25496: 25.1.91; INSIDE_EMACS env variable is not set in eshell
  2020-03-30 23:11           ` Noam Postavsky
@ 2020-04-02 23:06             ` Noam Postavsky
  2020-04-03 12:17             ` Eli Zaretskii
  1 sibling, 0 replies; 14+ messages in thread
From: Noam Postavsky @ 2020-04-02 23:06 UTC (permalink / raw)
  To: Federico Tedin; +Cc: 25496

tags 25496 fixed
close 25496 28.1
quit

Noam Postavsky <npostavs@gmail.com> writes:
> I'll wait a couple of days (in case there are any more suggested
> changes) and then push to master.

Done.

[1: f28166dc9a]: 2020-04-02 18:59:57 -0400
  Copy INSIDE_EMACS env variable to subprocesses in Eshell (Bug#25496)
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=f28166dc9a56111606be8ac50ad38179a66ea636





^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#25496: 25.1.91; INSIDE_EMACS env variable is not set in eshell
  2020-03-30 23:11           ` Noam Postavsky
  2020-04-02 23:06             ` Noam Postavsky
@ 2020-04-03 12:17             ` Eli Zaretskii
  2020-04-04 10:08               ` Federico Tedin
  1 sibling, 1 reply; 14+ messages in thread
From: Eli Zaretskii @ 2020-04-03 12:17 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: 25496, federicotedin

> From: Noam Postavsky <npostavs@gmail.com>
> Date: Mon, 30 Mar 2020 19:11:04 -0400
> Cc: 25496@debbugs.gnu.org
> 
> > +If the value is a string, the value for the variable with that name in
> > +the current environment will be returned.  If no variable with that
> > +name exists in the environment, but if a symbol with that same name
> > +exists and has a value bound to it, then that value will be used.  You
> > +can prioritize symbol values over environment values by setting
> > +`eshell-prefer-lisp-variables' to t.
> > +
> > +If the value is a symbol, the value bound to that symbol will be used.
> 
> Much better, thanks.

Would it be possible to reword this so that passive tense is used less
frequently?  E.g., instead of "the variable with that name in the
current environment will be returned", how about "return the variable
with that name in the current environment"?  And similarly with the
rest of the doc string.

Thanks.





^ permalink raw reply	[flat|nested] 14+ messages in thread

* bug#25496: 25.1.91; INSIDE_EMACS env variable is not set in eshell
  2020-04-03 12:17             ` Eli Zaretskii
@ 2020-04-04 10:08               ` Federico Tedin
  2020-04-11  9:20                 ` Eli Zaretskii
  0 siblings, 1 reply; 14+ messages in thread
From: Federico Tedin @ 2020-04-04 10:08 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 25496, Noam Postavsky

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

>> I'll wait a couple of days (in case there are any more suggested
>> changes) and then push to master.
>
> Done.

Thanks!

> Would it be possible to reword this so that passive tense is used less
> frequently?  E.g., instead of "the variable with that name in the
> current environment will be returned", how about "return the variable
> with that name in the current environment"?  And similarly with the
> rest of the doc string.
>
> Thanks.

No problem, I've attached a patch with the changes.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Reword-documentation-for-eshell-variable-aliases-lis.patch --]
[-- Type: text/x-diff, Size: 2690 bytes --]

From 1e77ef50a7bf24e67fa6003bb768cdae2202a270 Mon Sep 17 00:00:00 2001
From: Federico Tedin <federicotedin@gmail.com>
Date: Sat, 4 Apr 2020 12:04:11 +0200
Subject: [PATCH] Reword documentation for eshell-variable-aliases-list

* lisp/eshell/esh-var.el (eshell-variable-aliases-list): Update
documentation string.
---
 lisp/eshell/esh-var.el | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el
index 3093abd830..70516b3b82 100644
--- a/lisp/eshell/esh-var.el
+++ b/lisp/eshell/esh-var.el
@@ -183,24 +183,24 @@ eshell-variable-aliases-list
 compute the string value that will be returned when the variable is
 accessed via the syntax `$NAME'.
 
-If the value is a function, that function will be called with two
-arguments: the list of the indices that was used in the reference, and
-whether the user is requesting the length of the ultimate element.
-For example, a reference of `$NAME[10][20]' would result in the
-function for alias `NAME' being called (assuming it were aliased to a
-function), and the arguments passed to this function would be the list
-'(10 20)', and nil.
-
-If the value is a string, the value for the variable with that name in
-the current environment will be returned.  If no variable with that
-name exists in the environment, but if a symbol with that same name
-exists and has a value bound to it, then that value will be used.  You
-can prioritize symbol values over environment values by setting
+If the value is a function, call that function with two arguments: the
+list of the indices that was used in the reference, and whether the
+user is requesting the length of the ultimate element.  For example, a
+reference of `$NAME[10][20]' would result in the function for alias
+`NAME' being called (assuming it were aliased to a function), and the
+arguments passed to this function would be the list '(10 20)', and
+nil.
+
+If the value is a string, return the value for the variable with that
+name in the current environment.  If no variable with that name exists
+in the environment, but if a symbol with that same name exists and has
+a value bound to it, return its value instead.  You can prioritize
+symbol values over environment values by setting
 `eshell-prefer-lisp-variables' to t.
 
-If the value is a symbol, the value bound to that symbol will be used.
+If the value is a symbol, return the value bound to it.
 
-If the value has any other type, `error' will be signaled.
+If the value has any other type, signal `error'.
 
 Additionally, each member may specify if it should be copied to the
 environment of created subprocesses."
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* bug#25496: 25.1.91; INSIDE_EMACS env variable is not set in eshell
  2020-04-04 10:08               ` Federico Tedin
@ 2020-04-11  9:20                 ` Eli Zaretskii
  0 siblings, 0 replies; 14+ messages in thread
From: Eli Zaretskii @ 2020-04-11  9:20 UTC (permalink / raw)
  To: Federico Tedin; +Cc: npostavs, 25496-done

> From: Federico Tedin <federicotedin@gmail.com>
> Cc: Noam Postavsky <npostavs@gmail.com>,  25496@debbugs.gnu.org
> Date: Sat, 04 Apr 2020 12:08:43 +0200
> 
> > Would it be possible to reword this so that passive tense is used less
> > frequently?  E.g., instead of "the variable with that name in the
> > current environment will be returned", how about "return the variable
> > with that name in the current environment"?  And similarly with the
> > rest of the doc string.
> >
> > Thanks.
> 
> No problem, I've attached a patch with the changes.

Thanks, pushed.





^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2020-04-11  9:20 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-20 22:32 bug#25496: 25.1.91; INSIDE_EMACS env variable is not set in eshell Alex Hutcheson
2017-01-23 20:20 ` Glenn Morris
2017-01-23 20:36   ` Alex Hutcheson
2020-03-08 16:50 ` Federico Tedin
2020-03-28 15:05   ` Federico Tedin
2020-03-29  0:21   ` Noam Postavsky
2020-03-29 22:29     ` Federico Tedin
2020-03-30  1:44       ` Noam Postavsky
2020-03-30 19:52         ` Federico Tedin
2020-03-30 23:11           ` Noam Postavsky
2020-04-02 23:06             ` Noam Postavsky
2020-04-03 12:17             ` Eli Zaretskii
2020-04-04 10:08               ` Federico Tedin
2020-04-11  9:20                 ` Eli Zaretskii

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).