bug#27361: 24.4; eshell/echo silently ignores -n argument

bug#27361: 24.4; eshell/echo silently ignores -n argument

[Phil Hagelberg]

When using eshell, the `echo' built-in doesn't respect the -n argument,
which is supposed to suppress the trailing newline.

Sometimes this is not a problem because you can see for yourself that
eshell did the wrong thing, but if you are piping it directly to another
command, it's easy to miss.

The eshell/echo function should be changed to either accept the -n
argument and omit the newline, or it should complain that -n is not a
supported argument.



In GNU Emacs 24.4.1 (x86_64-pc-linux-gnu, GTK+ Version 3.14.5)
 of 2015-03-07 on trouble, modified by Debian
Windowing system distributor `The X.Org Foundation', version 11.0.11604000
System Description:	Debian GNU/Linux 8.8 (jessie)

Configured using:
 `configure --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/emacs24:/etc/emacs:/usr/local/share/emacs/24.4/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/24.4/site-lisp:/usr/share/emacs/site-lisp
 --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/emacs24:/etc/emacs:/usr/local/share/emacs/24.4/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/24.4/site-lisp:/usr/share/emacs/site-lisp
 --with-x=yes --with-x-toolkit=gtk3 --with-toolkit-scroll-bars
 'CFLAGS=-g -O2 -fstack-protector-strong -Wformat
 -Werror=format-security -Wall' CPPFLAGS=-D_FORTIFY_SOURCE=2
 LDFLAGS=-Wl,-z,relro'

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

Major mode: EShell

Minor modes in effect:
  ido-ubiquitous-mode: t
  winner-mode: t
  diff-auto-refine-mode: t
  show-paren-mode: t
  tooltip-mode: t
  electric-indent-mode: t
  mouse-wheel-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
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent input:
e c h o SPC - n SPC w a <backspace> <backspace> c o 
m e SPC o n SPC n o w <return> M-p SPC | S-SPC c a 
t <return> M-x r e p o r t - e m <return>

Recent messages:
Loading em-dirs...done
Loading em-glob...done
Loading em-hist...done
Loading em-ls...done
Loading em-prompt...done
Loading em-script...done
Loading em-term...done
Loading em-unix...done
For information about GNU Emacs and the GNU system, type C-h C-a.
History item: 128

Load-path shadows:
~/.emacs.d/lib/yaml-mode/yaml-mode hides /usr/share/emacs24/site-lisp/yaml-mode/yaml-mode
/usr/share/emacs/site-lisp/mu4e/mu4e-speedbar hides /usr/share/emacs24/site-lisp/mu4e/mu4e-speedbar
/usr/share/emacs/site-lisp/mu4e/mu4e-message hides /usr/share/emacs24/site-lisp/mu4e/mu4e-message
/usr/share/emacs/site-lisp/mu4e/mu4e hides /usr/share/emacs24/site-lisp/mu4e/mu4e
/usr/share/emacs/site-lisp/mu4e/mu4e-about hides /usr/share/emacs24/site-lisp/mu4e/mu4e-about
/usr/share/emacs/site-lisp/mu4e/mu4e-contrib hides /usr/share/emacs24/site-lisp/mu4e/mu4e-contrib
/usr/share/emacs/site-lisp/mu4e/mu4e-mark hides /usr/share/emacs24/site-lisp/mu4e/mu4e-mark
/usr/share/emacs/site-lisp/mu4e/mu4e-vars hides /usr/share/emacs24/site-lisp/mu4e/mu4e-vars
/usr/share/emacs/site-lisp/mu4e/mu4e-headers hides /usr/share/emacs24/site-lisp/mu4e/mu4e-headers
/usr/share/emacs/site-lisp/mu4e/mu4e-proc hides /usr/share/emacs24/site-lisp/mu4e/mu4e-proc
/usr/share/emacs/site-lisp/mu4e/mu4e-main hides /usr/share/emacs24/site-lisp/mu4e/mu4e-main
/usr/share/emacs/site-lisp/mu4e/mu4e-meta hides /usr/share/emacs24/site-lisp/mu4e/mu4e-meta
/usr/share/emacs/site-lisp/mu4e/mu4e-draft hides /usr/share/emacs24/site-lisp/mu4e/mu4e-draft
/usr/share/emacs/site-lisp/mu4e/org-mu4e hides /usr/share/emacs24/site-lisp/mu4e/org-mu4e
/usr/share/emacs/site-lisp/mu4e/mu4e-utils hides /usr/share/emacs24/site-lisp/mu4e/mu4e-utils
/usr/share/emacs/site-lisp/mu4e/mu4e-compose hides /usr/share/emacs24/site-lisp/mu4e/mu4e-compose
/usr/share/emacs/site-lisp/mu4e/mu4e-view hides /usr/share/emacs24/site-lisp/mu4e/mu4e-view
/usr/share/emacs/site-lisp/mu4e/mu4e-lists hides /usr/share/emacs24/site-lisp/mu4e/mu4e-lists
/usr/share/emacs/site-lisp/mu4e/mu4e-actions hides /usr/share/emacs24/site-lisp/mu4e/mu4e-actions
/usr/share/emacs/24.4/site-lisp/debian-startup hides /usr/share/emacs/site-lisp/debian-startup
/usr/share/emacs24/site-lisp/cmake-data/cmake-mode hides /usr/share/emacs/site-lisp/cmake-mode
~/.emacs.d//phil/../custom hides /usr/share/emacs/24.4/lisp/custom
/usr/share/emacs/site-lisp/rst hides /usr/share/emacs/24.4/lisp/textmodes/rst
/usr/share/emacs24/site-lisp/dictionaries-common/flyspell hides /usr/share/emacs/24.4/lisp/textmodes/flyspell
/usr/share/emacs24/site-lisp/dictionaries-common/ispell hides /usr/share/emacs/24.4/lisp/textmodes/ispell

Features:
(shadow sort flyspell ispell gnus-util mail-extr emacsbug message
format-spec rfc822 mml mml-sec mm-decode mm-bodies mm-encode mail-parse
rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045
ietf-drums mm-util mail-prsvr mail-utils pp elisp-slime-nav help-mode
etags paredit whitespace idle-highlight-mode thingatpt hl-line
page-break-lines em-unix em-term term disp-table ehelp em-script
em-prompt em-ls em-hist em-pred em-glob em-dirs em-cmpl em-basic
em-banner em-alias pcomplete comint ansi-color esh-var esh-io esh-cmd
esh-opt esh-ext esh-proc esh-arg eldoc esh-groups eshell esh-module
esh-mode esh-util ido-ubiquitous ido-completing-read+ cus-edit cus-start
cus-load wid-edit winner ring smex find-file-in-project diff-mode
easymenu easy-mmode autoload advice help-fns paren edmacro kmacro
saveplace ido cl-macs cl gv cl-loaddefs cl-lib time-date tooltip
electric uniquify ediff-hook vc-hooks lisp-float-type mwheel x-win x-dnd
tool-bar dnd fontset image regexp-opt fringe tabulated-list newcomment
lisp-mode prog-mode register page menu-bar rfn-eshadow timer select
scroll-bar mouse jit-lock font-lock syntax facemenu font-core frame cham
georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese hebrew greek romanian slovak czech european ethiopic
indian cyrillic chinese case-table epa-hook jka-cmpr-hook help simple
abbrev minibuffer 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 make-network-process
dbusbind gfilenotify dynamic-setting system-font-setting
font-render-setting move-toolbar gtk x-toolkit x multi-tty emacs)

Memory information:
((conses 16 148161 6857)
 (symbols 48 24391 0)
 (miscs 40 86 137)
 (strings 32 31521 6206)
 (string-bytes 1 921308)
 (vectors 16 16803)
 (vector-slots 8 457088 3415)
 (floats 8 103 100)
 (intervals 56 328 0)
 (buffers 960 12)
 (heap 1024 33431 1918))

bug#27361: 24.4; eshell/echo silently ignores -n argument

[npostavs]

Phil Hagelberg <phil@hagelb.org> writes:

> When using eshell, the `echo' built-in doesn't respect the -n argument,
> which is supposed to suppress the trailing newline.
>
> Sometimes this is not a problem because you can see for yourself that
> eshell did the wrong thing, but if you are piping it directly to another
> command, it's easy to miss.
>
> The eshell/echo function should be changed to either accept the -n
> argument and omit the newline, or it should complain that -n is not a
> supported argument.

