bug#60999: 30.0.50; [PATCH] Add support for negative indices and index ranges in Eshell

[Jim Porter <jporterbugs@gmail.com>]

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

There are two suggestions in the "Bugs and ideas" section of the Eshell 
manual:

   Allow "$_[-1]", which would indicate the last element of the array

   Make "$x[*]" equal to listing out the full contents of "x"

I think these would be pretty useful, especially for the "$_" variable, 
which gets the last argument of the last command, but if you give it an 
index like "$_[N]", gets the Nth argument of the last command. However, 
it's not as easy to get the second-to-last argument of the last command, 
or to get *all* arguments of the last command. So the above two 
suggestions would be pretty helpful.

Attached is a patch to do this. For the second suggestion, I took some 
liberties and added range syntax, so that "$x[2..5]" returns elements 2, 
3, and 4 (zero-indexed) of x.

I have just one question though: this implementation treats ranges as 
half-open, i.e. "M..N" is [M, N). I think that's the best way of doing 
things (and it matches how 'seq-subseq' works). However, "M..N" is the 
Bash syntax, which uses a closed range, [M, N]. Maybe this would be too 
confusing for users? I'm open to using other tokens aside from ".." if 
that would help. Maybe "M:N" would work? That's the Python syntax, which 
behaves the same way as this patch. Any thoughts?

[-- Attachment #2: 0001-Add-support-for-negative-indices-and-index-ranges-in.patch --]
[-- Type: text/plain, Size: 18628 bytes --]

From 33960881a50daefdf8a866b7cfe56ea8d2a78b1c Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Fri, 20 Jan 2023 13:54:20 -0800
Subject: [PATCH] Add support for negative indices and index ranges in Eshell

* lisp/eshell/esh-util.el (eshell-integer-regexp): New defcustom.

* lisp/eshell/esh-var.el (eshell-parse-indices): Expand docstring.
(eshell-parse-index): New function.
(eshell-apply-indices): Use 'eshell-parse-index' to determine whether
to treat the first index as a regexp.  Simplify the implementation a
bit.
(eshell-index-range): New pcase macro...
(eshell-index-value): ... use it, and restructure the implementation.

* test/lisp/eshell/esh-var-tests.el (esh-var-test/interp-var-indices):
New function...
(esh-var-test/interp-var-indices/list)
(esh-var-test/interp-var-indices/vector)
(esh-var-test/interp-var-indices/ring)
(esh-var-test/interp-var-indices/split): ... use it.
(esh-var-test/interp-var-string-split-indices)
(esh-var-test/interp-var-regexp-split-indices)
(esh-var-test/interp-var-assoc): Expand tests to cover things that
look like numbers or ranges, but aren't.

* doc/misc/eshell.texi (Variables): Describe how to get all arguments
of the last command.
(Dollars Expansion): Explain negative indices and index ranges.
(Bugs and ideas): Remove now-implemented ideas.

* etc/NEWS: Announce this change.
---
 doc/misc/eshell.texi              |  24 +++---
 etc/NEWS                          |   7 ++
 lisp/eshell/esh-util.el           |   4 +
 lisp/eshell/esh-var.el            | 136 +++++++++++++++++++-----------
 test/lisp/eshell/esh-var-tests.el | 102 +++++++++++++++++-----
 5 files changed, 193 insertions(+), 80 deletions(-)

diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index 57a2020fdca..9477d0f5e31 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -1059,7 +1059,9 @@ Variables
 This refers to the last argument of the last command.  With a
 subscript, you can access any argument of the last command.  For
 example, @samp{$_[1]} refers to the second argument of the last
-command (excluding the command name itself).
+command (excluding the command name itself).  To get all arguments of
+the last command, you can use an index range like @samp{$_[..]}
+(@pxref{Dollars Expansion}).
 
 @vindex $$
 @item $$
@@ -1370,11 +1372,20 @@ Dollars Expansion
 @item a sequence
 Expands to the element at the (zero-based) index @var{i} of the
 sequence (@pxref{Sequences Arrays Vectors, Sequences, , elisp, The
-Emacs Lisp Reference Manual}).
+Emacs Lisp Reference Manual}).  If @var{i} is negative, @var{i} counts
+from the end, so -1 refers to the last element of the sequence.
+
+If @var{i} is a range like @code{@var{start}..@var{end}}, this expands
+to a subsequence from the indices @var{start} to @var{end}, where
+@var{end} is excluded.  @var{start} and/or @var{end} can also be
+omitted, which is equivalent to the start and/or end of the entire
+list.  For example, @samp{$@var{expr}[-2..]} expands to the last two
+values of @var{expr}.
 
 @item a string
 Split the string at whitespace, and then expand to the @var{i}th
