bug#18960: 24.4; Eshell history substitution causes error

bug#18960: 24.4; Eshell history substitution causes error

[Emanuel Evans]

I'm not entirely sure whether history substitution in Eshell is a
supported feature, but attempting to use it leads to an error. Steps to
reproduce (from emacs -Q, with debug-on-error set to t):

1. M-x eshell
2. cd /
3. ls bin
4. ^bin^usr

Expected result: output of "ls usr"
Actual result: stacktrace:

Debugger entered--Lisp error: (wrong-type-argument stringp 108)
  string-match("bin" 108)
  (if (string-match "bin" str) (setq str (replace-match "usr" t nil str)))
  (lambda (str) (if (string-match "bin" str) (setq str (replace-match "usr" t nil str))) str)(108)
  mapcar((lambda (str) (if (string-match "bin" str) (setq str (replace-match "usr" t nil str))) str) "lsbin")
  (lambda (lst) (mapcar (function (lambda (str) (if (string-match "bin" str) (setq str (replace-match "usr" t nil str))) str)) lst))("lsbin")
  eshell-hist-parse-modifier("lsbin" ":s/bin/usr/")
  eshell-history-reference("^bin^usr")
  eshell-expand-history-references(#<marker at 777 in *eshell*> 785)
  run-hook-with-args(eshell-expand-history-references #<marker at 777 in *eshell*> 785)
  #[0 "\300\304	`S\"\240\210\305\306	`S#\210\307	`S\"\211\205?.\310	!\210\300\304\n\vS\"\240\210\311\312!\210\313\314\315\316\317\320\321\300.\b\"\322\"\323\324%D\215\205?.\325 \207" [("^bin^usr") eshell-last-output-end eshell-last-input-start eshell-last-input-end buffer-substring-no-properties run-hook-with-args eshell-expand-input-functions eshell-parse-command-input eshell-update-markers run-hooks eshell-input-filter-functions eshell-terminal funcall make-byte-code 0 "\302\303\301!\203\f.\304\301!\210\207\305\301\300\242\"\210\207" vconcat vector [nil eshell-invoke-directly eval eshell-eval-command] 4 "\n\n(fn)" eshell-life-is-too-much] 10 "\n\n(fn)"]()
  funcall(#[0 "\300\304	`S\"\240\210\305\306	`S#\210\307	`S\"\211\205?.\310	!\210\300\304\n\vS\"\240\210\311\312!\210\313\314\315\316\317\320\321\300.\b\"\322\"\323\324%D\215\205?.\325 \207" [("^bin^usr") eshell-last-output-end eshell-last-input-start eshell-last-input-end buffer-substring-no-properties run-hook-with-args eshell-expand-input-functions eshell-parse-command-input eshell-update-markers run-hooks eshell-input-filter-functions eshell-terminal funcall make-byte-code 0 "\302\303\301!\203\f.\304\301!\210\207\305\301\300\242\"\210\207" vconcat vector [nil eshell-invoke-directly eval eshell-eval-command] 4 "\n\n(fn)" eshell-life-is-too-much] 10 "\n\n(fn)"])
  eshell-send-input(nil)
  call-interactively(eshell-send-input nil nil)
  command-execute(eshell-send-input)




In GNU Emacs 24.4.1 (x86_64-apple-darwin14.0.0, NS apple-appkit-1343.14)
 of 2014-10-29 on WorkBook-Pro-II.local
Configured using:
 `configure --prefix=/usr/local/Cellar/emacs/24.4
 --enable-locallisppath=/usr/local/share/emacs/site-lisp
 --infodir=/usr/local/Cellar/emacs/24.4/share/info/emacs --without-dbus
 --with-gnutls --with-rsvg --with-imagemagick --with-ns
 --disable-ns-self-contained'

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: EShell

Minor modes in effect:
  tooltip-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent input:
ESC [ ? 1 ; 2 c ESC x e s h TAB RET c d SPC / RET l 
s SPC b i n RET ^ b i n ^ u s r RET DEL DEL DEL DEL 
DEL DEL DEL DEL ESC x s e t - v a TAB RET d e u DEL 
b u g TAB o n TAB e r TAB RET t RET ESC O A ESC O A 
ESC O B RET ESC O A ESC O A ESC O A ESC O A ESC O A 
ESC O A ESC O B ESC O B ESC O B ESC O B ESC O B ESC 
O B DEL DEL DEL DEL DEL DEL DEL DEL DEL DEL DEL DEL 
DEL ^ b i n ^ u s r RET C-a C-@ ESC > ESC w q ESC x 
r e p o r t TAB RET

Recent messages:
History item: 11
History item: 12
History item: 13
History item: 14
History item: 1
delete-backward-char: Text is read-only
Entering debugger...
Mark set
Saved text from "Debugger entered--Lisp error: (wrong-typ"
Back to top level.

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug help-mode debug cus-edit wid-edit
cus-start cus-load em-unix em-term term disp-table ehelp em-script
em-prompt em-ls em-hist em-pred em-glob em-dirs em-cmpl em-basic
em-banner em-alias pcomplete comint ansi-color ring esh-var esh-io
esh-cmd esh-opt esh-ext esh-proc esh-arg eldoc esh-groups eshell
esh-module esh-mode esh-util gnutls cl-loaddefs cl-lib mm-archive
message format-spec rfc822 mml easymenu mml-sec mailabbrev gmm-utils
mailheader mm-decode mm-bodies mm-encode network-stream starttls
url-http tls mail-parse rfc2231 url-gw url-cache url-auth url url-proxy
url-privacy url-expand url-methods url-history url-cookie url-domsuf
url-util mailcap url-handlers url-parse url-vars epg smtpmail
auth-source eieio byte-opt bytecomp byte-compile cconv eieio-core
gnus-util password-cache sendmail rfc2047 rfc2045 ietf-drums mm-util
help-fns mail-prsvr mail-utils package epg-config xterm time-date
tooltip electric uniquify ediff-hook vc-hooks lisp-float-type mwheel
ns-win tool-bar dnd fontset image regexp-opt fringe tabulated-list
newcomment lisp-mode prog-mode register page menu-bar rfn-eshadow timer
select scroll-bar mouse jit-lock font-lock syntax facemenu font-core
frame cham georgian utf-8-lang misc-lang vietnamese tibetan thai
tai-viet lao korean japanese hebrew greek romanian slovak czech european
ethiopic indian cyrillic chinese case-table epa-hook jka-cmpr-hook help
simple abbrev minibuffer nadvice loaddefs button faces cus-face macroexp
files text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote make-network-process
cocoa ns multi-tty emacs)

Memory information:
((conses 16 151620 23576)
 (symbols 48 25175 0)
 (miscs 40 49 239)
 (strings 32 33041 6577)
 (string-bytes 1 919530)
 (vectors 16 22524)
 (vector-slots 8 1192094 156060)
 (floats 8 83 614)
 (intervals 56 511 95)
 (buffers 960 15))

bug#18960: 24.4; Eshell history substitution causes error

[Samer Masterson]

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

The attached patch fixes this issue. This is my second emacs patch, so let
me know if I'm doing anything wrong or nonstandard.

Best,
Samer

Patch:
1 file changed, 2 insertions(+), 2 deletions(-)
 lisp/eshell/em-hist.el | 4 ++--

Modified   lisp/eshell/em-hist.el
diff --git a/lisp/eshell/em-hist.el b/lisp/eshell/em-hist.el
index b721b5d..e7e9a1e 100644
--- a/lisp/eshell/em-hist.el
+++ b/lisp/eshell/em-hist.el
@@ -724,7 +724,7 @@ matched."
  (setq nth (eshell-hist-word-reference nth)))
       (unless (numberp mth)
  (setq mth (eshell-hist-word-reference mth)))
-      (cons (mapconcat 'identity (eshell-sublist textargs nth mth) "")
+      (cons (mapconcat 'identity (eshell-sublist textargs nth mth) " ")
     end))))

 (defun eshell-hist-parse-modifier (hist reference)
@@ -737,7 +737,7 @@ matched."
   (goto-char (point-min))
   (let ((modifiers (cdr (eshell-parse-modifiers))))
     (dolist (mod modifiers)
-      (setq hist (funcall mod hist)))
+      (setq hist (car (funcall mod (list hist)))))
     hist))
       (delete-region here (point)))))

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

bug#18960: 24.4; Eshell history substitution causes error

[Stefan Monnier]

> Modified   lisp/eshell/em-hist.el
> diff --git a/lisp/eshell/em-hist.el b/lisp/eshell/em-hist.el
> index b721b5d..e7e9a1e 100644
> --- a/lisp/eshell/em-hist.el
> +++ b/lisp/eshell/em-hist.el
> @@ -724,7 +724,7 @@ matched."
>   (setq nth (eshell-hist-word-reference nth)))
>        (unless (numberp mth)
>   (setq mth (eshell-hist-word-reference mth)))
> -      (cons (mapconcat 'identity (eshell-sublist textargs nth mth) "")
> +      (cons (mapconcat 'identity (eshell-sublist textargs nth mth) " ")
>      end))))

Can you explain why this is needed?

> @@ -737,7 +737,7 @@ matched."
>    (goto-char (point-min))
>    (let ((modifiers (cdr (eshell-parse-modifiers))))
>      (dolist (mod modifiers)
> -      (setq hist (funcall mod hist)))
> +      (setq hist (car (funcall mod (list hist)))))
>      hist))
>        (delete-region here (point)))))

The docstring of eshell-parse-modifiers says:

   Parse value modifiers and predicates at point.
   If ALLOW-PREDS is non-nil, predicates will be parsed as well.
   Return a cons cell of the form
   
     (PRED-FUNC-LIST . MOD-FUNC-LIST)
   
   NEW-STRING is STRING minus any modifiers.  PRED-FUNC-LIST is a list of
   predicate functions.  MOD-FUNC-LIST is a list of result modifier
   functions.  PRED-FUNCS take a filename and return t if the test
   succeeds; MOD-FUNCS take any string and preform a modification,
   returning the resultant string.

If I read this right, `mod' (which is one of MOD-FUNCS) should take
a string and return a string, IIUC.  So, maybe the right fix is not to
change this code but to change the functions returned by
`eshell-parse-modifiers'.



        Stefan

bug#18960: Fwd: bug#18960: 24.4; Eshell history substitution causes error

[Samer Masterson]

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

On Tue, Dec 9, 2014 at 2:16 PM, Stefan Monnier <monnier@iro.umontreal.ca>
wrote:

> > Modified   lisp/eshell/em-hist.el
> > diff --git a/lisp/eshell/em-hist.el b/lisp/eshell/em-hist.el
> > index b721b5d..e7e9a1e 100644
> > --- a/lisp/eshell/em-hist.el
> > +++ b/lisp/eshell/em-hist.el
> > @@ -724,7 +724,7 @@ matched."
> >   (setq nth (eshell-hist-word-reference nth)))
> >        (unless (numberp mth)
> >   (setq mth (eshell-hist-word-reference mth)))
> > -      (cons (mapconcat 'identity (eshell-sublist textargs nth mth) "")
> > +      (cons (mapconcat 'identity (eshell-sublist textargs nth mth) " ")
> >      end))))
>
> Can you explain why this is needed?
>

(eshell-sublist) will return a sublist of textargs, which might look like
("ehco" "hi" "there"). The call to mapconcat would join that list without
adding whitespace between the args previously, e.g. into "echohithere",
which is incorrect. This then gets fed into eshell-hist-parse-modifier as
event on line 663. eshell-hist-parse-modifier would then make the
substitution on "echohithere".


>
> > @@ -737,7 +737,7 @@ matched."
> >    (goto-char (point-min))
> >    (let ((modifiers (cdr (eshell-parse-modifiers))))
> >      (dolist (mod modifiers)
> > -      (setq hist (funcall mod hist)))
> > +      (setq hist (car (funcall mod (list hist)))))
> >      hist))
> >        (delete-region here (point)))))
>
> The docstring of eshell-parse-modifiers says:
>
>    Parse value modifiers and predicates at point.
>    If ALLOW-PREDS is non-nil, predicates will be parsed as well.
>    Return a cons cell of the form
>
>      (PRED-FUNC-LIST . MOD-FUNC-LIST)
>
>    NEW-STRING is STRING minus any modifiers.  PRED-FUNC-LIST is a list of
>    predicate functions.  MOD-FUNC-LIST is a list of result modifier
>    functions.  PRED-FUNCS take a filename and return t if the test
>    succeeds; MOD-FUNCS take any string and preform a modification,
>    returning the resultant string.
>
> If I read this right, `mod' (which is one of MOD-FUNCS) should take
> a string and return a string, IIUC.  So, maybe the right fix is not to
> change this code but to change the functions returned by
> `eshell-parse-modifiers'.
>
 Ah, I should have read the docstring for eshell-parse-modifiers instead of
simply looking at the function it returns :) I'll make that change. Also, I
assume that NEW-STRING and ALLOW-PREDS both refer to variables that were
removed when this file was made lexically scoped, and will remove them from
the docstring.

>
>
>
>         Stefan
>

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

bug#18960: 24.4; Eshell history substitution causes error

[Samer Masterson]

eshell-parse-modifiers pulls its modifiers from eshell-modifier-alist,
where are all functions that operate on lists of strings, so I updated
the docstring for eshell-parse-modifiers to reflect that. I've also
added my changes to the ChangeLog.

Best,
Samer

Changes from HEAD to working tree
3 files changed, 15 insertions(+), 8 deletions(-)
 lisp/ChangeLog         |    8 ++++++++
 lisp/eshell/em-hist.el |    4 ++--
 lisp/eshell/em-pred.el |   11 +++++------

	Modified   lisp/ChangeLog
diff --git a/lisp/ChangeLog b/lisp/ChangeLog
index 45ba279..c30c43a 100644
--- a/lisp/ChangeLog
+++ b/lisp/ChangeLog
@@ -1,3 +1,11 @@
+2014-12-10    <samer@localhost>
+
+	* eshell/em-hist.el (eshell-hist-parse-word-designator): Return
+	args joined with " ".
+	* eshell/em-pred.el (eshell-parse-modifiers): Correct docstring.
+	(eshell-hist-parse-modifier): Pass mod a list instead of a string
+	(bug#18960).
+
 2014-11-28  Martin Rudalics  <rudalics@gmx.at>

 	Fix two issues around help-window-select.  (Bug#11039) (Bug#19012)
	Modified   lisp/eshell/em-hist.el
diff --git a/lisp/eshell/em-hist.el b/lisp/eshell/em-hist.el
index b721b5d..e7e9a1e 100644
--- a/lisp/eshell/em-hist.el
+++ b/lisp/eshell/em-hist.el
@@ -724,7 +724,7 @@ matched."
 	(setq nth (eshell-hist-word-reference nth)))
       (unless (numberp mth)
 	(setq mth (eshell-hist-word-reference mth)))
-      (cons (mapconcat 'identity (eshell-sublist textargs nth mth) "")
+      (cons (mapconcat 'identity (eshell-sublist textargs nth mth) " ")
 	    end))))

 (defun eshell-hist-parse-modifier (hist reference)
@@ -737,7 +737,7 @@ matched."
 	  (goto-char (point-min))
 	  (let ((modifiers (cdr (eshell-parse-modifiers))))
 	    (dolist (mod modifiers)
-	      (setq hist (funcall mod hist)))
+	      (setq hist (car (funcall mod (list hist)))))
 	    hist))
       (delete-region here (point)))))

	Modified   lisp/eshell/em-pred.el
diff --git a/lisp/eshell/em-pred.el b/lisp/eshell/em-pred.el
index f55d687..7d6e326 100644
--- a/lisp/eshell/em-pred.el
+++ b/lisp/eshell/em-pred.el
@@ -296,16 +296,15 @@ This function is specially for adding onto
`eshell-parse-argument-hook'."

 (defun eshell-parse-modifiers ()
   "Parse value modifiers and predicates at point.
-If ALLOW-PREDS is non-nil, predicates will be parsed as well.
 Return a cons cell of the form

   (PRED-FUNC-LIST . MOD-FUNC-LIST)

-NEW-STRING is STRING minus any modifiers.  PRED-FUNC-LIST is a list of
-predicate functions.  MOD-FUNC-LIST is a list of result modifier
-functions.  PRED-FUNCS take a filename and return t if the test
-succeeds; MOD-FUNCS take any string and preform a modification,
-returning the resultant string."
+PRED-FUNC-LIST is a list of predicate functions.  MOD-FUNC-LIST
+is a list of result modifier functions.  PRED-FUNCS take a
+filename and return t if the test succeeds; MOD-FUNCS take any
+list of strings and perform a modification, returning the
+resultant list of strings."
   (let (negate follow preds mods)
     (condition-case nil
 	(while (not (eobp))

bug#18960: 24.4; Eshell history substitution causes error

[Samer Masterson]

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

Hi,

My copyright papers have been processed. Can we apply this to master?

Best,
Samer

Patch below:

Changes from HEAD to working tree
3 files changed, 15 insertions(+), 8 deletions(-)
 lisp/ChangeLog         |    8 ++++++++
 lisp/eshell/em-hist.el |    4 ++--
 lisp/eshell/em-pred.el |   11 +++++------

	Modified   lisp/ChangeLog
diff --git a/lisp/ChangeLog b/lisp/ChangeLog
index 45ba279..c30c43a 100644
--- a/lisp/ChangeLog
+++ b/lisp/ChangeLog
@@ -1,3 +1,11 @@
+2014-12-10    <samer <at> localhost>
+
+	* eshell/em-hist.el (eshell-hist-parse-word-designator): Return
+	args joined with " ".
+	* eshell/em-pred.el (eshell-parse-modifiers): Correct docstring.
+	(eshell-hist-parse-modifier): Pass mod a list instead of a string
+	(bug#18960).
+
 2014-11-28  Martin Rudalics  <rudalics <at> gmx.at>

 	Fix two issues around help-window-select.  (Bug#11039) (Bug#19012)
	Modified   lisp/eshell/em-hist.el
diff --git a/lisp/eshell/em-hist.el b/lisp/eshell/em-hist.el
index b721b5d..e7e9a1e 100644
--- a/lisp/eshell/em-hist.el
+++ b/lisp/eshell/em-hist.el
@@ -724,7 +724,7 @@ matched."
 	(setq nth (eshell-hist-word-reference nth)))
       (unless (numberp mth)
 	(setq mth (eshell-hist-word-reference mth)))
-      (cons (mapconcat 'identity (eshell-sublist textargs nth mth) "")
+      (cons (mapconcat 'identity (eshell-sublist textargs nth mth) " ")
 	    end))))

 (defun eshell-hist-parse-modifier (hist reference)
@@ -737,7 +737,7 @@ matched."
 	  (goto-char (point-min))
 	  (let ((modifiers (cdr (eshell-parse-modifiers))))
 	    (dolist (mod modifiers)
-	      (setq hist (funcall mod hist)))
+	      (setq hist (car (funcall mod (list hist)))))
 	    hist))
       (delete-region here (point)))))

	Modified   lisp/eshell/em-pred.el
diff --git a/lisp/eshell/em-pred.el b/lisp/eshell/em-pred.el
index f55d687..7d6e326 100644
--- a/lisp/eshell/em-pred.el
+++ b/lisp/eshell/em-pred.el
@@ -296,16 +296,15 @@ This function is specially for adding onto
`eshell-parse-argument-hook'."

 (defun eshell-parse-modifiers ()
   "Parse value modifiers and predicates at point.
-If ALLOW-PREDS is non-nil, predicates will be parsed as well.
 Return a cons cell of the form

   (PRED-FUNC-LIST . MOD-FUNC-LIST)

-NEW-STRING is STRING minus any modifiers.  PRED-FUNC-LIST is a list of
-predicate functions.  MOD-FUNC-LIST is a list of result modifier
-functions.  PRED-FUNCS take a filename and return t if the test
-succeeds; MOD-FUNCS take any string and preform a modification,
-returning the resultant string."
+PRED-FUNC-LIST is a list of predicate functions.  MOD-FUNC-LIST
+is a list of result modifier functions.  PRED-FUNCS take a
+filename and return t if the test succeeds; MOD-FUNCS take any
+list of strings and perform a modification, returning the
+resultant list of strings."
   (let (negate follow preds mods)
     (condition-case nil
 	(while (not (eobp))

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

bug#18960: 24.4; Eshell history substitution causes error

[Stefan Monnier]

> My copyright papers have been processed. Can we apply this to master?

Thanks, installed in master (tho I used your previous patch for that,
since the latest one had more transmission-induced errors).


        Stefan