It's not ignored, but eshell always adds a newline before the prompt.
Furthermore, the argument has the opposite sense to the traditional
shell interpretation:

    ~/src $ echo x | od -c
    0000000   x
    0000001
    ~/src $ echo -n x | od -c
    0000000   x  \n
    0000002

bug#27361: 24.4; eshell/echo silently ignores -n argument

[Phil Hagelberg]

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

npostavs@users.sourceforge.net writes:

> It's not ignored, but eshell always adds a newline before the prompt.
> Furthermore, the argument has the opposite sense to the traditional
> shell interpretation

Ah, I see. Yeah, doing the opposite of what the user asked for in both
cases is much worse than simply ignoring the argument when it's present.

-Phil

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

bug#27361: 24.4; eshell/echo silently ignores -n argument

[Glenn Morris]

echo -n is famously non-portable.
Perhaps eshell is trying to make a point. ;)
echo --help does document the -n behaviour, and it's been like that
since always.
With eshell-plain-echo-behavior set, -n is simply ignored.
I would have thought you must have eshell-plain-echo-behavior set
if you want anything like "standard" echo behaviour.

bug#27361: 24.4; eshell/echo silently ignores -n argument

[Glenn Morris]

It seems easy to get the behaviour that you want, for the
eshell-plain-echo-behavior case, which is the only relevant one.
At the same time, we can fix eshell/printnl for that case,
to avoid the doubled newlines that it currently gives.