-element of the resulting sequence.
+element of the resulting sequence.  As above, @var{i} can be a range
+like @code{@var{start}..@var{end}}.
 
 @item an alist
 If @var{i} is a non-numeric value, expand to the value associated with
@@ -2442,13 +2453,6 @@ Bugs and ideas
 
 This way, the user could change it to use rc syntax: @samp{>[2=1]}.
 
-@item Allow @samp{$_[-1]}, which would indicate the last element of the array
-
-@item Make @samp{$x[*]} equal to listing out the full contents of @samp{x}
-
-Return them as a list, so that @samp{$_[*]} is all the arguments of the
-last command.
-
 @item Copy ANSI code handling from @file{term.el} into @file{em-term.el}
 
 Make it possible for the user to send char-by-char to the underlying
diff --git a/etc/NEWS b/etc/NEWS
index 10e91ec4ab9..694f80ebc5b 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -143,6 +143,13 @@ of arguments into a command, such as when defining aliases.  For more
 information, see the "(eshell) Dollars Expansion" node in the Eshell
 manual.
 
++++
+*** Eshell now supports negative numbers and ranges for indices.
+Now, you can retrieve the last element of a list with '$my-list[-1]'
+or get a sublist of elements 2 through 4 with '$my-list[2..5]'.  For
+more information, see the "(eshell) Dollars Expansion" node in the
+Eshell manual.
+
 ---
 *** Eshell now uses 'field' properties in its output.
 In particular, this means that pressing the '<home>' key moves the
diff --git a/lisp/eshell/esh-util.el b/lisp/eshell/esh-util.el
index 544a8a74039..8df1dc9982b 100644
--- a/lisp/eshell/esh-util.el
+++ b/lisp/eshell/esh-util.el
@@ -101,6 +101,10 @@ eshell-number-regexp
 function `string-to-number'."
   :type 'regexp)
 
+(defcustom eshell-integer-regexp (rx (? "-") (+ digit))
+  "Regular expression used to match integer arguments."
+  :type 'regexp)
+
 (defcustom eshell-ange-ls-uids nil
   "List of user/host/id strings, used to determine remote ownership."
   :type '(repeat (cons :tag "Host for User/UID map"
diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el
index 83dd5cb50f5..60aab92b33e 100644
--- a/lisp/eshell/esh-var.el
+++ b/lisp/eshell/esh-var.el
@@ -587,6 +587,9 @@ eshell-glob-function
 
 (defun eshell-parse-indices ()
   "Parse and return a list of index-lists.
+This produces a series of Lisp forms to be processed by
+`eshell-prepare-indices' and ultimately evaluated by
+`eshell-do-eval'.
 
 For example, \"[0 1][2]\" becomes:
   ((\"0\" \"1\") (\"2\"))."
@@ -605,6 +608,36 @@ eshell-parse-indices
 	  (goto-char (1+ end)))))
     (nreverse indices)))
 
+(defun eshell-parse-index (index)
+  "Parse a single INDEX in string form.
+If INDEX looks like a number, return that number.
+
+If INDEX looks like \"[BEGIN]..[END]\", where BEGIN and END look
+like integers, return a cons cell of BEGIN and END as numbers;
+BEGIN and/or END can be omitted here, in which case their value
+in the cons is nil.
+
+Otherwise (including if INDEX is not a string), return
+the original value of INDEX."
+  (save-match-data
+    (cond
+     ((and (stringp index) (get-text-property 0 'number index))
+      (string-to-number index))
+     ((and (stringp index)
+           (not (text-property-any 0 (length index) 'escaped t index))
+           (string-match (rx string-start
+                             (group-n 1 (? (regexp eshell-integer-regexp)))
+                             ".."
+                             (group-n 2 (? (regexp eshell-integer-regexp)))
+                             string-end)
+                         index))
+      (let ((begin (match-string 1 index))
+            (end (match-string 2 index)))
+        (cons (unless (string-empty-p begin) (string-to-number begin))
+              (unless (string-empty-p end) (string-to-number end)))))
+     (t
+      index))))
+
 (defun eshell-eval-indices (indices)
   "Evaluate INDICES, a list of index-lists generated by `eshell-parse-indices'."
   (declare (obsolete eshell-prepare-indices "30.1"))
