Possible Eshell/Tramp Bug in Emacs 26

Possible Eshell/Tramp Bug in Emacs 26

[Jay Kamat]

Hi!

I'm not entirely sure if this is a problem on my end or not, so just to
be safe, I'm sending this to help-gnu-emacs rather than emacs-devel, if
this is an actual bug, feel free to forward this there.

On emacs 25:

1. Start emacs -Q
2. Evaluate (require 'em-tramp)
3. M-x eshell
4. run 'sudo ls'
5. after logging in via tramp, output of ls is displayed correctly 

On emacs 26 (d24ec5854098841388dfecf2c668e7f48f348af0):

1. Start emacs -Q
2. Evaluate (require 'em-tramp)
3. M-x eshell
4. run 'sudo ls'
5. after logging in via tramp, output of ls is:

'ls: No such file or directory'.

Can anyone else reproduce this as well, or is this just me (or has there
been a relevant recent change)? I can try to bisect this if that would
be useful.

Thanks,
-Jay

Re: Possible Eshell/Tramp Bug in Emacs 26

[Jay Kamat]

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).

Now, the entry:

alias sudo eshell/sudo

works for my purposes instead.

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!

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

-Jay

bug#28320: Possible Eshell/Tramp Bug in Emacs 26

[Michael Albinus]

Jay Kamat <jaygkamat@gmail.com> writes:

Hi Jay,

> 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).

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

> Now, the entry:
>
> alias sudo eshell/sudo
>
> works for my purposes instead.
>
> 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!
>
> Also, it would be nice if eshell aliases and other configuration were
> not loaded in an emacs -Q setting.

Noam, could you have a look on this?

> -Jay

Best regards, Michael.

bug#28568: 26.0.60; [eshell] Incompatible change in alias argument handling

[Noam Postavsky]

> Jay Kamat <jaygkamat@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@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.

bug#28568: 26.0.60; [eshell] Incompatible change in alias argument handling (Was: Possible Eshell/Tramp Bug in Emacs 26)

[Noam Postavsky]

> Jay Kamat <jaygkamat@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.

bug#28568: 26.0.60; [eshell] Incompatible change in alias argument handling

[Jay Kamat]

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

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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Revert-change-to-eshell-alias-argument-parsing-Bug-2.patch --]
[-- Type: text/x-diff, Size: 2017 bytes --]

From fd4a8b3b2b42dbf1520a6cd8cc3b265f55ea6ca6 Mon Sep 17 00:00:00 2001
From: Jay Kamat <jaygkamat@gmail.com>
Date: Wed, 27 Sep 2017 00:01:23 -0400
Subject: [PATCH] Revert change to eshell alias argument parsing (Bug#28568)

Update documentation of 'll' example alias to minimize confusion

This reverts commit e66e81679c3c91d6bf8f62c7abcd968430b4d1fe.
---
 doc/misc/eshell.texi    | 6 +++---
 lisp/eshell/em-alias.el | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index 8963826c4c..be8e659544 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -431,9 +431,9 @@ 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,
-running @samp{ll foo} in Eshell will actually run @samp{ls -l foo}.
-Aliases defined (or deleted) by the @command{alias} command are
+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).
diff --git a/lisp/eshell/em-alias.el b/lisp/eshell/em-alias.el
index f951efa65d..80b14a9b93 100644
--- a/lisp/eshell/em-alias.el
+++ b/lisp/eshell/em-alias.el
@@ -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

bug#28568: 26.0.60; [eshell] Incompatible change in alias argument handling

[Noam Postavsky]

block 24655 by 28568 
quit

Jay Kamat <jaygkamat@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.

bug#28568: 26.0.60; [eshell] Incompatible change in alias argument handling

[Noam Postavsky]

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

tags 28568 + patch
quit

Noam Postavsky <npostavs@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{}.


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

From a154f4dbb09b24e4fdba69991c97e2a7a8d8581e Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@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

bug#28568: 26.0.60; [eshell] Incompatible change in alias argument handling

[Eli Zaretskii]

> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Date: Fri, 29 Sep 2017 21:45:24 -0400
> Cc: 28568@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.

bug#28568: 26.0.60; [eshell] Incompatible change in alias argument handling

[Noam Postavsky]

tags 28568 fixed
close 28568
quit

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Noam Postavsky <npostavs@users.sourceforge.net>
>> Date: Fri, 29 Sep 2017 21:45:24 -0400
>> Cc: 28568@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