unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#30724: eshell: escaped tilde is not treated as such
@ 2018-03-06  4:30 Yegor Timoshenko
  2018-03-09  1:15 ` Noam Postavsky
  2018-07-06  7:24 ` Jonathan Kyle Mitchell
  0 siblings, 2 replies; 8+ messages in thread
From: Yegor Timoshenko @ 2018-03-06  4:30 UTC (permalink / raw)
  To: 30724

In M-x eshell (do not run):

  $ touch \~
  $ ls
  ~
  $ rm \~
  rm: cannot remove '~': Is a directory

I.e. it tried to remove home directory. I've found this by
accidentially wiping my home directory, but no worry, I've restored a
btrfs snapshot that was made several minutes before that.

GNU Emacs 27.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.22.26)
 of 2018-02-25





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

* bug#30724: eshell: escaped tilde is not treated as such
  2018-03-06  4:30 bug#30724: eshell: escaped tilde is not treated as such Yegor Timoshenko
@ 2018-03-09  1:15 ` Noam Postavsky
  2018-07-06  7:24 ` Jonathan Kyle Mitchell
  1 sibling, 0 replies; 8+ messages in thread
From: Noam Postavsky @ 2018-03-09  1:15 UTC (permalink / raw)
  To: Yegor Timoshenko; +Cc: 30724

tags 30724 + confirmed
quit

Yegor Timoshenko <yegortimoshenko@riseup.net> writes:

> In M-x eshell (do not run):
>
>   $ touch \~
>   $ ls
>   ~
>   $ rm \~
>   rm: cannot remove '~': Is a directory
>
> I.e. it tried to remove home directory. I've found this by
> accidentially wiping my home directory, but no worry, I've restored a
> btrfs snapshot that was made several minutes before that.
>
> GNU Emacs 27.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.22.26)
>  of 2018-02-25

Stepping through eshell/rm a bit, I see it gets #("~" 0 1 (escaped t))
as an argument.  So each command is responsible for handling the
escaping correctly.  I think every file-handling eshell function needs
review for this.





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

* bug#30724: eshell: escaped tilde is not treated as such
  2018-03-06  4:30 bug#30724: eshell: escaped tilde is not treated as such Yegor Timoshenko
  2018-03-09  1:15 ` Noam Postavsky
@ 2018-07-06  7:24 ` Jonathan Kyle Mitchell
  2018-07-07 19:17   ` Noam Postavsky
  1 sibling, 1 reply; 8+ messages in thread
From: Jonathan Kyle Mitchell @ 2018-07-06  7:24 UTC (permalink / raw)
  To: 30724; +Cc: Noam Postavsky, ; Yegor Timoshenko

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

tags 30724 + patch
quit

I found a way to get eshell to escape special chars.  It seems most of
the eshell builtin commands that take file arguments are already tagged
with the eshell-no-numeric-conversions property.  By adding the true
part of the if condition in eshell-lisp-command, it is possible to
quote the arguments of special filenames.  The attached patch checks
for "~" and "*".

;; for reference, here's the set of eshell builtins with the
;; eshell-no-numeric-conversions property
./esh-proc.el\0202:(put 'eshell/kill 'eshell-no-numeric-conversions t)
./em-dirs.el\0409:(put 'eshell/cd 'eshell-no-numeric-conversions t)
./em-dirs.el\0472:(put 'eshell/pushd 'eshell-no-numeric-conversions t)
./em-dirs.el\0502:(put 'eshell/popd 'eshell-no-numeric-conversions t)
./esh-ext.el\0261:(put 'eshell/addpath 'eshell-no-numeric-conversions t
./esh-util.el\091:  (put \\='find-file \\='eshell-no-numeric-
./em-script.el\0127:(put 'eshell/source 'eshell-no-numeric-conversions
./em-script.el\0140:(put 'eshell/. 'eshell-no-numeric-conversions t)
./em-unix.el\0167:(put 'eshell/man 'eshell-no-numeric-conversions t)
./em-unix.el\0309:(put 'eshell/rm 'eshell-no-numeric-conversions t)
./em-unix.el\0326:(put 'eshell/mkdir 'eshell-no-numeric-conversions t)
./em-unix.el\0342:(put 'eshell/rmdir 'eshell-no-numeric-conversions t)
./em-unix.el\0526:(put 'eshell/mv 'eshell-no-numeric-conversions t)
./em-unix.el\0563:(put 'eshell/cp 'eshell-no-numeric-conversions t)
./em-unix.el\0595:(put 'eshell/ln 'eshell-no-numeric-conversions t)
./em-unix.el\0647:(put 'eshell/cat 'eshell-no-numeric-conversions t)
./em-unix.el\0664:(put 'eshell/make 'eshell-no-numeric-conversions t)
./em-unix.el\01031:(put 'eshell/diff 'eshell-no-numeric-conversions t)
./em-unix.el\01050:(put 'eshell/locate 'eshell-no-numeric-conversions 
./em-unix.el\01059:(put 'eshell/occur 'eshell-no-numeric-conversions t)
./esh-cmd.el\01185:(put 'eshell/which 'eshell-no-numeric-conversions t)
./em-ls.el\0336:(put 'eshell/ls 'eshell-no-numeric-conversions t)
./em-tramp.el\097:(put 'eshell/su 'eshell-no-numeric-conversions t)
./em-tramp.el\0139:(put 'eshell/sudo 'eshell-no-numeric-conversions t)

--
Jonathan Kyle Mitchell

[-- Attachment #2: 0001-Check-for-filenames-special-to-the-shell-before-runn.patch --]
[-- Type: text/x-patch, Size: 1871 bytes --]

From 1aad963a71fca1abfff6d4522ddf83a07391712a Mon Sep 17 00:00:00 2001
From: Jonathan Kyle Mitchell <kyle@jonathanmitchell.org>
Date: Fri, 6 Jul 2018 01:40:32 -0500
Subject: [PATCH] Check for filenames special to the shell before running a
 command

Fix bug#30724 by checking if "*" and "~" are arguments to the current command
and quoting them relative to `default-directory' if so.  This leverages the
the fact that the existing eshell builtin commands that accept file arguments
are tagged with the eshell-no-numeric-conversions property.  The existing
details of eshell command execution are left unchanged.