@@ -716,56 +749,65 @@ eshell-apply-indices
 '/etc/passwd', the variable reference would look like:
 
   ${grep johnw /etc/passwd}[: 2]"
-  (while indices
-    (let ((refs (car indices)))
-      (when (stringp value)
-	(let (separator (index (caar indices)))
-          (when (and (stringp index)
-                     (not (get-text-property 0 'number index)))
-            (setq separator index
-                  refs (cdr refs)))
-	  (setq value (split-string value separator))
-          (unless quoted
-            (setq value (mapcar #'eshell-convert-to-number value)))))
-      (cond
-       ((< (length refs) 0)
-	(error "Invalid array variable index: %s"
-	       (eshell-stringify refs)))
-       ((= (length refs) 1)
-	(setq value (eshell-index-value value (car refs))))
-       (t
-	(let ((new-value (list t)))
-	  (while refs
-	    (nconc new-value
-		   (list (eshell-index-value value
-					     (car refs))))
-	    (setq refs (cdr refs)))
-	  (setq value (cdr new-value))))))
-    (setq indices (cdr indices)))
-  value)
+  (dolist (refs indices value)
+    ;; For string values, check if the first index looks like a
+    ;; regexp, and if so, use that to split the string.
+    (when (stringp value)
+      (let (separator (first (car refs)))
+        (when (stringp (eshell-parse-index first))
+          (setq separator first
+                refs (cdr refs)))
+        (setq value (split-string value separator))
+        (unless quoted
+          (setq value (mapcar #'eshell-convert-to-number value)))))
+    (cond
+     ((< (length refs) 0)
+      (error "Invalid array variable index: %s"
+             (eshell-stringify refs)))
+     ((= (length refs) 1)
+      (setq value (eshell-index-value value (car refs))))
+     (t
+      (let (new-value)
+        (dolist (ref refs)
+          (push (eshell-index-value value ref) new-value))
+        (setq value (nreverse new-value)))))))
+
+(pcase-defmacro eshell-index-range (start end)
+  "A pattern that matches an Eshell index range.
+EXPVAL should be a cons cell, with each slot containing either an
+integer or nil.  If this matches, bind the values of the sltos to
+START and END."
+  (list '\` (cons (list '\, `(and (or (pred integerp) (pred null)) ,start))
+                  (list '\, `(and (or (pred integerp) (pred null)) ,end)))))
 
 (defun eshell-index-value (value index)
   "Reference VALUE using the given INDEX."
-  (when (and (stringp index) (get-text-property 0 'number index))
-    (setq index (string-to-number index)))
-  (if (integerp index)
-      (cond
-       ((ring-p value)
-        (if (> index (ring-length value))
-            (error "Index exceeds length of ring")
-          (ring-ref value index)))
-       ((listp value)
-        (if (> index (length value))
-            (error "Index exceeds length of list")
-          (nth index value)))
-       ((vectorp value)
-        (if (> index (length value))
-            (error "Index exceeds length of vector")
-          (aref value index)))
-       (t
-        (error "Invalid data type for indexing")))
-    ;; INDEX is some non-integer value, so treat VALUE as an alist.
-    (cdr (assoc index value))))
+  (let ((parsed-index (eshell-parse-index index)))
+    (if (ring-p value)
+        (pcase parsed-index
+          ((pred integerp)
+           (ring-ref value parsed-index))
+          ((eshell-index-range start end)
+           (let* ((len (ring-length value))
+                  (real-start (mod (or start 0) len))
+                  (real-end (mod (or end len) len)))
+             (when (and (eq real-end 0)
+                        (not (eq end 0)))
+               (setq real-end len))
+             (ring-convert-sequence-to-ring
+              (seq-subseq (ring-elements value) real-start real-end))))
+          (_
+           (error "Invalid index for ring: %s" index)))
+      (pcase parsed-index
+        ((pred integerp)
+         (when (< parsed-index 0)
+           (setq parsed-index (+ parsed-index (length value))))
+         (seq-elt value parsed-index))
+        ((eshell-index-range start end)
+         (seq-subseq value (or start 0) end))
+        (_
+         ;; INDEX is some non-integer value, so treat VALUE as an alist.
+         (cdr (assoc parsed-index value)))))))
 
 ;;;_* Variable name completion
 
diff --git a/test/lisp/eshell/esh-var-tests.el b/test/lisp/eshell/esh-var-tests.el
index 12412d13640..6767d9289f9 100644
--- a/test/lisp/eshell/esh-var-tests.el
+++ b/test/lisp/eshell/esh-var-tests.el
@@ -72,52 +72,89 @@ esh-var-test/interp-list-var-concat
     (eshell-command-result-equal "echo a$'eshell-test-value'z"
                                  '("a1" 2 "3z"))))
 
-(ert-deftest esh-var-test/interp-var-indices ()
-  "Interpolate list variable with indices"
-  (let ((eshell-test-value '("zero" "one" "two" "three" "four")))
+(defun esh-var-test/interp-var-indices (function &optional range-function)
+  "Test interpolation of an indexable value with indices.
+FUNCTION is a function that takes a list of elements and returns
+the object to test.
+
+RANGE-FUNCTION is a function that takes a list of elements and
+returns the expected result of an index range for the object; if
+nil, use FUNCTION instead."
+  (let ((eshell-test-value
+         (funcall function '("zero" "one" "two" "three" "four")))
+        (range-function (or range-function function)))
+    ;; Positive indices
     (eshell-command-result-equal "echo $eshell-test-value[0]"
                                  "zero")
     (eshell-command-result-equal "echo $eshell-test-value[0 2]"
                                  '("zero" "two"))
     (eshell-command-result-equal "echo $eshell-test-value[0 2 4]"
-                                 '("zero" "two" "four"))))
-
-(ert-deftest esh-var-test/interp-var-indices-subcommand ()
-  "Interpolate list variable with subcommand expansion for indices."
-  (skip-unless (executable-find "echo"))
-  (let ((eshell-test-value '("zero" "one" "two" "three" "four")))
+                                 '("zero" "two" "four"))
+    ;; Negative indices
+    (eshell-command-result-equal "echo $eshell-test-value[-1]"
+                                 "four")
+    (eshell-command-result-equal "echo $eshell-test-value[-1 -3]"
+                                 '("four" "two"))
+    ;; Index ranges
     (eshell-command-result-equal
-     "echo $eshell-test-value[${*echo 0}]"
-     "zero")
+     "echo $eshell-test-value[1..4]"
+     (funcall range-function '("one" "two" "three")))
     (eshell-command-result-equal
-     "echo $eshell-test-value[${*echo 0} ${*echo 2}]"
-     '("zero" "two"))))
+     "echo $eshell-test-value[..2]"
+     (funcall range-function '("zero" "one")))
+    (eshell-command-result-equal
+     "echo $eshell-test-value[-2..]"
+     (funcall range-function '("three" "four")))
+    (eshell-command-result-equal
+     "echo $eshell-test-value[..]"
+     (funcall range-function '("zero" "one" "two" "three" "four")))
+    (eshell-command-result-equal
+     "echo $eshell-test-value[1..4 -2..]"
+     (list (funcall range-function '("one" "two" "three"))
+           (funcall range-function '("three" "four"))))))
+
+(ert-deftest esh-var-test/interp-var-indices/list ()
+  "Interpolate list variable with indices."
+  (esh-var-test/interp-var-indices #'identity))
+
+(ert-deftest esh-var-test/interp-var-indices/vector ()
+  "Interpolate vector variable with indices."
+  (esh-var-test/interp-var-indices #'vconcat))
 
-(ert-deftest esh-var-test/interp-var-split-indices ()
+(ert-deftest esh-var-test/interp-var-indices/ring ()
+  "Interpolate ring variable with indices."
+  (esh-var-test/interp-var-indices #'ring-convert-sequence-to-ring))
+
+(ert-deftest esh-var-test/interp-var-indices/split ()
   "Interpolate string variable with indices."
-  (let ((eshell-test-value "zero one two three four"))
-    (eshell-command-result-equal "echo $eshell-test-value[0]"
-                                 "zero")
-    (eshell-command-result-equal "echo $eshell-test-value[0 2]"
-                                 '("zero" "two"))
-    (eshell-command-result-equal "echo $eshell-test-value[0 2 4]"
-                                 '("zero" "two" "four"))))
+  (esh-var-test/interp-var-indices
+   (lambda (values) (string-join values " "))
+   #'identity))
 
 (ert-deftest esh-var-test/interp-var-string-split-indices ()
   "Interpolate string variable with string splitter and indices."
+  ;; Test using punctuation as a delimiter.
   (let ((eshell-test-value "zero:one:two:three:four"))
     (eshell-command-result-equal "echo $eshell-test-value[: 0]"
                                  "zero")
     (eshell-command-result-equal "echo $eshell-test-value[: 0 2]"
                                  '("zero" "two")))
+  ;; Test using a letter as a delimiter.
   (let ((eshell-test-value "zeroXoneXtwoXthreeXfour"))
     (eshell-command-result-equal "echo $eshell-test-value[X 0]"
                                  "zero")
     (eshell-command-result-equal "echo $eshell-test-value[X 0 2]"
+                                 '("zero" "two")))
+  ;; Test using a number as a delimiter.
+  (let ((eshell-test-value "zero0one0two0three0four"))
+    (eshell-command-result-equal "echo $eshell-test-value[\"0\" 0]"
+                                 "zero")
+    (eshell-command-result-equal "echo $eshell-test-value[\"0\" 0 2]"
                                  '("zero" "two"))))
 
 (ert-deftest esh-var-test/interp-var-regexp-split-indices ()
   "Interpolate string variable with regexp splitter and indices."
+  ;; Test using a regexp as a delimiter.
   (let ((eshell-test-value "zero:one!two:three!four"))
     (eshell-command-result-equal "echo $eshell-test-value['[:!]' 0]"
                                  "zero")
@@ -126,15 +163,34 @@ esh-var-test/interp-var-regexp-split-indices
     (eshell-command-result-equal "echo $eshell-test-value[\"[:!]\" 0]"
                                  "zero")
     (eshell-command-result-equal "echo $eshell-test-value[\"[:!]\" 0 2]"
+                                 '("zero" "two")))
+  ;; Test using a regexp that looks like range syntax as a delimiter.
+  (let ((eshell-test-value "zero0..0one0..0two0..0three0..0four"))
+    (eshell-command-result-equal "echo $eshell-test-value[\"0..0\" 0]"
+                                 "zero")
+    (eshell-command-result-equal "echo $eshell-test-value[\"0..0\" 0 2]"
                                  '("zero" "two"))))
 
 (ert-deftest esh-var-test/interp-var-assoc ()
   "Interpolate alist variable with index."
-  (let ((eshell-test-value '(("foo" . 1) (bar . 2))))
+  (let ((eshell-test-value '(("foo" . 1) (bar . 2) ("3" . "three"))))
     (eshell-command-result-equal "echo $eshell-test-value[foo]"
                                  1)
     (eshell-command-result-equal "echo $eshell-test-value[#'bar]"
-                                 2)))
+                                 2)
+    (eshell-command-result-equal "echo $eshell-test-value[\"3\"]"
+                                 "three")))
+
+(ert-deftest esh-var-test/interp-var-indices-subcommand ()
+  "Interpolate list variable with subcommand expansion for indices."
+  (skip-unless (executable-find "echo"))
+  (let ((eshell-test-value '("zero" "one" "two" "three" "four")))
+    (eshell-command-result-equal
+     "echo $eshell-test-value[${*echo 0}]"
+     "zero")
+    (eshell-command-result-equal
+     "echo $eshell-test-value[${*echo 0} ${*echo 2}]"
+     '("zero" "two"))))
 
 (ert-deftest esh-var-test/interp-var-length-list ()
   "Interpolate length of list variable."
-- 
2.25.1