I don't know how to explain in --help what -n means though. :)

--- a/lisp/eshell/em-basic.el
+++ b/lisp/eshell/em-basic.el
@@ -88,7 +88,8 @@ eshell-echo
 It returns a formatted value that should be passed to `eshell-print'
 or `eshell-printn' for display."
   (if eshell-plain-echo-behavior
-      (concat (apply 'eshell-flatten-and-stringify args) "\n")
+      (concat (apply 'eshell-flatten-and-stringify args)
+	      (unless output-newline "\n"))
     (let ((value
 	   (cond
 	    ((= (length args) 0) "")
@@ -126,7 +127,8 @@ eshell/printnl
   "Print out each of the arguments, separated by newlines."
   (let ((elems (eshell-flatten-list args)))
     (while elems
-      (eshell-printn (eshell-echo (list (car elems))))
+      (eshell-printn (eshell-echo (list (car elems))
+				  eshell-plain-echo-behavior))
       (setq elems (cdr elems)))))
 
 (defun eshell/listify (&rest args)

bug#27361: 24.4; eshell/echo silently ignores -n argument

[Lars Ingebrigtsen]

Glenn Morris <rgm@gnu.org> writes:

> It seems easy to get the behaviour that you want, for the
> eshell-plain-echo-behavior case, which is the only relevant one.
> At the same time, we can fix eshell/printnl for that case,
> to avoid the doubled newlines that it currently gives.
>
> I don't know how to explain in --help what -n means though. :)

[...]

> -      (concat (apply 'eshell-flatten-and-stringify args) "\n")
> +      (concat (apply 'eshell-flatten-and-stringify args)
> +	      (unless output-newline "\n"))

Hm...  with or without the patch, and with eshell-plain-echo-behavior
set, I don't see any difference?

I still get this in eshell:

~/src/emacs/trunk $ echo -n foo; echo bar
foo
bar

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#27361: Re: bug#27361: 24.4; eshell/echo silently ignores -n argument

[Jim Porter]

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

On 9/15/2020 8:27 AM, Lars Ingebrigtsen wrote:
> Glenn Morris <rgm@gnu.org> writes:
> 
>> -      (concat (apply 'eshell-flatten-and-stringify args) "\n")
>> +      (concat (apply 'eshell-flatten-and-stringify args)
>> +	      (unless output-newline "\n"))
> 
> Hm...  with or without the patch, and with eshell-plain-echo-behavior
> set, I don't see any difference?
> 
> I still get this in eshell:
> 
> ~/src/emacs/trunk $ echo -n foo; echo bar
> foo
> bar

I think this is due to `eshell-lisp-command' insisting that a newline is 
inserted after the output of *every* Lisp command (when executed 
interactively). It's easier to see what "-n" does by eval'ing:

   (eshell-command-result "echo hi")
   ;; and
   (eshell-command-result "echo -n hi")

One solution to this bug would be to set `eshell-ensure-newline-p' to 
nil inside `eshell-echo'; then it will opt out of `eshell-lisp-command's 
trailing-newline insertion.

I've attached a patch that does this, as well as (hopefully) resolving 
the unusual meaning of "-n" when echo outputs a Lisp-friendly result 
(i.e. when `eshell-plain-echo-behavior' is nil). Now, it warns the user 
to use "-N" instead if they want to add a trailing newline. Then, one 
day, the old meaning of "-n" can be removed. I'm not sure if this is the 
best way to emit a warning about this though; I wanted something that 
doesn't interfere with any Eshell scripts, but `display-warning' is a 
bit in-your-face.

I also simplified the logic of `eshell/printnl'. Since the way it called 
`eshell-echo' always resulted in it printing the stringified form of 
each argument, it's simpler just to call `eshell-stringify' directly.

Finally, I added a bit more detail about echo to the Eshell manual 
(taken partly from the commentary of eshell/em-basic.el), since I didn't 
quite understand its behavior initially.

[-- Attachment #2: 0001-In-Eshell-allow-n-to-suppress-the-trailing-newline-f.patch --]
[-- Type: text/plain, Size: 4058 bytes --]

From 45d10c678a3c58486e0571d3c3e8ce64feb39299 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sat, 15 Jan 2022 15:16:31 -0800
Subject: [PATCH] In Eshell, allow "-n" to suppress the trailing newline for
 "plain" echo

In addition, add a "-N" option to explicitly request a newline.

* lisp/eshell/em-basic.el (eshell-echo): Respect OUTPUT-NEWLINE even
when 'eshell-plain-echo-behavior' is non-nil.
(eshell/echo): Add "-N" option and recommend its use over "-n" in
Lisp-friendly echo.
(eshell/printnl): Simplify; 'eshell-stringify' is equivalent to
calling 'eshell-echo' here.

* doc/misc/eshell.texi (Built-in commands): Expand on the
documentation of echo.
---
 doc/misc/eshell.texi    | 13 ++++++++++---
 lisp/eshell/em-basic.el | 30 ++++++++++++++++++++++--------
 2 files changed, 32 insertions(+), 11 deletions(-)

diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index f1d7c63805..df6e3b861e 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -407,9 +407,16 @@ Built-ins
 
 @item echo
 @cmindex echo
-Echoes its input.  If @code{eshell-plain-echo-behavior} is
-non-@code{nil}, @command{echo} will try to behave more like a plain
-shell's @command{echo}.
+Echoes its input.  By default, this prints in a Lisp-friendly fashion
+(so that the value is useful to a Lisp command using the result of
+@command{echo} as an argument).  If a single argument is passed,
+@command{echo} prints that; if multiple arguments are passed, it
+prints a list of all the arguments; otherwise, it prints the empty
+string.
+
+If @code{eshell-plain-echo-behavior} is non-@code{nil}, @command{echo}
+will try to behave more like a plain shell's @command{echo}, printing
+each argument as a string, separated by a space.
 
 @item env
 @cmindex env
diff --git a/lisp/eshell/em-basic.el b/lisp/eshell/em-basic.el
index 27b343ad39..d3b15c900b 100644
--- a/lisp/eshell/em-basic.el
+++ b/lisp/eshell/em-basic.el
@@ -82,7 +82,11 @@ eshell-echo
 It returns a formatted value that should be passed to `eshell-print'
 or `eshell-printn' for display."
   (if eshell-plain-echo-behavior
-      (concat (apply 'eshell-flatten-and-stringify args) "\n")
+      (progn
+        ;; If the output does not end in a newline, do not emit one.
+        (setq eshell-ensure-newline-p nil)
+        (concat (apply #'eshell-flatten-and-stringify args)
+                (when output-newline "\n")))
     (let ((value
 	   (cond
 	    ((= (length args) 0) "")
@@ -109,18 +113,28 @@ eshell/echo
   "Implementation of `echo'.  See `eshell-plain-echo-behavior'."
   (eshell-eval-using-options
    "echo" args
-   '((?n nil nil output-newline "terminate with a newline")
+   '((?n nil (nil) output-newline "do not output the trailing newline")
+     (?N nil (t)   output-newline "terminate with a newline")
      (?h "help" nil nil "output this help screen")
      :preserve-args
-     :usage "[-n] [object]")
-   (eshell-echo args output-newline)))
+     :usage "[-n | -N] [object]")
+   (if eshell-plain-echo-behavior
+       (eshell-echo args (if output-newline (car output-newline) t))
+     ;; In Emacs 28.1 and earlier, "-n" was used to add a newline to
+     ;; non-plain echo in Eshell.  This caused confusion due to "-n"
+     ;; generally having the opposite meaning for echo.  Retain this
+     ;; compatibility for the time being.  For more info, see
+     ;; bug#27361.
+     (when (equal output-newline '(nil))
+       (display-warning
+        :warning "To terminate with a newline, you should use -N instead."))
+     (eshell-echo args output-newline))))
 
 (defun eshell/printnl (&rest args)
-  "Print out each of the arguments, separated by newlines."
+  "Print out each of the arguments as strings, separated by newlines."
   (let ((elems (flatten-tree args)))
-    (while elems
-      (eshell-printn (eshell-echo (list (car elems))))
-      (setq elems (cdr elems)))))
+    (dolist (elem elems)
+      (eshell-printn (eshell-stringify elem)))))
 
 (defun eshell/listify (&rest args)
   "Return the argument(s) as a single list."
-- 
2.25.1

bug#27361: Re: bug#27361: 24.4; eshell/echo silently ignores -n argument

[Lars Ingebrigtsen]

Jim Porter <jporterbugs@gmail.com> writes:

> I've attached a patch that does this, as well as (hopefully) resolving
> the unusual meaning of "-n" when echo outputs a Lisp-friendly result
> (i.e. when `eshell-plain-echo-behavior' is nil). Now, it warns the

Thanks; applied to Emacs 29.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#27361: Re: bug#27361: 24.4; eshell/echo silently ignores -n argument

[Jim Porter]

On 1/20/2022 1:36 AM, Lars Ingebrigtsen wrote:
> Jim Porter <jporterbugs@gmail.com> writes:
> 
>> I've attached a patch that does this, as well as (hopefully) resolving
>> the unusual meaning of "-n" when echo outputs a Lisp-friendly result
>> (i.e. when `eshell-plain-echo-behavior' is nil). Now, it warns the
> 
> Thanks; applied to Emacs 29.

Thanks. Hopefully the warning about using "-n" with Lisp-friendly echo 
isn't too obtrusive for people, but if it is, we could probably replace 
it with `message'.