bug#29821: Ensure quick substitution only occurs at start of line

[Jay Kamat]

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

> With your patch, if I do
>
>     ~/src/emacs $ ^this string not present in history^blah^
>
> I get the latest entry in the history substituted and re-executed.  In
> bash I get
>
>     ~/tmp$ ^this string not present in history^blah^
>     bash: :s^this string not present in history^blah^: substitution failed
>
> (if it's easier, I think it would be okay if eshell prints an error
> using the !!:s/foo/bar/ syntax, but this case must be an error)

Ah, I did notice that, but I was not sure whether it was a bug or
desired behavior (as it seemed to occur for me even before this patch).
I've added a tiny change to the patch to fix that, but it has the side
effect of doing:

> *[j@laythe emacs-bisect]$ echo "foo"(:s/bar/baz/)
> foo: substitution failed

But I think that's an OK change, especially if we want to error out on
^bar^baz when no search is found.

I also discovered another issue (which existed before as well):

> *[j@laythe emacs-bisect]$ echo one one one
> ("one" "one" "one")
> *[j@laythe emacs-bisect]$ !!:sg/one/two
> :sg/one/two
> Wrong type argument: integer-or-marker-p, nil

but I'd rather take a look at that later on to avoid cluttering this
changeset. (and I'm not sure if I'm just using the feature incorrectly).

Thanks,
-Jay

>From d4ba97c7b15c48d3394b8567b05fae31874220a7 Mon Sep 17 00:00:00 2001
From: Jay Kamat <jaygkamat@gmail.com>
Date: Fri, 22 Dec 2017 15:34:44 -0800
Subject: [PATCH] Prevent expansion of quick substitutions when not at start of
 line

See bug #29157 for an initial report

Also, allow spaces inside substitutions, so ^foo bar^baz works.

* lisp/eshell/em-hist.el (eshell-expand-history-references): Expand
  history substitution before other types of expansions, and expand
  them with the whole line.
(eshell-history-substitution): New function to expand only
substitutions, taking in the entire typed line rather than individual
arguments.
---
 lisp/eshell/em-hist.el | 57 ++++++++++++++++++++++++++++++++------------------
 lisp/eshell/em-pred.el |  3 ++-
 2 files changed, 39 insertions(+), 21 deletions(-)

diff --git a/lisp/eshell/em-hist.el b/lisp/eshell/em-hist.el
index df462a7058..d4d4b93b81 100644
--- a/lisp/eshell/em-hist.el
+++ b/lisp/eshell/em-hist.el
@@ -581,21 +581,30 @@ eshell-hist-parse-arguments
 
 (defun eshell-expand-history-references (beg end)
   "Parse and expand any history references in current input."
-  (let ((result (eshell-hist-parse-arguments beg end)))
+  (let ((result (eshell-hist-parse-arguments beg end))
+       (full-line (buffer-substring-no-properties beg end)))
     (when result
       (let ((textargs (nreverse (nth 0 result)))
            (posb (nreverse (nth 1 result)))
-           (pose (nreverse (nth 2 result))))
+           (pose (nreverse (nth 2 result)))
+           (full-line-subst (eshell-history-substitution full-line)))
        (save-excursion
-         (while textargs
-           (let ((str (eshell-history-reference (car textargs))))
-             (unless (eq str (car textargs))
-               (goto-char (car posb))
-               (insert-and-inherit str)
-               (delete-char (- (car pose) (car posb)))))
-           (setq textargs (cdr textargs)
-                 posb (cdr posb)
-                 pose (cdr pose))))))))
+         (if full-line-subst
+             ;; Found a ^foo^bar substitution
+             (progn
+               (goto-char beg)
+               (insert-and-inherit full-line-subst)
+               (delete-char (- end beg)))
+           ;; Try to expand other substitutions
+           (while textargs
+             (let ((str (eshell-history-reference (car textargs))))
+               (unless (eq str (car textargs))
+                 (goto-char (car posb))
+                 (insert-and-inherit str)
+                 (delete-char (- (car pose) (car posb)))))
+             (setq textargs (cdr textargs)
+                   posb (cdr posb)
+                   pose (cdr pose)))))))))
 
 (defvar pcomplete-stub)
 (defvar pcomplete-last-completion-raw)
@@ -630,20 +639,28 @@ eshell-complete-history-reference
                   (setq history (cdr history)))
                 (cdr fhist)))))))
 
+(defun eshell-history-substitution (line)
+  "Expand whole-line history substitutions by converting them to
+!!:s/a/b/ syntax.
+Returns nil if no match found."
+  ;; `^string1^string2^'
+  ;;      Quick Substitution.  Repeat the last command, replacing
+  ;;      STRING1 with STRING2.  Equivalent to `!!:s/string1/string2/'
+  (when (and (eshell-using-module 'eshell-pred)
+            (string-match "^\\^\\([^^]+\\)\\^\\([^^]+\\)\\^?\\s-*$"
+                          line))
+    (let* ((reference (format "!!:s/%s/%s/"
+                             (match-string 1 line)
+                             (match-string 2 line)))
+          (result (eshell-history-reference reference)))
+      (unless (eq result reference)
+       result))))
+
 (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/'
-  (if (and (eshell-using-module 'eshell-pred)
-          (string-match "\\^\\([^^]+\\)\\^\\([^^]+\\)\\^?\\s-*$"
-                        reference))
-      (setq reference (format "!!:s/%s/%s/"
-                             (match-string 1 reference)
-                             (match-string 2 reference))))
   ;; `!'
   ;;      Start a history substitution, except when followed by a
   ;;      space, tab, the end of the line, = or (.
diff --git a/lisp/eshell/em-pred.el b/lisp/eshell/em-pred.el
index 72a7bc4afc..86a9d4262a 100644
--- a/lisp/eshell/em-pred.el
+++ b/lisp/eshell/em-pred.el
@@ -545,7 +545,8 @@ eshell-pred-substitute
          (function
           (lambda (str)
             (if (string-match ,match str)
-                (setq str (replace-match ,replace t nil str)))
+                (setq str (replace-match ,replace t nil str))
+              (error (concat str ": substitution failed")))
             str)) lst)))))
 
 (defun eshell-include-members (&optional invert-p)
-- 
2.11.0