* lisp/eshell/esh-cmd.el (eshell-lisp-command)
---
 lisp/eshell/esh-cmd.el | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/lisp/eshell/esh-cmd.el b/lisp/eshell/esh-cmd.el
index 61c0ebc71d..a22309a85e 100644
--- a/lisp/eshell/esh-cmd.el
+++ b/lisp/eshell/esh-cmd.el
@@ -1310,9 +1310,20 @@ eshell-lisp-command
 		  (setq eshell-last-arguments args
 			eshell-last-command-name
 			(concat "#<function " (symbol-name object) ">"))
-		  ;; if any of the arguments are flagged as numbers
-		  ;; waiting for conversion, convert them now
-		  (unless (get object 'eshell-no-numeric-conversions)
+		  (if (get object 'eshell-no-numeric-conversions)
+		      ;; check for filenames special to the shell and
+		      ;; quote them relative to `default-directory'
+		      (while args
+			(let ((arg (car args)))
+			  (if (and (stringp arg)
+				   (member arg '("~" "*")))
+			      (setcar args (concat (file-name-quote
+						    (expand-file-name
+						     default-directory))
+						   arg))))
+			(setq args (cdr args)))
+		    ;; if any of the arguments are flagged as numbers
+		    ;; waiting for conversion, convert them now
 		    (while args
 		      (let ((arg (car args)))
 			(if (and (stringp arg)
-- 
2.17.1


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

* bug#30724: eshell: escaped tilde is not treated as such
  2018-07-06  7:24 ` Jonathan Kyle Mitchell
@ 2018-07-07 19:17   ` Noam Postavsky
  2018-07-15 17:18     ` Jonathan Kyle Mitchell
  0 siblings, 1 reply; 8+ messages in thread
From: Noam Postavsky @ 2018-07-07 19:17 UTC (permalink / raw)
  To: Jonathan Kyle Mitchell; +Cc: 30724, Yegor Timoshenko

Jonathan Kyle Mitchell <kyle@jonathanmitchell.org> writes:

> tags 30724 + patch
> quit

You have to send this control@debbugs.gnu.org to have effect, generally
you should use Bcc so that further replies don't go there (which is why
you don't see it when other people do this).

> I found a way to get eshell to escape special chars.  It seems most of
> the eshell builtin commands that take file arguments are already tagged
> with the eshell-no-numeric-conversions property.  By adding the true
> part of the if condition in eshell-lisp-command, it is possible to
> quote the arguments of special filenames.  The attached patch checks
> for "~" and "*".

Perhaps we should distinguish between file and non-numeric arguments
though?  E.g., I think the file-name-quote might not make sense for the
commands below:

> ./esh-proc.el\0202:(put 'eshell/kill 'eshell-no-numeric-conversions t)

> ./em-unix.el\0167:(put 'eshell/man 'eshell-no-numeric-conversions t)

> ./em-unix.el\0664:(put 'eshell/make 'eshell-no-numeric-conversions t)

> ./em-unix.el\01050:(put 'eshell/locate 'eshell-no-numeric-conversions 
> ./em-unix.el\01059:(put 'eshell/occur 'eshell-no-numeric-conversions t)
> ./esh-cmd.el\01185:(put 'eshell/which 'eshell-no-numeric-conversions t)

> ./em-tramp.el\097:(put 'eshell/su 'eshell-no-numeric-conversions t)
> ./em-tramp.el\0139:(put 'eshell/sudo 'eshell-no-numeric-conversions t)


> Fix bug#30724 by checking if "*" and "~" are arguments to the current command
> and quoting them relative to `default-directory' if so.  This leverages the
> the fact that the existing eshell builtin commands that accept file arguments
> are tagged with the eshell-no-numeric-conversions property.  The existing
> details of eshell command execution are left unchanged.
>
> * lisp/eshell/esh-cmd.el (eshell-lisp-command)

The ChangeLog item should at the beginning, as in,

* lisp/eshell/esh-cmd.el (eshell-lisp-command): Fix bug#30724 by
checking if "*" and "~" are arguments to the current command ...





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

* bug#30724: eshell: escaped tilde is not treated as such
  2018-07-07 19:17   ` Noam Postavsky
@ 2018-07-15 17:18     ` Jonathan Kyle Mitchell
  2018-07-17  0:14       ` Noam Postavsky
  0 siblings, 1 reply; 8+ messages in thread
From: Jonathan Kyle Mitchell @ 2018-07-15 17:18 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: 30724, Yegor Timoshenko

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

On Sat, 2018-07-07 at 15:17 -0400, Noam Postavsky wrote:
> Perhaps we should distinguish between file and non-numeric arguments
> though?  E.g., I think the file-name-quote might not make sense for
> the
> commands below:
> 
> > ./esh-proc.el\0202:(put 'eshell/kill 'eshell-no-numeric-conversions 
> > t)
> > ./em-unix.el\0167:(put 'eshell/man 'eshell-no-numeric-conversions
> > t)
> > ./em-unix.el\0664:(put 'eshell/make 'eshell-no-numeric-conversions
> > t)
> > ./em-unix.el\01050:(put 'eshell/locate 'eshell-no-numeric-
> > conversions 
> > ./em-unix.el\01059:(put 'eshell/occur 'eshell-no-numeric-
> > conversions t)
> > ./esh-cmd.el\01185:(put 'eshell/which 'eshell-no-numeric-
> > conversions t)
> > ./em-tramp.el\097:(put 'eshell/su 'eshell-no-numeric-conversions t)
> > ./em-tramp.el\0139:(put 'eshell/sudo 'eshell-no-numeric-conversions 
> > t)

After tracing through the execution of eshell some more, I noticed that
eshell sets an escaped text property on the arguments when the user
escaped them with `\' or quote characters but there is nothing in
eshell that does anything with it apparently.

I've attached a new patch that checks for the escaped text property on
any single character argumentand.  I've also added a new symbol
property to the following functions that take filename arguments and
use that to check instead of the eshell-no-numeric-conversions
property.

(eshell/cd)
(eshell/pushd)
(eshell/popd)
(eshell/ls)
(eshell/rm)
(eshell/mkdir)
(eshell/rmdir)
(eshell/mv)
(eshell/cp)
(eshell/ln)
(eshell/cat)
(eshell/du)
(eshell/diff)
(eshell/addpath)

> 
> The ChangeLog item should at the beginning, as in,
> 
> * lisp/eshell/esh-cmd.el (eshell-lisp-command): Fix bug#30724 by
> checking if "*" and "~" are arguments to the current command ...

Thanks for the comments and suggestions, I fixed the commit message in the new patch.

--
Jonathan Kyle Mitchell

[-- Attachment #2: 0001-Check-for-special-filenames-in-eshell-Bug-30724.patch --]
[-- Type: text/x-patch, Size: 6833 bytes --]

From 98c15a273b0fff39447427a47442856ce1161bad Mon Sep 17 00:00:00 2001
From: Jonathan Kyle Mitchell <kyle@jonathanmitchell.org>
Date: Sun, 15 Jul 2018 10:57:23 -0500
Subject: [PATCH] Check for special filenames in eshell (Bug#30724)

	* lisp/eshell/em-dirs.el (eshell/cd): Add
	eshell-escape-special-filenames to symbol plist.
	(eshell/pushd): Likewise.
	(eshell/popd): Likewise.

	* lisp/eshell/em-ls.el (eshell/ls): Add
	eshell-escape-special-filenames to symbol plist.

	* lisp/eshell/em-unix.el (eshell/rm): Add
	eshell-escape-special-filenames to symbol plist.
	(eshell/mkdir): Likewise.
	(eshell/rmdir): Likewise.
	(eshell/mv): Likewise.
	(eshell/cp): Likewise.
	(eshell/ln): Likewise.
	(eshell/cat): Likewise.
	(eshell/du): Likewise.
	(eshell/diff): Likewise.

	* lisp/eshell/esh-cmd.el (eshell-lisp-command):	Check for
	single character escaped string arguments (Bug#30724).

	* lisp/eshell/esh-ext.el (eshell/addpath): Add
	eshell-escape-special-filenames to symbol plist.
---
 lisp/eshell/em-dirs.el |  3 +++
 lisp/eshell/em-ls.el   |  1 +
 lisp/eshell/em-unix.el | 10 ++++++++++
 lisp/eshell/esh-cmd.el | 32 +++++++++++++++++++++-----------
 lisp/eshell/esh-ext.el |  1 +
 5 files changed, 36 insertions(+), 11 deletions(-)

diff --git a/lisp/eshell/em-dirs.el b/lisp/eshell/em-dirs.el
index ec380e6701..25dd6e0887 100644
--- a/lisp/eshell/em-dirs.el
+++ b/lisp/eshell/em-dirs.el
@@ -407,6 +407,7 @@ eshell/cd
 	nil))))
 
 (put 'eshell/cd 'eshell-no-numeric-conversions t)
+(put 'eshell/cd 'eshell-escape-special-filenames t)
 
 (defun eshell-add-to-dir-ring (path)
   "Add PATH to the last-dir-ring, if applicable."
@@ -470,6 +471,7 @@ eshell/pushd
   nil)
 
 (put 'eshell/pushd 'eshell-no-numeric-conversions t)
+(put 'eshell/pushd 'eshell-escape-special-filenames t)
 
 ;;; popd [+n]
 (defun eshell/popd (&rest args)
@@ -500,6 +502,7 @@ eshell/popd
   nil)
 
 (put 'eshell/popd 'eshell-no-numeric-conversions t)
+(put 'eshell/popd 'eshell-escape-special-filenames t)
 
 (defun eshell/dirs (&optional if-verbose)
   "Implementation of dirs in Lisp."
diff --git a/lisp/eshell/em-ls.el b/lisp/eshell/em-ls.el
index 900b28905b..3995bd9c61 100644
--- a/lisp/eshell/em-ls.el
+++ b/lisp/eshell/em-ls.el
@@ -334,6 +334,7 @@ eshell/ls
     (apply 'eshell-do-ls args)))
 
 (put 'eshell/ls 'eshell-no-numeric-conversions t)
+(put 'eshell/ls 'eshell-escape-special-filenames t)
 
 (declare-function eshell-glob-regexp "em-glob" (pattern))
 
diff --git a/lisp/eshell/em-unix.el b/lisp/eshell/em-unix.el
index a18fb85507..e821b315b8 100644
--- a/lisp/eshell/em-unix.el
+++ b/lisp/eshell/em-unix.el
@@ -307,6 +307,7 @@ eshell/rm
    nil))
 
 (put 'eshell/rm 'eshell-no-numeric-conversions t)
+(put 'eshell/rm 'eshell-escape-special-filenames t)
 
 (defun eshell/mkdir (&rest args)
   "Implementation of mkdir in Lisp."
@@ -324,6 +325,7 @@ eshell/mkdir
    nil))
 
 (put 'eshell/mkdir 'eshell-no-numeric-conversions t)
+(put 'eshell/mkdir 'eshell-escape-special-filenames t)
 
 (defun eshell/rmdir (&rest args)
   "Implementation of rmdir in Lisp."
@@ -340,6 +342,7 @@ eshell/rmdir
    nil))
 
 (put 'eshell/rmdir 'eshell-no-numeric-conversions t)
+(put 'eshell/rmdir 'eshell-escape-special-filenames t)
 
 (defvar no-dereference)
 
@@ -524,6 +527,7 @@ eshell/mv
 			     eshell-mv-overwrite-files))))
 
 (put 'eshell/mv 'eshell-no-numeric-conversions t)
+(put 'eshell/mv 'eshell-escape-special-filenames t)
 
 (defun eshell/cp (&rest args)
   "Implementation of cp in Lisp."
@@ -561,6 +565,7 @@ eshell/cp
 			   eshell-cp-overwrite-files preserve)))
 
 (put 'eshell/cp 'eshell-no-numeric-conversions t)
+(put 'eshell/cp 'eshell-escape-special-filenames t)
 
 (defun eshell/ln (&rest args)
   "Implementation of ln in Lisp."
@@ -593,6 +598,7 @@ eshell/ln
 			     eshell-ln-overwrite-files))))
 
 (put 'eshell/ln 'eshell-no-numeric-conversions t)
+(put 'eshell/ln 'eshell-escape-special-filenames t)
 
 (defun eshell/cat (&rest args)
   "Implementation of cat in Lisp.
@@ -645,6 +651,7 @@ eshell/cat
      (setq eshell-ensure-newline-p nil))))
 
 (put 'eshell/cat 'eshell-no-numeric-conversions t)
+(put 'eshell/cat 'eshell-escape-special-filenames t)
 
 ;; special front-end functions for compilation-mode buffers
 
@@ -927,6 +934,8 @@ eshell/du
 	     (eshell-print (concat (eshell-du-size-string size)
 				   "total\n"))))))))
 
+(put 'eshell/du 'eshell-escape-special-filenames t)
+
 (defvar eshell-time-start nil)
 
 (defun eshell-show-elapsed-time ()
@@ -1029,6 +1038,7 @@ eshell/diff
   nil)
 
 (put 'eshell/diff 'eshell-no-numeric-conversions t)
+(put 'eshell/diff 'eshell-escape-special-filenames t)
 
 (defvar locate-history-list)
 
diff --git a/lisp/eshell/esh-cmd.el b/lisp/eshell/esh-cmd.el
index 61c0ebc71d..8d52b9fc9c 100644
--- a/lisp/eshell/esh-cmd.el
+++ b/lisp/eshell/esh-cmd.el
@@ -1310,17 +1310,27 @@ eshell-lisp-command
 		  (setq eshell-last-arguments args
 			eshell-last-command-name
 			(concat "#<function " (symbol-name object) ">"))
-		  ;; if any of the arguments are flagged as numbers
-		  ;; waiting for conversion, convert them now
-		  (unless (get object 'eshell-no-numeric-conversions)
-		    (while args
-		      (let ((arg (car args)))
-			(if (and (stringp arg)
-				 (> (length arg) 0)
-				 (not (text-property-not-all
-				       0 (length arg) 'number t arg)))
-			    (setcar args (string-to-number arg))))
-		      (setq args (cdr args))))
+		  (let ((numeric (not (get object
+                                           'eshell-no-numeric-conversions)))
+			(escaped (get object 'eshell-escape-special-filenames)))
+		    (when (or numeric escaped)
+		      (while args
+			(let ((arg (car args)))
+			  (cond ((and numeric (stringp arg) (> (length arg) 0)
+				      (text-property-any 0 (length arg)
+							 'number t arg))
+				 ;; if any of the arguments are flagged as
+				 ;; numbers waiting for conversion, convert
+				 ;; them now
+				 (setcar args (string-to-number arg)))
+				((and escaped (stringp arg) (= (length arg) 1)
+				      (text-property-any 0 (length arg)
+							 'escaped t arg))
+				 ;; if any of the arguments are single
+				 ;; character escaped filenames,
+				 ;; prepend "./"
+				 (setcar args (concat "./" arg)))))
+			(setq args (cdr args)))))
 		  (eshell-apply object eshell-last-arguments))
 	      (setq eshell-last-arguments args
 		    eshell-last-command-name "#<Lisp object>")
diff --git a/lisp/eshell/esh-ext.el b/lisp/eshell/esh-ext.el
index ba5182deb4..8cb0f685a4 100644
--- a/lisp/eshell/esh-ext.el
+++ b/lisp/eshell/esh-ext.el
@@ -259,6 +259,7 @@ eshell/addpath
        (eshell-printn dir)))))
 
 (put 'eshell/addpath 'eshell-no-numeric-conversions t)
+(put 'eshell/addpath 'eshell-escape-special-filenames t)
 
 (defun eshell-script-interpreter (file)
   "Extract the script to run from FILE, if it has #!<interp> in it.
-- 
2.17.1


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

* bug#30724: eshell: escaped tilde is not treated as such
  2018-07-15 17:18     ` Jonathan Kyle Mitchell
@ 2018-07-17  0:14       ` Noam Postavsky
  2018-07-18  3:54         ` Jonathan Kyle Mitchell
  0 siblings, 1 reply; 8+ messages in thread
From: Noam Postavsky @ 2018-07-17  0:14 UTC (permalink / raw)
  To: Jonathan Kyle Mitchell; +Cc: 30724, Yegor Timoshenko

Jonathan Kyle Mitchell <kyle@jonathanmitchell.org> writes:

> After tracing through the execution of eshell some more, I noticed that
> eshell sets an escaped text property on the arguments when the user
> escaped them with `\' or quote characters but there is nothing in
> eshell that does anything with it apparently.

Yeah, this sort of half-finished thing seems to pop up in eshell quite a
bit.

> Subject: [PATCH] Check for special filenames in eshell (Bug#30724)
>
> 	* lisp/eshell/em-dirs.el (eshell/cd): Add
> 	eshell-escape-special-filenames to symbol plist.
> 	(eshell/pushd): Likewise.
> 	(eshell/popd): Likewise.
>
> 	* lisp/eshell/em-ls.el (eshell/ls): Add
> 	eshell-escape-special-filenames to symbol plist.
>
> 	* lisp/eshell/em-unix.el (eshell/rm): Add
> 	eshell-escape-special-filenames to symbol plist.
> 	(eshell/mkdir): Likewise.
> 	(eshell/rmdir): Likewise.
> 	(eshell/mv): Likewise.
> 	(eshell/cp): Likewise.
> 	(eshell/ln): Likewise.
> 	(eshell/cat): Likewise.
> 	(eshell/du): Likewise.
> 	(eshell/diff): Likewise.
>
> 	* lisp/eshell/esh-cmd.el (eshell-lisp-command):	Check for
> 	single character escaped string arguments (Bug#30724).
>
> 	* lisp/eshell/esh-ext.el (eshell/addpath): Add
> 	eshell-escape-special-filenames to symbol plist.

The "Add eshell-escape-special-filenames to symbol plist" should be
coalesced, as in:

* lisp/eshell/em-dirs.el (eshell/cd, eshell/pushd, eshell/popd):
* lisp/eshell/em-ls.el (eshell/ls):
* lisp/eshell/em-unix.el (eshell/rm, eshell/mkdir, eshell/rmdir)
(eshell/mv, eshell/cp, eshell/ln, eshell/cat, eshell/du, eshell/diff):
* lisp/eshell/esh-ext.el (eshell/addpath): Add
eshell-escape-special-filenames to symbol plist.

* lisp/eshell/esh-cmd.el (eshell-lisp-command):	Check for
single character escaped string arguments (Bug#30724).
  

> +		  (let ((numeric (not (get object
> +                                           'eshell-no-numeric-conversions)))
> +			(escaped (get object 'eshell-escape-special-filenames)))

I'm not sure 'escaped' or 'eshell-escape-special-filenames' are really
the right names here, as they're not actually indicating any escaping.
Maybe the property should be something like 'eshell-filename-arguments'?

> +		    (when (or numeric escaped)
> +		      (while args
> +			(let ((arg (car args)))
> +			  (cond ((and numeric (stringp arg) (> (length arg) 0)
> +				      (text-property-any 0 (length arg)
> +							 'number t arg))
> +				 ;; if any of the arguments are flagged as
> +				 ;; numbers waiting for conversion, convert
> +				 ;; them now

Comments should be capitalized and end with a period (I know the
original didn't, but since we're updating the code, now is a good time
to fix it).

> +				 (setcar args (string-to-number arg)))
> +				((and escaped (stringp arg) (= (length arg) 1)
> +				      (text-property-any 0 (length arg)
> +							 'escaped t arg))
> +				 ;; if any of the arguments are single
> +				 ;; character escaped filenames,
> +				 ;; prepend "./"
> +				 (setcar args (concat "./" arg)))))

It looks like this kind of escaping isn't actually needed for "*" which
your previous patch mentioned.  Perhaps just check for "~" explicitly,
rather than any single character filenames.  Or are there other examples
besides "~" which need "./" to prevent interpretation?






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

* bug#30724: eshell: escaped tilde is not treated as such
  2018-07-17  0:14       ` Noam Postavsky
@ 2018-07-18  3:54         ` Jonathan Kyle Mitchell
  2018-07-22  1:34           ` Noam Postavsky
  0 siblings, 1 reply; 8+ messages in thread
From: Jonathan Kyle Mitchell @ 2018-07-18  3:54 UTC (permalink / raw)
  To: Noam Postavsky; +Cc: 30724, Yegor Timoshenko

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

On Mon, 2018-07-16 at 20:14 -0400, Noam Postavsky wrote:
> The "Add eshell-escape-special-filenames to symbol plist" should be
> coalesced, as in:

Done in the new attached patch.

> I'm not sure 'escaped' or 'eshell-escape-special-filenames' are
> really
> the right names here, as they're not actually indicating any
> escaping.
> Maybe the property should be something like 'eshell-filename-
> arguments'?

I changed the property name to eshell-filename-arguments.

> Comments should be capitalized and end with a period (I know the
> original didn't, but since we're updating the code, now is a good
> time
> to fix it).

Fixed both the original and new comment.

> It looks like this kind of escaping isn't actually needed for "*"
> which
> your previous patch mentioned.  Perhaps just check for "~"
> explicitly,
> rather than any single character filenames.  Or are there other
> examples
> besides "~" which need "./" to prevent interpretation?

I can't think of any other cases that would need to be check either, so
I changed to to just string-match "~" only.

--
Jonathan Kyle Mitchell

[-- Attachment #2: 0001-Check-for-special-filenames-in-eshell-Bug-30724.patch --]
[-- Type: text/x-patch, Size: 8080 bytes --]

From 2b86bff2c3d62936fb53a5cb3540f86fcd920a84 Mon Sep 17 00:00:00 2001
From: Jonathan Kyle Mitchell <kyle@jonathanmitchell.org>
Date: Mon, 16 Jul 2018 21:46:20 -0500
Subject: [PATCH] Check for special filenames in eshell (Bug#30724)

	* lisp/eshell/esh-cmd.el (eshell-lisp-command): Check for "~"
	in lisp commands with the eshell-filename-arguments property
	(Bug#30724).

	* lisp/eshell/em-dirs.el (eshell/cd, eshell/pushd, eshell/popd):
	* lisp/eshell/em-ls.el (eshell/ls):
	* lisp/eshell/em-unix.el (eshell/rm, eshell/mkdir, eshell/rmdir):
	(eshell/mv, eshell/cp, eshell/ln, eshell/cat, eshell/du, eshell/diff):
	* lisp/eshell/esh-ext.el (eshell/addpath): Add
	eshell-filename-arguments to symbol plist.
---
 lisp/eshell/em-dirs.el |  3 +++
 lisp/eshell/em-ls.el   |  1 +
 lisp/eshell/em-unix.el | 10 +++++++
 lisp/eshell/esh-cmd.el | 59 ++++++++++++++++++++++++------------------
 lisp/eshell/esh-ext.el |  1 +
 5 files changed, 49 insertions(+), 25 deletions(-)

diff --git a/lisp/eshell/em-dirs.el b/lisp/eshell/em-dirs.el
index ec380e6701..5180a0700d 100644
--- a/lisp/eshell/em-dirs.el
+++ b/lisp/eshell/em-dirs.el
@@ -407,6 +407,7 @@ eshell/cd
 	nil))))
 
 (put 'eshell/cd 'eshell-no-numeric-conversions t)
+(put 'eshell/cd 'eshell-filename-arguments t)
 
 (defun eshell-add-to-dir-ring (path)
   "Add PATH to the last-dir-ring, if applicable."
@@ -470,6 +471,7 @@ eshell/pushd
   nil)
 
 (put 'eshell/pushd 'eshell-no-numeric-conversions t)
+(put 'eshell/pushd 'eshell-filename-arguments t)
 
 ;;; popd [+n]
 (defun eshell/popd (&rest args)
@@ -500,6 +502,7 @@ eshell/popd
   nil)
 
 (put 'eshell/popd 'eshell-no-numeric-conversions t)
+(put 'eshell/pop 'eshell-filename-arguments t)
 
 (defun eshell/dirs (&optional if-verbose)
   "Implementation of dirs in Lisp."
diff --git a/lisp/eshell/em-ls.el b/lisp/eshell/em-ls.el
index 900b28905b..2b568a991a 100644
--- a/lisp/eshell/em-ls.el
+++ b/lisp/eshell/em-ls.el
@@ -334,6 +334,7 @@ eshell/ls
     (apply 'eshell-do-ls args)))
 
 (put 'eshell/ls 'eshell-no-numeric-conversions t)
+(put 'eshell/ls 'eshell-filename-arguments t)
 
 (declare-function eshell-glob-regexp "em-glob" (pattern))
 
diff --git a/lisp/eshell/em-unix.el b/lisp/eshell/em-unix.el
index a18fb85507..c912c15ac7 100644
--- a/lisp/eshell/em-unix.el
+++ b/lisp/eshell/em-unix.el
@@ -307,6 +307,7 @@ eshell/rm
    nil))
 
 (put 'eshell/rm 'eshell-no-numeric-conversions t)
+(put 'eshell/rm 'eshell-filename-arguments t)
 
 (defun eshell/mkdir (&rest args)
   "Implementation of mkdir in Lisp."
@@ -324,6 +325,7 @@ eshell/mkdir
    nil))
 
 (put 'eshell/mkdir 'eshell-no-numeric-conversions t)
+(put 'eshell/mkdir 'eshell-filename-arguments t)
 
 (defun eshell/rmdir (&rest args)
   "Implementation of rmdir in Lisp."
@@ -340,6 +342,7 @@ eshell/rmdir
    nil))
 
 (put 'eshell/rmdir 'eshell-no-numeric-conversions t)
+(put 'eshell/rmdir 'eshell-filename-arguments t)
 
 (defvar no-dereference)
 
@@ -524,6 +527,7 @@ eshell/mv
 			     eshell-mv-overwrite-files))))
 
 (put 'eshell/mv 'eshell-no-numeric-conversions t)
