GNU bug report logs - #28568
26.0.60; [eshell] Incompatible change in alias argument handling

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 28568 in the body.
You can then email your comments to 28568 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> users.sourceforge.net>
To: bug-gnu-emacs <at> gnu.org
Subject: 26.0.60; [eshell] Incompatible change in alias argument handling
Date: Sat, 23 Sep 2017 09:21:47 -0400

> Jay Kamat <jaygkamat <at> gmail.com> writes:
>
>> It seems that the fix to
>> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=27954 (commit
>> e66e81679c3c91d6bf8f62c7abcd968430b4d1fe) caused this issue. I had an
>> eshell alias defined as
>>
>> alias sudo eshell/sudo $*
>>
>> Which seems to no longer work (with the error described).

Right.  The problem is that the arguments are now getting submitted twice.

sudo ls -> {{eshell/sudo $*}} ls -> eshell/sudo ls ls

I'm thinking we should probably revert the fix for Bug#27954; the
comments at the top of em-alias.el explicitly say using '$*' (and '$1',
'$2') in the alias definition is the supported way to handle arguments.

;;;_* Creating aliases
;;
;; The user interface is simple: type 'alias' followed by the command
;; name followed by the definition.  Argument references are made
;; using '$1', '$2', etc., or '$*'.  For example:
;;
;;   alias ll 'ls -l $*'
;;
;; This will cause the command 'll NEWS' to be replaced by 'ls -l
;; NEWS'.  This is then passed back to the command parser for
;; reparsing.{Only the command text specified in the alias definition
;; will be reparsed.  Argument references (such as '$*') are handled
;; using variable values, which means that the expansion will not be
;; reparsed, but used directly.}

The manual is a bit inconsistent about it, with `(eshell) Built-ins'
suggesting

    alias sudo '*sudo $*'

and `(eshell) Aliases' suggesting

    alias ll ls -l

I think this just means we should correct the `(eshell) Aliases' page.


Michael Albinus <michael.albinus <at> gmx.de> writes:

> There's also bug#28320, which seems to report the same problem.

No, that is a different problem, I've opened a new bug for this one.

Message #8 received at 28568 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> users.sourceforge.net>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: help-gnu-emacs <at> gnu.org, Jay Kamat <jaygkamat <at> gmail.com>,
 28568 <at> debbugs.gnu.org
Subject: Re: bug#28568: 26.0.60;
 [eshell] Incompatible change in alias argument handling (Was:
 Possible Eshell/Tramp Bug in Emacs 26)
Date: Sat, 23 Sep 2017 10:55:55 -0400

> Jay Kamat <jaygkamat <at> gmail.com> writes:
>>
>> This is a little bit annoying since it means that I can't share these
>> aliases across emacs25 and emacs26 without problems. One solution I
>> could do is setting `eshell-prefer-lisp-functions' instead of using this
>> alias, but I would have liked to only override sudo (and leave the rest
>> as system). If anyone knows a better solution, let me know!

Another possible alternative is adding "sudo" to
`eshell-complex-commands' (adding `eshell-tramp' to
`eshell-modules-list' does this, see `eshell-tramp-initialize').

    eshell-complex-commands is a variable defined in ‘esh-cmd.el’.
    Its value is ("ls")

      This variable may be risky if used as a file-local variable.

    Documentation:
    A list of commands names or functions, that determine complexity.
    That is, if a command is defined by a function named eshell/NAME,
    and NAME is part of this list, it is invoked as a complex command.
    Complex commands are always correct, but run much slower.  If a
    command works fine without being part of this list, then it doesn’t
    need to be.

    If an entry is a function, it will be called with the name, and should
    return non-nil if the command is complex.

    You can customize this variable.

>> Also, it would be nice if eshell aliases and other configuration were
>> not loaded in an emacs -Q setting.

Yes, that would make sense too.

Message #11 received at 28568 <at> debbugs.gnu.org (full text, mbox):

From: Jay Kamat <jaygkamat <at> gmail.com>
To: Noam Postavsky <npostavs <at> users.sourceforge.net>
Cc: 28568 <at> debbugs.gnu.org
Subject: Re: bug#28568: 26.0.60;
 [eshell] Incompatible change in alias argument handling
Date: Wed, 27 Sep 2017 00:28:24 -0400

[Message part 1 (text/plain, inline)]

Hi!

