Proposal: new syntax for eshell-buffer-shorthand (related: bug#19391)

Proposal: new syntax for eshell-buffer-shorthand (related: bug#19391)

[Samer Masterson]

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

Hi list,

Eshell has a buffer shorthand, which you can enable by making 
"eshell-buffer-shorthand" non-nil. The shorthand looks like this:

~$ echo "words" > '*scratch*

It's currently broken in two ways:
 - Creating a symbol with a single quote doesn't work in eshell, 
because the single quote starts a string (which follows POSIX).
 - The feature works by adding a branch to eshell-get-target that 
checks if the target is a symbol, which means internal eshell functions 
that create symbols to be evaluated are broken, like the external 
subshell syntax (bug#19391).

I propose that we change eshell-buffer-shorthand in the following ways:
 - The syntax for the shorthand should be distinct from any other lisp 
objects, so that we don't have to put out fires when people try to do 
things that are valid but overlap with the shorthand (such as 
bug#19391). Something like #<name-of-buffer> would work.
 - The shorthand should be processed as a buffer (similar to how 
#<buffer name-of-buffer> is processed). That means the shorthand won't 
require extra code anywhere except for the reader.
 - The shorthand should be on by default, and shouldn't have an option 
to disable it. The reason for this is that the shorthand would be 
universally beneficial and backwards compatible, and making it 
controlled by an option is inviting bugs when we shouldn't be.

So the above example would look like:

~$ echo "words" > #<*scratch*>

I haven't looked into how eshell reads input, so I don't know if there 
are any significant technical issues with using this syntax. If there 
aren't any, I'll start working on this. Let me know if you have any 
questions.

-samer

[-- Attachment #2: Type: text/html, Size: 1939 bytes --]

[PATCH] Proposal: new syntax for eshell-buffer-shorthand (related: bug#19391)

[Samer Masterson]

Thanks for looking at this. Patch below

-s

From b00818b79ce7ee0c88a9689f69ad9e3716c3c4ab Mon Sep 17 00:00:00 2001
From: Samer Masterson <samer@samertm.com>
Date: Fri, 15 May 2015 19:42:00 -0700
Subject: [PATCH] eshell: Introduce new buffer syntax

The new buffer syntax '#<buffer-name>' is equivalent to '#<buffer
buffer-name>'.  Remove `eshell-buffer-shorthand', as it is no longer
needed (Bug#19319).

* lisp/eshell/esh-io.el (eshell-buffer-shorthand): Remove.
(eshell-get-target): Remove shorthand-specific code.
* lisp/eshell/esh-arg.el (eshell-parse-special-reference): Parse
'#<buffer-name>'.
---
 etc/NEWS               |  6 ++++++
 lisp/eshell/esh-arg.el | 41 +++++++++++++++++++++++++----------------
 lisp/eshell/esh-io.el  | 48 ++++++++++++++++++++----------------------------
 3 files changed, 51 insertions(+), 44 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 48c3a2a5..9c9f13f 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -659,6 +659,12 @@ command line's password prompt.
 *** The new built-in command `clear' can scroll window contents out of sight.
 If provided with an optional non-nil argument, the scrollback contents will be cleared.
 
+*** New buffer syntax '#<buffer-name>', which is equivalent to
+'#<buffer buffer-name>'.  This shorthand makes interacting with
+buffers from eshell more convenient.  Custom variable
+`eshell-buffer-shorthand', which has been broken for a while, has been
+removed.
+
 ** Browse-url
 
 *** Support for the Conkeror web browser.
diff --git a/lisp/eshell/esh-arg.el b/lisp/eshell/esh-arg.el
index a5f697f..49ba727 100644
--- a/lisp/eshell/esh-arg.el
+++ b/lisp/eshell/esh-arg.el
@@ -357,22 +357,31 @@ after are both returned."
 	  (goto-char (1+ end)))))))
 
 (defun eshell-parse-special-reference ()
-  "Parse a special syntax reference, of the form '#<type arg>'."
-  (if (and (not eshell-current-argument)
-	   (not eshell-current-quoted)
-	   (looking-at "#<\\(buffer\\|process\\)\\s-"))
-      (let ((here (point)))
-	(goto-char (match-end 0))
-	(let* ((buffer-p (string= (match-string 1) "buffer"))
-	       (end (eshell-find-delimiter ?\< ?\>)))
-	  (if (not end)
-	      (throw 'eshell-incomplete ?\<)
-	    (if (eshell-arg-delimiter (1+ end))
-		(prog1
-		    (list (if buffer-p 'get-buffer-create 'get-process)
-			  (buffer-substring-no-properties (point) end))
-		  (goto-char (1+ end)))
-	      (ignore (goto-char here))))))))
+  "Parse a special syntax reference, of the form '#<args>'.
+
+args           := `type' `whitespace' `arbitrary-args' | `arbitrary-args'
+type           := \"buffer\" or \"process\"
+arbitrary-args := any string of characters.
+
+If the form has no 'type', the syntax is parsed as if 'type' were
+\"buffer\"."
+  (when (and (not eshell-current-argument)
+             (not eshell-current-quoted)
+             (looking-at "#<\\(\\(buffer\\|process\\)\\s-\\)?"))
+    (let ((here (point)))
+      (goto-char (match-end 0)) ;; Go to the end of the match.
+      (let ((buffer-p (if (match-string 1)
+                          (string= (match-string 2) "buffer")
+                        t)) ;; buffer-p is non-nil by default.
+            (end (eshell-find-delimiter ?\< ?\>)))
+        (when (not end)
+          (throw 'eshell-incomplete ?\<))
+        (if (eshell-arg-delimiter (1+ end))
+            (prog1
+                (list (if buffer-p 'get-buffer-create 'get-process)
+                      (buffer-substring-no-properties (point) end))
+              (goto-char (1+ end)))
+          (ignore (goto-char here)))))))
 
 (defun eshell-parse-delimiter ()
   "Parse an argument delimiter, which is essentially a command operator."
diff --git a/lisp/eshell/esh-io.el b/lisp/eshell/esh-io.el
index 7dfc39f..dc731bc 100644
--- a/lisp/eshell/esh-io.el
+++ b/lisp/eshell/esh-io.el
@@ -31,6 +31,18 @@
 ;; consistent with most shells.  Therefore, only unique features are
 ;; mentioned here.
 ;;
+;;;_* Redirect to a Buffer or Process
+;;
+;; Buffers and processes can be named with '#<buffer buffer-name>' and
+;; '#<process process-name>', respectively. As a shorthand,
+;; '#<buffer-name>' without the explicit "buffer" arg is equivalent to
+;; '#<buffer buffer-name>'.
+;;
+;;   echo hello > #<buffer *scratch*> # Overwrite '*scratch*' with 'hello'.
+;;   echo hello > #<*scratch*>        # Same as the command above.
+;;
+;;   echo hello > #<process shell> # Pipe "hello" into the shell process.
+;;
 ;;;_* Insertion
 ;;
 ;; To insert at the location of point in a buffer, use '>>>':
@@ -98,19 +110,6 @@ other buffers) ."
   :type 'integer
   :group 'eshell-io)
 
-(defcustom eshell-buffer-shorthand nil
-  "If non-nil, a symbol name can be used for a buffer in redirection.
-If nil, redirecting to a buffer requires buffer name syntax.  If this
-variable is set, redirection directly to Lisp symbols will be
-impossible.
-
-Example:
-
-  echo hello > '*scratch*  ; works if `eshell-buffer-shorthand' is t
-  echo hello > #<buffer *scratch*>  ; always works"
-  :type 'boolean
-  :group 'eshell-io)
-
 (defcustom eshell-print-queue-size 5
   "The size of the print queue, for doing buffered printing.
 This is basically a speed enhancement, to avoid blocking the Lisp code
@@ -355,21 +354,14 @@ it defaults to `insert'."
 		   (goto-char (point-max))))
 	    (point-marker))))))
 
-   ((or (bufferp target)
-	(and (boundp 'eshell-buffer-shorthand)
-	     (symbol-value 'eshell-buffer-shorthand)
-	     (symbolp target)
-	     (not (memq target '(t nil)))))
-    (let ((buf (if (bufferp target)
-		   target
-		 (get-buffer-create
-		  (symbol-name target)))))
-      (with-current-buffer buf
-	(cond ((eq mode 'overwrite)
-	       (erase-buffer))
-	      ((eq mode 'append)
-	       (goto-char (point-max))))
-	(point-marker))))
+
+   ((bufferp target)
+    (with-current-buffer target
+      (cond ((eq mode 'overwrite)
+             (erase-buffer))
+            ((eq mode 'append)
+             (goto-char (point-max))))
+      (point-marker)))
 
    ((functionp target) nil)

Re: [PATCH] Proposal: new syntax for eshell-buffer-shorthand (related: bug#19391)

[Eli Zaretskii]

> From: Samer Masterson <samer@samertm.com>
> Date: Fri, 15 May 2015 19:59:12 -0700
> Cc: Eli Zaretskii <eliz@gnu.org>
> 
> Thanks for looking at this. Patch below

Thanks.

It looks like you are using all-SPC indentation, without any TABs,
perhaps due to the recent change in .dir-locals.el that caused that.
Could you please send the patch in the form equivalent to "diff -w" (I
hope Git allows that)?  It's hard to read a large patch where most of
the lines "changed" just due to whitespace.

Re: [PATCH] Proposal: new syntax for eshell-buffer-shorthand (related: bug#19391)

[Samer Masterson]

Eli Zaretskii <eliz@gnu.org> writes:

> Could you please send the patch in the form equivalent to "diff -w" (I
> hope Git allows that)?  It's hard to read a large patch where most of
> the lines "changed" just due to whitespace.

My bad, I've attached the diff with whitespace ignored. What's the
process for getting write access to the Emacs repository? If it's
easier, I can install the change after you review it.

Thanks for the review.

-s

Diff below:

diff --git a/etc/NEWS b/etc/NEWS
index 48c3a2a5..9c9f13f 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -659,6 +659,12 @@ command line's password prompt.
 *** The new built-in command `clear' can scroll window contents out of sight.
 If provided with an optional non-nil argument, the scrollback contents will be cleared.
 
+*** New buffer syntax '#<buffer-name>', which is equivalent to
+'#<buffer buffer-name>'.  This shorthand makes interacting with
+buffers from eshell more convenient.  Custom variable
+`eshell-buffer-shorthand', which has been broken for a while, has been
+removed.
+
 ** Browse-url
 
 *** Support for the Conkeror web browser.
diff --git a/lisp/eshell/esh-arg.el b/lisp/eshell/esh-arg.el
index a5f697f..49ba727 100644
--- a/lisp/eshell/esh-arg.el
+++ b/lisp/eshell/esh-arg.el
@@ -357,22 +357,31 @@ after are both returned."
 	  (goto-char (1+ end)))))))
 
 (defun eshell-parse-special-reference ()
-  "Parse a special syntax reference, of the form '#<type arg>'."
-  (if (and (not eshell-current-argument)
+  "Parse a special syntax reference, of the form '#<args>'.
+
+args           := `type' `whitespace' `arbitrary-args' | `arbitrary-args'
+type           := \"buffer\" or \"process\"
+arbitrary-args := any string of characters.
+
+If the form has no 'type', the syntax is parsed as if 'type' were
+\"buffer\"."
+  (when (and (not eshell-current-argument)
              (not eshell-current-quoted)
-	   (looking-at "#<\\(buffer\\|process\\)\\s-"))
+             (looking-at "#<\\(\\(buffer\\|process\\)\\s-\\)?"))
     (let ((here (point)))
-	(goto-char (match-end 0))
-	(let* ((buffer-p (string= (match-string 1) "buffer"))
+      (goto-char (match-end 0)) ;; Go to the end of the match.
+      (let ((buffer-p (if (match-string 1)
+                          (string= (match-string 2) "buffer")
+                        t)) ;; buffer-p is non-nil by default.
             (end (eshell-find-delimiter ?\< ?\>)))
-	  (if (not end)
-	      (throw 'eshell-incomplete ?\<)
+        (when (not end)
+          (throw 'eshell-incomplete ?\<))
         (if (eshell-arg-delimiter (1+ end))
             (prog1
                 (list (if buffer-p 'get-buffer-create 'get-process)
                       (buffer-substring-no-properties (point) end))
               (goto-char (1+ end)))
-	      (ignore (goto-char here))))))))
+          (ignore (goto-char here)))))))
 
 (defun eshell-parse-delimiter ()
   "Parse an argument delimiter, which is essentially a command operator."
diff --git a/lisp/eshell/esh-io.el b/lisp/eshell/esh-io.el
index 7dfc39f..dc731bc 100644
--- a/lisp/eshell/esh-io.el
+++ b/lisp/eshell/esh-io.el
@@ -31,6 +31,18 @@
 ;; consistent with most shells.  Therefore, only unique features are
 ;; mentioned here.
 ;;
+;;;_* Redirect to a Buffer or Process
+;;
+;; Buffers and processes can be named with '#<buffer buffer-name>' and
+;; '#<process process-name>', respectively. As a shorthand,
+;; '#<buffer-name>' without the explicit "buffer" arg is equivalent to
+;; '#<buffer buffer-name>'.
+;;
+;;   echo hello > #<buffer *scratch*> # Overwrite '*scratch*' with 'hello'.
+;;   echo hello > #<*scratch*>        # Same as the command above.
+;;
+;;   echo hello > #<process shell> # Pipe "hello" into the shell process.
+;;
 ;;;_* Insertion
 ;;
 ;; To insert at the location of point in a buffer, use '>>>':
@@ -98,19 +110,6 @@ other buffers) ."
   :type 'integer
   :group 'eshell-io)
 
-(defcustom eshell-buffer-shorthand nil
-  "If non-nil, a symbol name can be used for a buffer in redirection.
-If nil, redirecting to a buffer requires buffer name syntax.  If this
-variable is set, redirection directly to Lisp symbols will be
-impossible.
-
-Example:
-
-  echo hello > '*scratch*  ; works if `eshell-buffer-shorthand' is t
-  echo hello > #<buffer *scratch*>  ; always works"
-  :type 'boolean
-  :group 'eshell-io)
-
 (defcustom eshell-print-queue-size 5
   "The size of the print queue, for doing buffered printing.
 This is basically a speed enhancement, to avoid blocking the Lisp code
@@ -355,21 +354,14 @@ it defaults to `insert'."
 		   (goto-char (point-max))))
 	    (point-marker))))))
 
-   ((or (bufferp target)
-	(and (boundp 'eshell-buffer-shorthand)
-	     (symbol-value 'eshell-buffer-shorthand)
-	     (symbolp target)
-	     (not (memq target '(t nil)))))
-    (let ((buf (if (bufferp target)
-		   target
-		 (get-buffer-create
-		  (symbol-name target)))))
-      (with-current-buffer buf
+
+   ((bufferp target)
+    (with-current-buffer target
       (cond ((eq mode 'overwrite)
              (erase-buffer))
             ((eq mode 'append)
              (goto-char (point-max))))
-	(point-marker))))
+      (point-marker)))
 
    ((functionp target) nil)

Re: [PATCH] Proposal: new syntax for eshell-buffer-shorthand (related: bug#19391)

[Eli Zaretskii]

> From: Samer Masterson <samer@samertm.com>
> Cc: emacs-devel@gnu.org
> Date: Sat, 16 May 2015 01:05:36 -0700
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Could you please send the patch in the form equivalent to "diff -w" (I
> > hope Git allows that)?  It's hard to read a large patch where most of
> > the lines "changed" just due to whitespace.
> 
> My bad, I've attached the diff with whitespace ignored.

Thanks, this LGTM.

> What's the process for getting write access to the Emacs repository?

Obtain a user-id on Savannah (if you didn't already), and then ask
here.

Re: [PATCH] Proposal: new syntax for eshell-buffer-shorthand (related: bug#19391)

[Samer Masterson]

Eli Zaretskii <eliz@gnu.org> writes:

> Thanks, this LGTM.

Thanks!

> Obtain a user-id on Savannah (if you didn't already), and then ask
> here.

My username is "samer". Thanks for the help.

-s

Re: [PATCH] Proposal: new syntax for eshell-buffer-shorthand (related: bug#19391)

[Stefan Monnier]

> My username is "samer". Thanks for the help.

From your Savannah account, request membership in the "emacs" group.


        Stefan

Re: [PATCH] Proposal: new syntax for eshell-buffer-shorthand (related: bug#19391)

[Samer Masterson]

Eli Zaretskii <eliz@gnu.org> writes:

> Thanks, this LGTM.

Installed as e37da5a4a8055826f0fc1051083495a828509672. Thanks for the
review!

-s

Re: [PATCH] Proposal: new syntax for eshell-buffer-shorthand (related: bug#19391)

[Stefan Monnier]

> hope Git allows that)?  It's hard to read a large patch where most of
> the lines "changed" just due to whitespace.

Agreed.  But when you're stuck with it,
we have M-x diff-ignore-whitespace-hunk RET


        Stefan