+(put 'eshell/mv 'eshell-filename-arguments t)
 
 (defun eshell/cp (&rest args)
   "Implementation of cp in Lisp."
@@ -561,6 +565,7 @@ eshell/cp
 			   eshell-cp-overwrite-files preserve)))
 
 (put 'eshell/cp 'eshell-no-numeric-conversions t)
+(put 'eshell/cp 'eshell-filename-arguments t)
 
 (defun eshell/ln (&rest args)
   "Implementation of ln in Lisp."
@@ -593,6 +598,7 @@ eshell/ln
 			     eshell-ln-overwrite-files))))
 
 (put 'eshell/ln 'eshell-no-numeric-conversions t)
+(put 'eshell/ln 'eshell-filename-arguments t)
 
 (defun eshell/cat (&rest args)
   "Implementation of cat in Lisp.
@@ -645,6 +651,7 @@ eshell/cat
      (setq eshell-ensure-newline-p nil))))
 
 (put 'eshell/cat 'eshell-no-numeric-conversions t)
+(put 'eshell/cat 'eshell-filename-arguments t)
 
 ;; special front-end functions for compilation-mode buffers
 
@@ -927,6 +934,8 @@ eshell/du
 	     (eshell-print (concat (eshell-du-size-string size)
 				   "total\n"))))))))
 