I was getting quite annoyed with this bug (since it breaks all my eshell
aliases), and I would really hate to see this bug in the emacs 26
release, so I wanted to push it along. I've attached a patch that simply
reverts the fix to #27954 (commit e66e81679c), which seems to solve the
problem.

From bug #27954, the issue was:

> In eshell:
>
> alias ll ls -l
>
> gives a full listing of all files as result of the following eshell-command:
>
> ll test.txt
>
> But, I expect only the listing for the file test.txt.

which can be fixed by changing the alias to:

alias ll 'ls -l $*'

which works fine for me in both cases ('ll' and 'll test.txt').

All this patch is doing is reverting commit e66e81679c and updating the
alias documentation, so feel free to remake and commit this yourself if
you wish (or if I'm doing it wrong).

Thanks,
-Jay

[0001-Revert-change-to-eshell-alias-argument-parsing-Bug-2.patch (text/x-diff, attachment)]

Message #14 received at 28568 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> users.sourceforge.net>
To: Jay Kamat <jaygkamat <at> gmail.com>
Cc: 28568 <at> debbugs.gnu.org
Subject: Re: bug#28568: 26.0.60;
 [eshell] Incompatible change in alias argument handling
Date: Wed, 27 Sep 2017 08:55:34 -0400

block 24655 by 28568 
quit

Jay Kamat <jaygkamat <at> gmail.com> writes:

> I was getting quite annoyed with this bug (since it breaks all my eshell
> aliases), and I would really hate to see this bug in the emacs 26
> release

Yeah, it would be quite bad to have this in a release since then we'd
have to say something like "for Emacs 26 change your aliases this way,
but then for other versions change them back that way (and if you're
switching between versions, tough luck)".  I've marked this as release
blocking so we don't forget.

> All this patch is doing is reverting commit e66e81679c and updating the
> alias documentation, so feel free to remake and commit this yourself if
> you wish (or if I'm doing it wrong).

Yes, that's basically what I had in mind, although I'll probably add a
bit more explanation to the doc.

Message #19 received at 28568 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> users.sourceforge.net>
To: Jay Kamat <jaygkamat <at> gmail.com>
Cc: 28568 <at> debbugs.gnu.org
Subject: Re: bug#28568: 26.0.60;
 [eshell] Incompatible change in alias argument handling
Date: Fri, 29 Sep 2017 21:45:24 -0400

[Message part 1 (text/plain, inline)]

tags 28568 + patch
quit

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

>> All this patch is doing is reverting commit e66e81679c and updating the
>> alias documentation, so feel free to remake and commit this yourself if
>> you wish (or if I'm doing it wrong).
>
> Yes, that's basically what I had in mind, although I'll probably add a
> bit more explanation to the doc.

Here it is, not sure if I've got the texinfo formatting right.  I'm not
really clear on the difference between @samp{}, @command{}, and @code{}.

[0001-Revert-Don-t-lose-arguments-to-eshell-aliases-Bug-27.patch (text/x-diff, inline)]

From a154f4dbb09b24e4fdba69991c97e2a7a8d8581e Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs <at> gmail.com>
Date: Fri, 29 Sep 2017 21:00:10 -0400
Subject: [PATCH] Revert "Don't lose arguments to eshell aliases (Bug#27954)"

It broke the established argument handling methods provided by eshell
aliases (Bug#28568).
* doc/misc/eshell.texi (Aliases): Fix example, call out use of
arguments in aliases.
* lisp/eshell/em-alias.el (eshell-maybe-replace-by-alias): Ignore
ARGS.
---
 doc/misc/eshell.texi    | 9 ++++++++-
 lisp/eshell/em-alias.el | 6 +++---
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index 8963826c4c..8dff739612 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -431,13 +431,20 @@ Aliases
 
 Aliases are commands that expand to a longer input line.  For example,
 @command{ll} is a common alias for @code{ls -l}, and would be defined
-with the command invocation @samp{alias ll ls -l}; with this defined,
+with the command invocation @samp{alias ll 'ls -l $*'}; with this defined,
 running @samp{ll foo} in Eshell will actually run @samp{ls -l foo}.
 Aliases defined (or deleted) by the @command{alias} command are
 automatically written to the file named by @code{eshell-aliases-file},
 which you can also edit directly (although you will have to manually
 reload it).
 
+Note that unlike aliases in Bash, arguments must be handled
+explicitly.  Typically the alias definition would end in @samp{$*} to
+pass all arguments along.  More selective use of arguments via
+@samp{$1}, @samp{$2}, etc., is also possible.  For example,
+@samp{alias mcd 'mkdir $1 && cd $1'} would cause @samp{mcd foo} to
+create and switch to a directory called @samp{foo}.
+
 @node History
 @section History
 @cmindex history
diff --git a/lisp/eshell/em-alias.el b/lisp/eshell/em-alias.el
index f951efa65d..742234574f 100644
--- a/lisp/eshell/em-alias.el
+++ b/lisp/eshell/em-alias.el
@@ -214,8 +214,8 @@ eshell-lookup-alias
 
 (defvar eshell-prevent-alias-expansion nil)
 
-(defun eshell-maybe-replace-by-alias (command args)
-  "If COMMAND has an alias definition, call that instead using ARGS."
+(defun eshell-maybe-replace-by-alias (command _args)
+  "Call COMMAND's alias definition, if it exists."
   (unless (and eshell-prevent-alias-expansion
 	       (member command eshell-prevent-alias-expansion))
     (let ((alias (eshell-lookup-alias command)))
@@ -225,7 +225,7 @@ eshell-maybe-replace-by-alias
                         (eshell-command-arguments ',eshell-last-arguments)
                         (eshell-prevent-alias-expansion
                          ',(cons command eshell-prevent-alias-expansion)))
-                    ,(eshell-parse-command (nth 1 alias) args)))))))
+                    ,(eshell-parse-command (nth 1 alias))))))))
 
 (defun eshell-alias-completions (name)
   "Find all possible completions for NAME.
-- 
2.11.0

Message #24 received at 28568 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Noam Postavsky <npostavs <at> users.sourceforge.net>
Cc: jaygkamat <at> gmail.com, 28568 <at> debbugs.gnu.org
Subject: Re: bug#28568: 26.0.60;
 [eshell] Incompatible change in alias argument handling
Date: Sat, 30 Sep 2017 11:06:06 +0300

> From: Noam Postavsky <npostavs <at> users.sourceforge.net>
> Date: Fri, 29 Sep 2017 21:45:24 -0400
> Cc: 28568 <at> debbugs.gnu.org
> 
> Here it is, not sure if I've got the texinfo formatting right.  I'm not
> really clear on the difference between @samp{}, @command{}, and @code{}.

Commands typed by the user should be in @kbd.  Here, for example:

>  Aliases are commands that expand to a longer input line.  For example,
>  @command{ll} is a common alias for @code{ls -l}, and would be defined
> -with the command invocation @samp{alias ll ls -l}; with this defined,
> +with the command invocation @samp{alias ll 'ls -l $*'}; with this defined,
                               ^^^^^^^^^^^^^^^^^^^^^^^^^^

@command is the markup for shell commands, @code is for symbols, and
@samp for a sequence ("sample") of text characters.

> +Note that unlike aliases in Bash, arguments must be handled
> +explicitly.  Typically the alias definition would end in @samp{$*} to
> +pass all arguments along.  More selective use of arguments via
> +@samp{$1}, @samp{$2}, etc., is also possible.  For example,
> +@samp{alias mcd 'mkdir $1 && cd $1'} would cause @samp{mcd foo} to
   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^             ^^^^^^^^^^^^^^
These two should use @kbd.

Thanks.

Message #27 received at 28568 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> users.sourceforge.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: jaygkamat <at> gmail.com, 28568 <at> debbugs.gnu.org
Subject: Re: bug#28568: 26.0.60;
 [eshell] Incompatible change in alias argument handling
Date: Sat, 30 Sep 2017 20:12:57 -0400

tags 28568 fixed
close 28568
quit

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

>> From: Noam Postavsky <npostavs <at> users.sourceforge.net>
>> Date: Fri, 29 Sep 2017 21:45:24 -0400
>> Cc: 28568 <at> debbugs.gnu.org
>> 
>> Here it is, not sure if I've got the texinfo formatting right.  I'm not
>> really clear on the difference between @samp{}, @command{}, and @code{}.
>
> Commands typed by the user should be in @kbd.

Fixed and pushed to emacs-26.

[1: ba9139c501]: 2017-09-30 20:01:33 -0400
  Revert "Don't lose arguments to eshell aliases (Bug#27954)"
  http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=ba9139c501ed8220980e898f127e293e8f263ea1

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.