Subject: bug#60999: closed (Re: bug#60999: 30.0.50; [PATCH] Add support for negative indices and index ranges in Eshell)

From: [email-masked] (GNU bug Tracking System)
Date: Sat, 28 Jan 2023 02:16:02 +0000

Your bug report

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

which was filed against the emacs package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
60999: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=60999
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#60999: 30.0.50; [PATCH] Add support for negative indices and index ranges in Eshell
From: Jim Porter <[email-masked]>
To: Eli Zaretskii <[email-masked]>
Cc: [email-masked]
Date: Fri, 27 Jan 2023 18:15:28 -0800
List-Id: <debbugs-submit.debbugs.gnu.org>

On 1/27/2023 10:57 AM, Eli Zaretskii wrote:
> Feel free.

Thanks. Merged as 5642bf0b97. I'll send a message to emacs-devel over 
the weekend.

Closing this now.

Subject: 30.0.50; [PATCH] Add support for negative indices and index ranges in Eshell
From: Jim Porter <[email-masked]>
To: [email-masked]
Date: Sat, 21 Jan 2023 19:47:47 -0800
List-Id: <debbugs-submit.debbugs.gnu.org>

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?

From 33960881a50daefdf8a866b7cfe56ea8d2a78b1c Mon Sep 17 00:00:00 2001
From: Jim Porter <[email-masked]>
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