+(put 'eshell/du 'eshell-filename-arguments t)
+
 (defvar eshell-time-start nil)
 
 (defun eshell-show-elapsed-time ()
@@ -1029,6 +1038,7 @@ eshell/diff
   nil)
 
 (put 'eshell/diff 'eshell-no-numeric-conversions t)
+(put 'eshell/diff 'eshell-filename-arguments t)
 
 (defvar locate-history-list)
 
diff --git a/lisp/eshell/esh-cmd.el b/lisp/eshell/esh-cmd.el
index 61c0ebc71d..358f0f957e 100644
--- a/lisp/eshell/esh-cmd.el
+++ b/lisp/eshell/esh-cmd.el
@@ -1304,32 +1304,41 @@ eshell-lisp-command
   "Insert Lisp OBJECT, using ARGS if a function."
   (catch 'eshell-external               ; deferred to an external command
     (let* ((eshell-ensure-newline-p (eshell-interactive-output-p))
-	   (result
-	    (if (functionp object)
-		(progn
-		  (setq eshell-last-arguments args
-			eshell-last-command-name
-			(concat "#<function " (symbol-name object) ">"))
-		  ;; if any of the arguments are flagged as numbers
-		  ;; waiting for conversion, convert them now
-		  (unless (get object 'eshell-no-numeric-conversions)
-		    (while args
-		      (let ((arg (car args)))
-			(if (and (stringp arg)
-				 (> (length arg) 0)
-				 (not (text-property-not-all
-				       0 (length arg) 'number t arg)))
-			    (setcar args (string-to-number arg))))
-		      (setq args (cdr args))))
-		  (eshell-apply object eshell-last-arguments))
-	      (setq eshell-last-arguments args
-		    eshell-last-command-name "#<Lisp object>")
-	      (eshell-eval object))))
+           (result
+            (if (functionp object)
+                (progn
+                  (setq eshell-last-arguments args
+                        eshell-last-command-name
+                        (concat "#<function " (symbol-name object) ">"))
+                  (let ((numeric (not (get object
+                                           'eshell-no-numeric-conversions)))
+                        (fname-args (get object 'eshell-filename-arguments)))
+                    (when (or numeric fname-args)
+                      (while args
+                        (let ((arg (car args)))
+                          (cond ((and numeric (stringp arg) (> (length arg) 0)
+                                      (text-property-any 0 (length arg)
+                                                         'number t arg))
+                                 ;; If any of the arguments are
+                                 ;; flagged as numbers waiting for
+                                 ;; conversion, convert them now.
+                                 (setcar args (string-to-number arg)))
+                                ((and fname-args (stringp arg)
+                                      (string-equal arg "~"))
+                                 ;; If any of the arguments match "~",
+                                 ;; prepend "./" to treat it as a
+                                 ;; regular file name.
+                                 (setcar args (concat "./" arg)))))
+                        (setq args (cdr args)))))
+                  (eshell-apply object eshell-last-arguments))
+              (setq eshell-last-arguments args
+                    eshell-last-command-name "#<Lisp object>")
+              (eshell-eval object))))
       (if (and eshell-ensure-newline-p
-	       (save-excursion
-		 (goto-char eshell-last-output-end)
-		 (not (bolp))))
-	  (eshell-print "\n"))
+               (save-excursion
+                 (goto-char eshell-last-output-end)
+                 (not (bolp))))
+          (eshell-print "\n"))
       (eshell-close-handles 0 (list 'quote result)))))
 
 (defalias 'eshell-lisp-command* 'eshell-lisp-command)
diff --git a/lisp/eshell/esh-ext.el b/lisp/eshell/esh-ext.el
index ba5182deb4..244cc7ff1f 100644
--- a/lisp/eshell/esh-ext.el
+++ b/lisp/eshell/esh-ext.el
@@ -259,6 +259,7 @@ eshell/addpath
        (eshell-printn dir)))))
 
 (put 'eshell/addpath 'eshell-no-numeric-conversions t)
+(put 'eshell/addpath 'eshell-filename-arguments t)
 
 (defun eshell-script-interpreter (file)
   "Extract the script to run from FILE, if it has #!<interp> in it.
-- 
2.17.1


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

* bug#30724: eshell: escaped tilde is not treated as such
  2018-07-18  3:54         ` Jonathan Kyle Mitchell
@ 2018-07-22  1:34           ` Noam Postavsky
  0 siblings, 0 replies; 8+ messages in thread
From: Noam Postavsky @ 2018-07-22  1:34 UTC (permalink / raw)
  To: Jonathan Kyle Mitchell; +Cc: 30724, Yegor Timoshenko

tags 30724 fixed
close 30724 26.2
quit

Jonathan Kyle Mitchell <kyle@jonathanmitchell.org> writes:

> Done in the new attached patch.

Pushed to emacs-26.

[1: 5de444112c]: 2018-07-21 21:07:07 -0400
  Check for special filenames in eshell (Bug#30724)
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=5de444112cf19c078d4a74752a50e890233ef033>

> 	* lisp/eshell/em-unix.el (eshell/rm, eshell/mkdir, eshell/rmdir):

A minor nitpick about formatting here, ChangeLog entries for one file
continued over multiple lines shouldn't have a colon at the end of line
(I fixed this before pushing).

> 	(eshell/mv, eshell/cp, eshell/ln, eshell/cat, eshell/du, eshell/diff):





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

end of thread, other threads:[~2018-07-22  1:34 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-03-06  4:30 bug#30724: eshell: escaped tilde is not treated as such Yegor Timoshenko
2018-03-09  1:15 ` Noam Postavsky
2018-07-06  7:24 ` Jonathan Kyle Mitchell
2018-07-07 19:17   ` Noam Postavsky
2018-07-15 17:18     ` Jonathan Kyle Mitchell
2018-07-17  0:14       ` Noam Postavsky
2018-07-18  3:54         ` Jonathan Kyle Mitchell
2018-07-22  1:34           ` Noam Postavsky

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

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

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