bug#29157: 25.3; Eshell parsing fails sometimes, e.g. "date" and "sed"

bug#29157: 25.3; Eshell parsing fails sometimes, e.g. "date" and "sed"

[Pierre Neidhardt]

- emacs -Q
- M-x eshell

	> date +%Z
	Invalid time specification
	
	> date "+%Z"
	Invalid time specification

	> date #+%Z
	Sun Nov  5 12:35:51 2017

	> echo $PATH | sed "s/[^o]foo[^o]/bar/g"
	Unknown predicate character ‘b’

Am I misunderstanding Eshell's syntax?



In GNU Emacs 25.3.1 (x86_64-pc-linux-gnu, GTK+ Version 3.22.16)
 of 2017-11-05 built on dhiov23k
Windowing system distributor 'The X.Org Foundation', version 11.0.11905000
System Description:	Gentoo Base System release 2.4.1

Configured using:
 'configure --prefix=/usr --build=x86_64-pc-linux-gnu
 --host=x86_64-pc-linux-gnu --mandir=/usr/share/man
 --infodir=/usr/share/info --datadir=/usr/share --sysconfdir=/etc
 --localstatedir=/var/lib --disable-dependency-tracking
 --disable-silent-rules --docdir=/usr/share/doc/emacs-25.3
 --htmldir=/usr/share/doc/emacs-25.3/html --libdir=/usr/lib64
 --program-suffix=-emacs-25 --infodir=/usr/share/info/emacs-25
 --localstatedir=/var
 --enable-locallisppath=/etc/emacs:/usr/share/emacs/site-lisp
 --with-gameuser=:gamestat --without-compress-install
 --with-file-notification=inotify --enable-acl --without-dbus
 --without-modules --without-gpm --without-hesiod --without-kerberos
 --without-kerberos5 --with-xml2 --without-selinux --with-gnutls
 --without-wide-int --with-zlib --with-sound=alsa --with-x --without-ns
 --without-gconf --without-gsettings --without-toolkit-scroll-bars
 --with-gif --with-jpeg --with-png --with-rsvg --with-tiff --with-xpm
 --with-imagemagick --with-xft --without-cairo --without-libotf
 --without-m17n-flt --with-x-toolkit=gtk3 --without-xwidgets
 GENTOO_PACKAGE=app-editors/emacs-25.3 'CFLAGS=-march=ivybridge -O2
 -pipe' CPPFLAGS= 'LDFLAGS=-Wl,-O1 -Wl,--as-needed''

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND NOTIFY ACL GNUTLS LIBXML2
FREETYPE XFT ZLIB GTK3 X11

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

bug#29157: 25.3; Eshell parsing fails sometimes, e.g. "date" and "sed"

[Noam Postavsky]

Pierre Neidhardt <ambrevar@gmail.com> writes:

> - emacs -Q
> - M-x eshell
>
> 	> date +%Z
> 	Invalid time specification

    ~/src/emacs $ which date
    eshell/date is an alias for ‘current-time-string’ in ‘em-unix.el’.
    ~/src/emacs $ which *date
    /bin/date
    ~/src/emacs $ *date +%Z
    EST

> 	> echo $PATH | sed "s/[^o]foo[^o]/bar/g"
> 	Unknown predicate character ‘b’

