GNU bug report logs - #29821
eshell: Ensure quick substitution only occurs at start of line

Previous Next

Full log

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

From: Noam Postavsky <npostavs <at> users.sourceforge.net>
To: Jay Kamat <jaygkamat <at> gmail.com>
Cc: 29821 <at> debbugs.gnu.org, Andreas Schwab <schwab <at> linux-m68k.org>
Subject: Re: bug#29821: Ensure quick substitution only occurs at start of line
Date: Tue, 02 Jan 2018 20:51:36 -0500

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

> Noam Postavsky <npostavs <at> users.sourceforge.net> writes:
>
>> Couldn't you error here (if the line matches ^...^...^) instead of
>> returning nil, and then avoid affecting the other substitution?
>> (although I agree signaling an error in the other place is probably
>> acceptable)
>
> I could be missing something, but I don't think this is that easy. In
> the case of a failed search for something like '!!:s/a/b/',
> `eshell-history-reference' previously returned the previous line,
> unmodified.

Oh, yes, I was confused by your docstring.  By "if no match found" you
meant when the line doesn't match ^foo^bar^ at all; I had somehow got
the impression you meant that there was no match for "foo".

> +(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)

This eq test will always be nil, right?  Because the only time it's t
is when you pass something that's not a history reference, but the thing
we passed is a history reference by construction.  So this could be
simplified to

  (when (and (eshell-using-module 'eshell-pred)
             (string-match "^\\^\\([^^]+\\)\\^\\([^^]+\\)\\^?\\s-*$"
                           line))
    (eshell-history-reference
     (format "!!:s/%s/%s/"
             (match-string 1 line)
             (match-string 2 line))))

That, plus rephrasing the docstring so the first sentence fits on one
line (it should probably also mention ^foo^bar^ syntax), and I think the
patch is good to go (for master).

Previous Next

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