M-x toggle-debug-on-error gives more hints about this.  Apparently this
matches a "history reference":

    (defun eshell-history-reference (reference)
      "Expand directory stack REFERENCE.
    The syntax used here was taken from the Bash info manual.
    Returns the resultant reference, or the same string REFERENCE if none
    matched."
      ;; `^string1^string2^'
      ;;      Quick Substitution.  Repeat the last command, replacing
      ;;      STRING1 with STRING2.  Equivalent to `!!:s/string1/string2/'

Debugger entered--Lisp error: (error "Unknown predicate character ‘b’")
  signal(error ("Unknown predicate character ‘b’"))
  error("Unknown predicate character `%c'" 98)
  eshell-parse-modifiers()
  eshell-hist-parse-modifier("printf '<<%s>>\\n' \"s/[^o]foo/\"" ":s/o]foo[/o]/bar/g\"/")
  eshell-history-reference("\"s/[^o]foo[^o]/bar/g\"")
  eshell-expand-history-references(#<marker at 43 in *eshell*> 81)
  run-hook-with-args(eshell-expand-history-references #<marker at 43 in *eshell*> 81)
  eshell-send-input(nil)
  funcall-interactively(eshell-send-input nil)
  call-interactively(eshell-send-input nil nil)
  command-execute(eshell-send-input)


My suggestion for this is just to stop adding
eshell-expand-history-references to eshell-expand-input-functions, and
maybe even deprecate eshell-expand-history-references.  I find the
history expansion mechanism to be an annoyance in bash as well.

--- i/lisp/eshell/em-hist.el
+++ w/lisp/eshell/em-hist.el
@@ -218,9 +218,6 @@ eshell-input-filter-initial-space
 
 (defun eshell-hist-initialize ()
   "Initialize the history management code for one Eshell buffer."
-  (add-hook 'eshell-expand-input-functions
-	    'eshell-expand-history-references nil t)
-
   (when (eshell-using-module 'eshell-cmpl)
     (add-hook 'pcomplete-try-first-hook
 	      'eshell-complete-history-reference nil t))

bug#29157: 25.3; Eshell parsing fails sometimes, e.g. "date" and "sed"

[Pierre Neidhardt]

Noam Postavsky <npostavs@users.sourceforge.net> writes:

> Pierre Neidhardt <ambrevar@gmail.com> writes:
>
>> - emacs -Q
>> - M-x eshell
>>
>> 	> date +%Z
>> 	Invalid time specification
>
>     ~/src/emacs $ which date
>     eshell/date is an alias for ‘current-time-string’ in ‘em-unix.el’.
>     ~/src/emacs $ which *date
>     /bin/date
>     ~/src/emacs $ *date +%Z
>     EST

Duh!  That was stupid, sorry.  Anyways, that might ring an alarm here:
maybe eshell/date should not exist.  What's the point of having it?  I'm
not sure.  It is obviously less powerful than the system `date'.

>> 	> echo $PATH | sed "s/[^o]foo[^o]/bar/g"
>> 	Unknown predicate character ‘b’
>
> M-x toggle-debug-on-error gives more hints about this.  Apparently this
> matches a "history reference":
>
>     (defun eshell-history-reference (reference)
>       "Expand directory stack REFERENCE.
>     The syntax used here was taken from the Bash info manual.
>     Returns the resultant reference, or the same string REFERENCE if none
>     matched."
>       ;; `^string1^string2^'
>       ;;      Quick Substitution.  Repeat the last command, replacing
>       ;;      STRING1 with STRING2.  Equivalent to `!!:s/string1/string2/'
>
> Debugger entered--Lisp error: (error "Unknown predicate character ‘b’")
>   signal(error ("Unknown predicate character ‘b’"))
>   error("Unknown predicate character `%c'" 98)
>   eshell-parse-modifiers()
>   eshell-hist-parse-modifier("printf '<<%s>>\\n' \"s/[^o]foo/\"" ":s/o]foo[/o]/bar/g\"/")
>   eshell-history-reference("\"s/[^o]foo[^o]/bar/g\"")
>   eshell-expand-history-references(#<marker at 43 in *eshell*> 81)
>   run-hook-with-args(eshell-expand-history-references #<marker at 43 in *eshell*> 81)
>   eshell-send-input(nil)
>   funcall-interactively(eshell-send-input nil)
>   call-interactively(eshell-send-input nil nil)
>   command-execute(eshell-send-input)
>
>
> My suggestion for this is just to stop adding
> eshell-expand-history-references to eshell-expand-input-functions, and
> maybe even deprecate eshell-expand-history-references.  I find the
> history expansion mechanism to be an annoyance in bash as well.
>
> --- i/lisp/eshell/em-hist.el
> +++ w/lisp/eshell/em-hist.el
> @@ -218,9 +218,6 @@ eshell-input-filter-initial-space
>  
>  (defun eshell-hist-initialize ()
>    "Initialize the history management code for one Eshell buffer."
> -  (add-hook 'eshell-expand-input-functions
> -	    'eshell-expand-history-references nil t)
> -
>    (when (eshell-using-module 'eshell-cmpl)
>      (add-hook 'pcomplete-try-first-hook
>  	      'eshell-complete-history-reference nil t))

That does it for me.  I did not even know this feature existed.  I'd it
proves more useful in an environment with poor selection / editing
capabilities, i.e. a terminal shell.  Emacs does not need that when you
can fuzzy-search your history and modify your prompt with arbitrary
bindings / Lisp code.

-- 
Pierre Neidhardt

There are more things in heaven and earth than any place else.

bug#29157: 25.3; Eshell parsing fails sometimes, e.g. "date" and "sed"

[Andreas Schwab]

On Nov 05 2017, Noam Postavsky <npostavs@users.sourceforge.net> wrote:

>       ;; `^string1^string2^'
>       ;;      Quick Substitution.  Repeat the last command, replacing
>       ;;      STRING1 with STRING2.  Equivalent to `!!:s/string1/string2/'

That substitution is only supposed to be performed when occuring at the
start of the line.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

bug#29157: 25.3; Eshell parsing fails sometimes, e.g. "date" and "sed"

[Noam Postavsky]

Andreas Schwab <schwab@linux-m68k.org> writes:

> On Nov 05 2017, Noam Postavsky <npostavs@users.sourceforge.net> wrote:
>
>>       ;; `^string1^string2^'
>>       ;;      Quick Substitution.  Repeat the last command, replacing
>>       ;;      STRING1 with STRING2.  Equivalent to `!!:s/string1/string2/'
>
> That substitution is only supposed to be performed when occuring at the
> start of the line.

Hmm, I see that's the case by experimenting with bash, although the bash
manual doesn't seem to actually say that.

bug#29157: 25.3; Eshell parsing fails sometimes, e.g. "date" and "sed"

[Noam Postavsky]

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

Pierre Neidhardt <ambrevar@gmail.com> writes:

> Anyways, that might ring an alarm here: maybe eshell/date should not
> exist.  What's the point of having it?  I'm not sure.  It is obviously
> less powerful than the system `date'.

Eshell has lots of commands like that.  I guess it makes it more portable?

> That does it for me.  I did not even know this feature existed.  I'd it
> proves more useful in an environment with poor selection / editing
> capabilities, i.e. a terminal shell.  Emacs does not need that when you
> can fuzzy-search your history and modify your prompt with arbitrary
> bindings / Lisp code.

I agree.  Although the expansion in this case is arguably a bug (as
Andreas pointed out), I don't have much interest in fixing it.  I
propose just to disable it by default (in master).


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 1803 bytes --]

From c2753c383e603acfe15f70ee0cb3c93e624c6c39 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Wed, 22 Nov 2017 21:59:35 -0500
Subject: [PATCH] Disable history expansion in eshell (Bug#29157)

History expansion is not so useful since interactive history commands
are already provided.  It can produce surprising errors when the user
is not aware of the history designator syntax.
* lisp/eshell/em-hist.el (eshell-hist-initialize): Don't add
eshell-expand-history-references to eshell-expand-input-functions.
* etc/NEWS: Announce it.
---
 etc/NEWS               | 9 +++++++++
 lisp/eshell/em-hist.el | 3 ---
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index c47ca42d27..5a01b912ec 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -109,6 +109,15 @@ Snake and Pong are more playable on HiDPI displays.
 *** Completing filenames in the minibuffer via 'C-TAB' now uses the
 styles as configured by the variable 'completion-styles'.
 
+** Eshell
+
+---
+*** Expansion of history event designators is disabled by default.
+To restore the old behavior, use
+
+    (add-hook 'eshell-expand-input-functions
+              #'eshell-expand-history-references)
+
 \f
 * New Modes and Packages in Emacs 27.1
 
diff --git a/lisp/eshell/em-hist.el b/lisp/eshell/em-hist.el
index 8084c12653..df462a7058 100644
--- a/lisp/eshell/em-hist.el
+++ b/lisp/eshell/em-hist.el
@@ -218,9 +218,6 @@ eshell-input-filter-initial-space
 
 (defun eshell-hist-initialize ()
   "Initialize the history management code for one Eshell buffer."
-  (add-hook 'eshell-expand-input-functions
-	    'eshell-expand-history-references nil t)
-
   (when (eshell-using-module 'eshell-cmpl)
     (add-hook 'pcomplete-try-first-hook
 	      'eshell-complete-history-reference nil t))
-- 
2.11.0

bug#29157: 25.3; Eshell parsing fails sometimes, e.g. "date" and "sed"

[Pierre Neidhardt]

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


Noam Postavsky <npostavs@users.sourceforge.net> writes:

> Pierre Neidhardt <ambrevar@gmail.com> writes:
>
>> Anyways, that might ring an alarm here: maybe eshell/date should not
>> exist.  What's the point of having it?  I'm not sure.  It is obviously
>> less powerful than the system `date'.
>
> Eshell has lots of commands like that.  I guess it makes it more portable?

I'm really not sure where to go from here.

Disabling eshell/date makes Eshell less portable on one system at least,
that is Windows.  But what does "portability" mean in this context?  Are
the coreutils meant to be part of Eshell?  Why?  Supporting `date' but not
its arguments does not make up for actual portability I believe.  Case
in point: I got fooled.

Let's take the case of BSD vs. GNU: bash or zsh do not wrap around `ls',
so the behaviour will not be the same on BSD and GNU.  Why should Eshell
be any different?

Eshell is meant to be interactive: does portability matter in that case?

My current stance leans towards removing `date' and other commands that
seem to be here for "portability" only.

Wrappers like `grep' have a clear purpose: redirecting to a dedicated buffer.
I think it's find to keep such commands.

> I agree.  Although the expansion in this case is arguably a bug (as
> Andreas pointed out), I don't have much interest in fixing it.  I
> propose just to disable it by default (in master).

Thank you.

--
Pierre Neidhardt

Would the last person to leave Michigan please turn out the lights?

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

bug#29157: 25.3; Eshell parsing fails sometimes, e.g. "date" and "sed"

[Noam Postavsky]

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

Pierre Neidhardt <ambrevar@gmail.com> writes:

> Disabling eshell/date makes Eshell less portable on one system at least,
> that is Windows.  But what does "portability" mean in this context?  Are
> the coreutils meant to be part of Eshell?  Why?  Supporting `date' but not
> its arguments does not make up for actual portability I believe.  Case
> in point: I got fooled.
>
> Let's take the case of BSD vs. GNU: bash or zsh do not wrap around `ls',
> so the behaviour will not be the same on BSD and GNU.  Why should Eshell
> be any different?

Eshell isn't exactly the same as bash or zsh.  You can use M-x shell if
you prefer them.

We could fallback to the external command if given arguments.  This is
being done currently for other commands like eshell/rm (for unrecognized
arguments, that is).


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 1349 bytes --]

From ab25f638ccff0ebec36b78f9b47092fe9fb103b3 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Thu, 23 Nov 2017 07:51:13 -0500
Subject: [PATCH] eshell/date: use external date for any arguments (Bug#29157)

* lisp/eshell/em-unix.el (eshell/date): Throw `eshell-external' if
given any arguments.
(eshell-unix-initialize): Add "date" to `eshell-complex-commands'.
---
 lisp/eshell/em-unix.el | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/lisp/eshell/em-unix.el b/lisp/eshell/em-unix.el
index c486d2c51d..342a045d42 100644
--- a/lisp/eshell/em-unix.el
+++ b/lisp/eshell/em-unix.el
@@ -148,13 +148,18 @@ eshell-unix-initialize
   (make-local-variable 'eshell-complex-commands)
   (setq eshell-complex-commands
 	(append '("grep" "egrep" "fgrep" "agrep" "glimpse" "locate"
-		  "cat" "time" "cp" "mv" "make" "du" "diff")
+		  "cat" "date" "time" "cp" "mv" "make" "du" "diff")
 		eshell-complex-commands)))
 
-(defalias 'eshell/date     'current-time-string)
 (defalias 'eshell/basename 'file-name-nondirectory)
 (defalias 'eshell/dirname  'file-name-directory)
 
+(defun eshell/date (&rest args)
+  (when args
+    (throw 'eshell-external
+           (eshell-external-command "date" args)))
+  (current-time-string))
+
 (defvar em-interactive)
 (defvar em-preview)
 (defvar em-recursive)
-- 
2.11.0

bug#29157: 25.3; Eshell parsing fails sometimes, e.g. "date" and "sed"

[Eli Zaretskii]

> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Date: Thu, 23 Nov 2017 07:59:10 -0500
> Cc: 29157@debbugs.gnu.org
> 
> > Disabling eshell/date makes Eshell less portable on one system at least,
> > that is Windows.  But what does "portability" mean in this context?  Are
> > the coreutils meant to be part of Eshell?  Why?  Supporting `date' but not
> > its arguments does not make up for actual portability I believe.  Case
> > in point: I got fooled.
> >
> > Let's take the case of BSD vs. GNU: bash or zsh do not wrap around `ls',
> > so the behaviour will not be the same on BSD and GNU.  Why should Eshell
> > be any different?
> 
> Eshell isn't exactly the same as bash or zsh.  You can use M-x shell if
> you prefer them.
> 
> We could fallback to the external command if given arguments.  This is
> being done currently for other commands like eshell/rm (for unrecognized
> arguments, that is).

That doesn't sound right to me (for rm as well): it will fail in
strange ways for systems where the external command is absent or
deficient.

Eshell has both internal and external implementations because it wants
to be able to handle Lisp objects and Lisp-like syntax, not just
files, pipes, and other shell stuff.  So people who expect Eshell to
be just another shell are expecting something that Eshell was never
designed to be.  That's why Eshell offers the possibility to
optionally invoke the external implementation -- but it should be done
explicitly by the users, not by us second-guessing what they mean,
because reliably guessing which arguments are for an external command
and which for the internal Eshell implementation is impossible.
Observe:

	~/git/emacs/branch $ date 42
	Wed Dec 31 19:00:42 1969
But
	~/git/emacs/branch $ *date 42
	/bin/date: invalid date ‘42’

So I'm not sure such a naïve solution is TRT in this case, because we
are losing valuable features by doing that, and those features are not
just an accident, they were intentionally included in Eshell.

bug#29157: 25.3; Eshell parsing fails sometimes, e.g. "date" and "sed"

[Pierre Neidhardt]

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


Eli Zaretskii <eliz@gnu.org> writes:

> Observe:
>
> 	~/git/emacs/branch $ date 42
> 	Wed Dec 31 19:00:42 1969
> But
> 	~/git/emacs/branch $ *date 42
> 	/bin/date: invalid date ‘42’
>
> So I'm not sure such a naïve solution is TRT in this case, because we
> are losing valuable features by doing that, and those features are not
> just an accident, they were intentionally included in Eshell.

I think you are right.  I did not know that eshell/date could be used
this way.

The issue here is mostly my lack of awareness about what is an Elisp
command and what is a system program.

Maybe having different syntax highlighting for the "verb" depending on
whether it's a system program or an Elisp command would help avoiding
the pitfall.

Is there a trivial way to do this?  If not I'll work on it.  Maybe
create a package if that does not fit the philosophy of mainline Emacs.

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

bug#29157: 25.3; Eshell parsing fails sometimes, e.g. "date" and "sed"

[Michael Albinus]

Pierre Neidhardt <ambrevar@gmail.com> writes:

Hi Piere,

> The issue here is mostly my lack of awareness about what is an Elisp
> command and what is a system program.

You can always use `which' in the eshell buffer:

--8<---------------cut here---------------start------------->8---
~ $ which date
eshell/date is an alias for ‘current-time-string’ in ‘em-unix.el’.
~ $ which *date
/bin/date
--8<---------------cut here---------------end--------------->8---

And `which' itself is an eshell function:

--8<---------------cut here---------------start------------->8---
~ $ which which
eshell/which is a compiled Lisp function in ‘esh-cmd.el’.
~ $ which *which
/usr/bin/which
--8<---------------cut here---------------end--------------->8---

See also (info "(eshell) Built-ins")

Best regards, Michael.

bug#29157: 25.3; Eshell parsing fails sometimes, e.g. "date" and "sed"

[Pierre Neidhardt]

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


Of course, I know that, but I meant "off the top of my head".
Knowing which one is which requires some experience.

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

bug#29157: 25.3; Eshell parsing fails sometimes, e.g. "date" and "sed"

[Noam Postavsky]

Pierre Neidhardt <ambrevar@gmail.com> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> We could fallback to the external command if given arguments.  This is
>>> being done currently for other commands like eshell/rm (for unrecognized
>>> arguments, that is).

>> That doesn't sound right to me (for rm as well): it will fail in
>> strange ways for systems where the external command is absent or
>> deficient.

Currently eshell falls back to external command for unrecognized
arguments when passing :external to eshell-eval-using-options.  A quick
grep for :external brings up: rm, mkdir, rmdir, mv, cp, ln, cat, du,
time, env, ls.

>> Observe:
>>
>> 	~/git/emacs/branch $ date 42
>> 	Wed Dec 31 19:00:42 1969
>> But
>> 	~/git/emacs/branch $ *date 42
>> 	/bin/date: invalid date ‘42’
>>
>> So I'm not sure such a naïve solution is TRT in this case, because we
>> are losing valuable features by doing that,

So we could also check for an integer argument.

>> and those features are not
>> just an accident, they were intentionally included in Eshell.

Hmm, my impression of eshell is more like "throw a bunch of features at
the wall and see what sticks" but without the "see what sticks" part.

> The issue here is mostly my lack of awareness about what is an Elisp
> command and what is a system program.
>
> Maybe having different syntax highlighting for the "verb" depending on
> whether it's a system program or an Elisp command would help avoiding
> the pitfall.
>
> Is there a trivial way to do this?  If not I'll work on it.

There is no such feature, as far as I know.

bug#29157: 25.3; Eshell parsing fails sometimes, e.g. "date" and "sed"

[Eli Zaretskii]

> From: Pierre Neidhardt <ambrevar@gmail.com>
> Cc: Noam Postavsky <npostavs@users.sourceforge.net>, 29157@debbugs.gnu.org
> Date: Sat, 25 Nov 2017 18:54:51 +0100
> 
> > 	~/git/emacs/branch $ date 42
> > 	Wed Dec 31 19:00:42 1969
> > But
> > 	~/git/emacs/branch $ *date 42
> > 	/bin/date: invalid date ‘42’
> >
> > So I'm not sure such a naïve solution is TRT in this case, because we
> > are losing valuable features by doing that, and those features are not
> > just an accident, they were intentionally included in Eshell.
> 
> I think you are right.  I did not know that eshell/date could be used
> this way.

Not just eshell/date: many Eshell built-ins behave like that, and they
do that on purpose.

> The issue here is mostly my lack of awareness about what is an Elisp
> command and what is a system program.

Why do you need to know that?

If you want to know that so you could always get the same responses as
from another system shell, then perhaps we should have an option to
tell Eshell to always invoke an external program (maybe we already
have such an option, but I couldn't find it).

> Maybe having different syntax highlighting for the "verb" depending on
> whether it's a system program or an Elisp command would help avoiding
> the pitfall.

Isn't it true that a verb that doesn't begin with a '*' is _never_ a
system program in Eshell?

bug#29157: 25.3; Eshell parsing fails sometimes, e.g. "date" and "sed"

[Pierre Neidhardt]

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


Eli Zaretskii <eliz@gnu.org> writes:

>> The issue here is mostly my lack of awareness about what is an Elisp
>> command and what is a system program.
>
> Why do you need to know that?
>
> If you want to know that so you could always get the same responses as
> from another system shell, then perhaps we should have an option to
> tell Eshell to always invoke an external program (maybe we already
> have such an option, but I couldn't find it).

No, not like that, more like a friendly reminder: "this 'date' behaves
the Eshell way, while that 'rmdir' is the system program".

>> Maybe having different syntax highlighting for the "verb" depending on
>> whether it's a system program or an Elisp command would help avoiding
>> the pitfall.
>
> Isn't it true that a verb that doesn't begin with a '*' is _never_ a
> system program in Eshell?

I'm tempted to answer "no, it's not true", but we might be
misunderstood.

As far as I got it, the '*' is here to force Eshell to use the system
program, while no '*' tells Eshell to use its own version if available,
or the system program otherwise.

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

bug#29157: 25.3; Eshell parsing fails sometimes, e.g. "date" and "sed"

[Eli Zaretskii]

> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Cc: Eli Zaretskii <eliz@gnu.org>,  29157@debbugs.gnu.org
> Date: Sat, 25 Nov 2017 13:50:46 -0500
> 
> >> That doesn't sound right to me (for rm as well): it will fail in
> >> strange ways for systems where the external command is absent or
> >> deficient.
> 
> Currently eshell falls back to external command for unrecognized
> arguments when passing :external to eshell-eval-using-options.  A quick
> grep for :external brings up: rm, mkdir, rmdir, mv, cp, ln, cat, du,
> time, env, ls.

By "unrecognized arguments" do you mean unrecognized switches?  If so,
this is not the same issue as the one I was raising.

> >> Observe:
> >>
> >> 	~/git/emacs/branch $ date 42
> >> 	Wed Dec 31 19:00:42 1969
> >> But
> >> 	~/git/emacs/branch $ *date 42
> >> 	/bin/date: invalid date ‘42’
> >>
> >> So I'm not sure such a naïve solution is TRT in this case, because we
> >> are losing valuable features by doing that,
> 
> So we could also check for an integer argument.

But it isn't just integers:

  ~ $ date 42 "EDT-5"
  Thu Jan  1 05:00:42 1970
  ~ $ date (list 23065 51329 644000 0)
  Sat Nov 25 21:46:09 2017

> >> and those features are not
> >> just an accident, they were intentionally included in Eshell.
> 
> Hmm, my impression of eshell is more like "throw a bunch of features at
> the wall and see what sticks" but without the "see what sticks" part.

That's one way of looking at Eshell, although it's not my way.  Eshell
was intended to be used from Lisp programs, and that's why its
built-ins accept Lisp objects, values, and expressions as arguments.

bug#29157: 25.3; Eshell parsing fails sometimes, e.g. "date" and "sed"

[Eli Zaretskii]

> From: Pierre Neidhardt <ambrevar@gmail.com>
> Cc: npostavs@users.sourceforge.net, 29157@debbugs.gnu.org
> Date: Sat, 25 Nov 2017 20:36:36 +0100
> 
> > If you want to know that so you could always get the same responses as
> > from another system shell, then perhaps we should have an option to
> > tell Eshell to always invoke an external program (maybe we already
> > have such an option, but I couldn't find it).
> 
> No, not like that, more like a friendly reminder: "this 'date' behaves
> the Eshell way, while that 'rmdir' is the system program".

But the answer to that question depends on the arguments and sometimes
on the switches, doesn't it?  E.g., Eshell's 'rm' can delete processes
and buffers, and unintern symbols, in addition to deleting files.
What exactly it does depends on the arguments.  And if you invoke it
with -d switch, it will call the external program, but if you invoke
with -f or -i or -n, it will use the built-in.  So just given the
verb, I don't see how you can have that indication.

> > Isn't it true that a verb that doesn't begin with a '*' is _never_ a
> > system program in Eshell?
> 
> I'm tempted to answer "no, it's not true", but we might be
> misunderstood.
> 
> As far as I got it, the '*' is here to force Eshell to use the system
> program, while no '*' tells Eshell to use its own version if available,
> or the system program otherwise.

So you want to have an indication when there's _no_ built-in
implementation at all, is that it?

bug#29157: 25.3; Eshell parsing fails sometimes, e.g. "date" and "sed"

[Noam Postavsky]

Eli Zaretskii <eliz@gnu.org> writes:

> By "unrecognized arguments" do you mean unrecognized switches?  If so,
> this is not the same issue as the one I was raising.

Ah, I see.  I wan't really aware of that distinction.

>> So we could also check for an integer argument.
>
> But it isn't just integers:
>
>   ~ $ date 42 "EDT-5"
>   Thu Jan  1 05:00:42 1970
>   ~ $ date (list 23065 51329 644000 0)
>   Sat Nov 25 21:46:09 2017

Ugh, stop taking me so literally, and just read my mind! ;P

How about checking for a set of arguments that is compatible with what
current-time-string accepts.

bug#29157: 25.3; Eshell parsing fails sometimes, e.g. "date" and "sed"

[Eli Zaretskii]

> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Cc: 29157@debbugs.gnu.org,  ambrevar@gmail.com
> Date: Sat, 25 Nov 2017 15:06:01 -0500
> 
> >   ~ $ date 42 "EDT-5"
> >   Thu Jan  1 05:00:42 1970
> >   ~ $ date (list 23065 51329 644000 0)
> >   Sat Nov 25 21:46:09 2017
> 
> Ugh, stop taking me so literally, and just read my mind! ;P
> 
> How about checking for a set of arguments that is compatible with what
> current-time-string accepts.

Is it really possible to do that reliably?

Or maybe you meant to catch errors signaled by current-time-string,
and invoke the external command then.  If so, I'd agree.  But we
should then allow customization of the external command's name,
because on Windows it will be something like 'gdate', to avoid calling
the incompatible Windows shell's built-in (which also prompts
interactively for input).

bug#29157: 25.3; Eshell parsing fails sometimes, e.g. "date" and "sed"

[Noam Postavsky]

Eli Zaretskii <eliz@gnu.org> writes:

>> How about checking for a set of arguments that is compatible with what
>> current-time-string accepts.
>
> Is it really possible to do that reliably?

Should be okay if we err on the side of current-time-string:

  (pcase (cl-list* (nth 0 args) (nth 1 args) (nthcdr 2 args)))
    (`(,(or (pred listp) (pred integerp))
       ,(or 'nil 't 'wall (pred stringp)))
     t))

> Or maybe you meant to catch errors signaled by current-time-string,

That's another possibility, it would be more precise.

> and invoke the external command then.  If so, I'd agree.  But we
> should then allow customization of the external command's name,
> because on Windows it will be something like 'gdate', to avoid calling
> the incompatible Windows shell's built-in (which also prompts
> interactively for input).

I'm not in front of a Windows box at the moment, but I thought a cmd.exe
builtin like that could only be invoked by doing calling "cmd /C date".

bug#29157: 25.3; Eshell parsing fails sometimes, e.g. "date" and "sed"

[John Wiegley]

>>>>> "EZ" == Eli Zaretskii <eliz@gnu.org> writes:

EZ> Not just eshell/date: many Eshell built-ins behave like that, and they do
EZ> that on purpose.

eshell/date is an alias for `current-time-string'. The fact that this Lisp
function accepts arguments, and those arguments can be passed on the Eshell
command-line, isn't something I thought of at the time.

In hindsight, a separate eshell/date function should have been created that
would handle its arguments like the system command.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

bug#29157: 25.3; Eshell parsing fails sometimes, e.g. "date" and "sed"

[John Wiegley]

>>>>> "EZ" == Eli Zaretskii <eliz@gnu.org> writes:

EZ> That's one way of looking at Eshell, although it's not my way. Eshell was
EZ> intended to be used from Lisp programs, and that's why its built-ins
EZ> accept Lisp objects, values, and expressions as arguments.

Eshell was intended to be a UNIX-like environment on non-UNIX systems, and as
a bonus to provide useful interactions with Lisp. It wasn't built first as a
Lisp interaction mode.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

bug#29157: 25.3; Eshell parsing fails sometimes, e.g. "date" and "sed"

[Eli Zaretskii]

> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Cc: 29157@debbugs.gnu.org,  ambrevar@gmail.com
> Date: Sat, 25 Nov 2017 16:41:52 -0500
> 
> > and invoke the external command then.  If so, I'd agree.  But we
> > should then allow customization of the external command's name,
> > because on Windows it will be something like 'gdate', to avoid calling
> > the incompatible Windows shell's built-in (which also prompts
> > interactively for input).
> 
> I'm not in front of a Windows box at the moment, but I thought a cmd.exe
> builtin like that could only be invoked by doing calling "cmd /C date".

By default, yes.  But people tend to do weird things with shell setup
in Emacs.  And customization is an opt-in feature, so the default is
unaffected.

bug#29157: 25.3; Eshell parsing fails sometimes, e.g. "date" and "sed"

[Pierre Neidhardt]

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


Eli Zaretskii <eliz@gnu.org> writes:

>> > If you want to know that so you could always get the same responses as
>> > from another system shell, then perhaps we should have an option to
>> > tell Eshell to always invoke an external program (maybe we already
>> > have such an option, but I couldn't find it).
>>
>> No, not like that, more like a friendly reminder: "this 'date' behaves
>> the Eshell way, while that 'rmdir' is the system program".
>
> But the answer to that question depends on the arguments and sometimes
> on the switches, doesn't it?  E.g., Eshell's 'rm' can delete processes
> and buffers, and unintern symbols, in addition to deleting files.
> What exactly it does depends on the arguments.  And if you invoke it
> with -d switch, it will call the external program, but if you invoke
> with -f or -i or -n, it will use the built-in.  So just given the
> verb, I don't see how you can have that indication.

Wow, I did not know that.  This is not documented in the docstring, but
I just saw it is mentioned in the help message.

That maybe it the root of the issue: what's the standard way of
documenting 'eshell/*' commands?

I think both `-h' and `C-h f' should document the same thing, it's
confusing otherwise.  Lest users suffer too much from the "Where did I
find that valuable help again?" syndrom.

>> > Isn't it true that a verb that doesn't begin with a '*' is _never_ a
>> > system program in Eshell?
>>
>> I'm tempted to answer "no, it's not true", but we might be
>> misunderstood.
>>
>> As far as I got it, the '*' is here to force Eshell to use the system
>> program, while no '*' tells Eshell to use its own version if available,
>> or the system program otherwise.
>
> So you want to have an indication when there's _no_ built-in
> implementation at all, is that it?

No.  Basically if I write "rm" in Eshell, Eshell will _always_ call
eshell/rm.  Only afterwards it will make a call to /bin/rm, depending on
the arguments.
As a user, what I want to know is what Eshell will call _first_, because
then I can know the starting point of what Eshell is going to do.

Basically, my idea is simple:

- If 'eshell/foo' exists, use some eshell-builtin face on "foo". The
user will then know that s/he should lookup the documentation of
eshell/foo.

- Otherwise use the normal face.  The user will then refer to the man
  page and the like.

--
Pierre Neidhardt

If the grass is greener on other side of fence, consider what may be
fertilizing it.

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

bug#29157: 25.3; Eshell parsing fails sometimes, e.g. "date" and "sed"

[Eli Zaretskii]

> From: "John Wiegley" <johnw@gnu.org>
> Cc: Pierre Neidhardt <ambrevar@gmail.com>,  29157@debbugs.gnu.org,  npostavs@users.sourceforge.net
> Date: Sat, 25 Nov 2017 19:21:01 -0800
> 
> >>>>> "EZ" == Eli Zaretskii <eliz@gnu.org> writes:
> 
> EZ> Not just eshell/date: many Eshell built-ins behave like that, and they do
> EZ> that on purpose.
> 
> eshell/date is an alias for `current-time-string'. The fact that this Lisp
> function accepts arguments, and those arguments can be passed on the Eshell
> command-line, isn't something I thought of at the time.

Maybe not in this case, but other Eshell commands definitely feature
similar behaviors, and there you cannot convince me it's an accident,
because the behavior is documented in the doc string.

> In hindsight, a separate eshell/date function should have been created that
> would handle its arguments like the system command.

There is such a function: *date.

bug#29157: 25.3; Eshell parsing fails sometimes, e.g. "date" and "sed"

[Eli Zaretskii]

> From: "John Wiegley" <johnw@gnu.org>
> Cc: Noam Postavsky <npostavs@users.sourceforge.net>,  29157@debbugs.gnu.org,  ambrevar@gmail.com
> Date: Sat, 25 Nov 2017 19:21:49 -0800
> 
> >>>>> "EZ" == Eli Zaretskii <eliz@gnu.org> writes:
> 
> EZ> That's one way of looking at Eshell, although it's not my way. Eshell was
> EZ> intended to be used from Lisp programs, and that's why its built-ins
> EZ> accept Lisp objects, values, and expressions as arguments.
> 
> Eshell was intended to be a UNIX-like environment on non-UNIX systems, and as
> a bonus to provide useful interactions with Lisp. It wasn't built first as a
> Lisp interaction mode.

Well, maybe historically I'm wrong, but factually I'm right: what we
have now is a shell that can act on Lisp objects and Lisp expressions,
and this fact is even documented in the manual.  I personally find
this feature useful and a lot of fun, and I think it would be a pity
to lose it.

bug#29157: 25.3; Eshell parsing fails sometimes, e.g. "date" and "sed"

[Eli Zaretskii]

> From: Pierre Neidhardt <ambrevar@gmail.com>
> Cc: npostavs@users.sourceforge.net, 29157@debbugs.gnu.org
> Date: Sun, 26 Nov 2017 10:17:30 +0100
> 
> > But the answer to that question depends on the arguments and sometimes
> > on the switches, doesn't it?  E.g., Eshell's 'rm' can delete processes
> > and buffers, and unintern symbols, in addition to deleting files.
> > What exactly it does depends on the arguments.  And if you invoke it
> > with -d switch, it will call the external program, but if you invoke
> > with -f or -i or -n, it will use the built-in.  So just given the
> > verb, I don't see how you can have that indication.
> 
> Wow, I did not know that.  This is not documented in the docstring, but
> I just saw it is mentioned in the help message.
> 
> That maybe it the root of the issue: what's the standard way of
> documenting 'eshell/*' commands?
> 
> I think both `-h' and `C-h f' should document the same thing, it's
> confusing otherwise.

Patches to that effect are welcome.

> > So you want to have an indication when there's _no_ built-in
> > implementation at all, is that it?
> 
> No.  Basically if I write "rm" in Eshell, Eshell will _always_ call
> eshell/rm.  Only afterwards it will make a call to /bin/rm, depending on
> the arguments.
> As a user, what I want to know is what Eshell will call _first_, because
> then I can know the starting point of what Eshell is going to do.
> 
> Basically, my idea is simple:
> 
> - If 'eshell/foo' exists, use some eshell-builtin face on "foo". The
> user will then know that s/he should lookup the documentation of
> eshell/foo.
> 
> - Otherwise use the normal face.  The user will then refer to the man
>   page and the like.

Well, I think you said the same thing I did, just from a different
POV.  So we are in agreement.  Feel free to work on such a feature.

bug#29157: 25.3; Eshell parsing fails sometimes, e.g. "date" and "sed"

[John Wiegley]

>>>>> "EZ" == Eli Zaretskii <eliz@gnu.org> writes:

EZ> Well, maybe historically I'm wrong, but factually I'm right: what we have
EZ> now is a shell that can act on Lisp objects and Lisp expressions, and this
EZ> fact is even documented in the manual. I personally find this feature
EZ> useful and a lot of fun, and I think it would be a pity to lose it.

Sure, very much agreed, it was an unintended result but a welcome one. :)

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

bug#29157: 25.3; Eshell parsing fails sometimes, e.g. "date" and "sed"

[John Wiegley]

>>>>> Eli Zaretskii <eliz@gnu.org> writes:

> Maybe not in this case, but other Eshell commands definitely feature similar
> behaviors, and there you cannot convince me it's an accident, because the
> behavior is documented in the doc string.

Wait, not trying to convince you of anything, just responding to what
eshell/date acts weird when given arguments. It's "news to me" in other words.

>> In hindsight, a separate eshell/date function should have been created that
>> would handle its arguments like the system command.

> There is such a function: *date.

I mean, an implementation that works when no system "date" command exists, yet
accepts arguments similar to what system date accepts.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

bug#29157: 25.3; Eshell parsing fails sometimes, e.g. "date" and "sed"

[Eli Zaretskii]

> From: John Wiegley <johnw@gnu.org>
> Cc: ambrevar@gmail.com,  29157@debbugs.gnu.org,  npostavs@users.sourceforge.net
> Date: Sun, 26 Nov 2017 13:45:30 -0800
> 
> >> In hindsight, a separate eshell/date function should have been created that
> >> would handle its arguments like the system command.
> 
> > There is such a function: *date.
> 
> I mean, an implementation that works when no system "date" command exists, yet
> accepts arguments similar to what system date accepts.

If someone wants to work on Eshell's 'date' so that it accepts more
complicated arguments that the Coreutils version does, they should
feel free, of course.

bug#29157: 25.3; Eshell parsing fails sometimes, e.g. "date" and "sed"

[Noam Postavsky]

close 29157 27.1
quit

Pierre Neidhardt <ambrevar@gmail.com> writes:

>> I agree.  Although the expansion in this case is arguably a bug (as
>> Andreas pointed out), I don't have much interest in fixing it.  I
>> propose just to disable it by default (in master).
>
> Thank you.

Pushed to master [1: 1cdd0e8cd8].  Closing the bug as it's basically
"working as designed" (although enhanced versions of eshell/date would
be welcomed as mentioned earlier on this  bug thread).

[1: 1cdd0e8cd8]: 2017-12-03 15:39:02 -0500
  Disable history expansion in eshell (Bug#29157)
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=1cdd0e8cd801aa1d6f04ab4d8e6097a46af8c951

bug#29157: 25.3; Eshell parsing fails sometimes, e.g. "date" and "sed"

[John Wiegley]

>>>>> "NP" == Noam Postavsky <npostavs@users.sourceforge.net> writes:

NP> Pushed to master [1: 1cdd0e8cd8]. Closing the bug as it's basically
NP> "working as designed" (although enhanced versions of eshell/date would be
NP> welcomed as mentioned earlier on this bug thread).

Hi Noam, if would have been nice for you to discuss your change in this thread
before pushing it.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

bug#29157: 25.3; Eshell parsing fails sometimes, e.g. "date" and "sed"

[Noam Postavsky]

"John Wiegley" <johnw@gnu.org> writes:

>>>>>> "NP" == Noam Postavsky <npostavs@users.sourceforge.net> writes:
>
> NP> Pushed to master [1: 1cdd0e8cd8]. Closing the bug as it's basically
> NP> "working as designed" (although enhanced versions of eshell/date would be
> NP> welcomed as mentioned earlier on this bug thread).
>
> Hi Noam, if would have been nice for you to discuss your change in this thread
> before pushing it.

Hmm, I believe I have discussed it, see:

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=29157#8  // initial post of the change
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=29157#11 // Pierre agreed with it
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=29157#20 // Full change with NEWS posted
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=29157#23 // Pierre agreed again
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=29157#95 // Pushed after more than 1 week and no objections

bug#29157: 25.3; Eshell parsing fails sometimes, e.g. "date" and "sed"

[John Wiegley]

>>>>> "EZ" == Eli Zaretskii <eliz@gnu.org> writes:

EZ> If someone wants to work on Eshell's 'date' so that it accepts more
EZ> complicated arguments that the Coreutils version does, they should feel
EZ> free, of course.

I imagine it would start somewhere like this:

--8<---------------cut here---------------start------------->8---
(defun eshell/date (&rest args)
  "Implementation of date in Lisp."
  (setq args (eshell-flatten-list args))
  (eshell-eval-using-options
   "rm" args
   '(
     (?d  "date"      nil nil "display time described by STRING, not 'now'")
     (nil "debug"     nil nil "annotate the parsed date and warn about questionable usage")
     (?f  "file"      nil nil "like --date; once for each line of DATEFILE")
     (?I  "iso-8601"  nil nil "output date/time in ISO 8601 format")
     (?R  "rfc-email" nil nil "output date and time in RFC 5322 format")
     (nil "rfc-3339"  nil nil "output date/time in RFC 3339 format")
     (?r  "reference" nil nil "display the last modification time of FILE")
     (?s  "set"       nil nil "set time described by STRING")
     (?u  "utc"       nil nil "print or set Coordinated Universal Time (UTC)")
     (nil "universal" nil nil "print or set Coordinated Universal Time (UTC)")
     (nil "help"      nil nil "display this help and exit")
     (nil "version"   nil nil "output version information and exit")
     :preserve-args
     :external "date"
     :show-usage
     :usage "[OPTION]... [+FORMAT]
Display the current time in the given FORMAT, or set the system date.")
   (while args
     (let ((entry (car args)))
       )
     (setq args (cdr args)))
   nil))
--8<---------------cut here---------------end--------------->8---

--
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2