all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#60999: 30.0.50; [PATCH] Add support for negative indices and index ranges in Eshell
@ 2023-01-22  3:47 Jim Porter
  2023-01-22  6:29 ` Eli Zaretskii
  0 siblings, 1 reply; 8+ messages in thread
From: Jim Porter @ 2023-01-22  3:47 UTC (permalink / raw)
  To: 60999

[-- 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


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* bug#60999: 30.0.50; [PATCH] Add support for negative indices and index ranges in Eshell
  2023-01-22  3:47 bug#60999: 30.0.50; [PATCH] Add support for negative indices and index ranges in Eshell Jim Porter
@ 2023-01-22  6:29 ` Eli Zaretskii
  2023-01-22  8:13   ` Jim Porter
  0 siblings, 1 reply; 8+ messages in thread
From: Eli Zaretskii @ 2023-01-22  6:29 UTC (permalink / raw)
  To: Jim Porter; +Cc: 60999

> Date: Sat, 21 Jan 2023 19:47:47 -0800
> From: Jim Porter <jporterbugs@gmail.com>
> 
> 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?

I think compatibility to other shells is an advantage.

> +(defcustom eshell-integer-regexp (rx (? "-") (+ digit))
> +  "Regular expression used to match integer arguments."
> +  :type 'regexp)

Why is this a defcustom? what alternative could a user possibly want?

Thanks.





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#60999: 30.0.50; [PATCH] Add support for negative indices and index ranges in Eshell
  2023-01-22  6:29 ` Eli Zaretskii
@ 2023-01-22  8:13   ` Jim Porter
  2023-01-27  1:23     ` Jim Porter
  0 siblings, 1 reply; 8+ messages in thread
From: Jim Porter @ 2023-01-22  8:13 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 60999

On 1/21/2023 10:29 PM, Eli Zaretskii wrote:
>> Date: Sat, 21 Jan 2023 19:47:47 -0800
>> From: Jim Porter <jporterbugs@gmail.com>
>>
>> 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? [snip]
> 
> I think compatibility to other shells is an advantage.

True. Though, in practice I find that the half-open range is easier to 
use whenever things become more complex than just using constant values. 
For example, truncating a larger list to be the same size as a shorter 
one ("$#" means "length of"):

   $long-list[..$#short-list]

Using Bash-style closed ranges, you'd have to do:

   $long-list[..${1- $#short-list}]

(Note that unlike Bash syntax, I made this so that M and N are optional. 
That's another thing we could consider changing.)

Half-open ranges are also more consistent with Emacs Lisp in my 
experience. It would be strange (at least to me) to have things like 
'seq-subseq', 'add-text-properties', 'substring', etc use one way of 
specifying ranges, and Eshell to use another, especially when you can 
call any of those functions within Eshell. But like you say, 
compatibility with other shells is an advantage too...

In that sense, some non-Bash syntax might be nice. However, Eshell 
subscripts are already fairly complex, so we'd need to be careful about 
what we choose so that we don't break compatibility. For example, when 
subscripting a string, the first element in the subscript can be a 
regexp that splits the string, so to get the first element in your PATH, 
you could do:

   $PATH[: 0]

If ":" was the range separator, this would change meaning. There are 
ways to prevent that incompatible change if we're careful, but it's 
something to keep in mind. ".." has the advantage that while it's a 
valid regexp, it's not very useful in this context, so we probably 
wouldn't break anything in practice.

>> +(defcustom eshell-integer-regexp (rx (? "-") (+ digit))
>> +  "Regular expression used to match integer arguments."
>> +  :type 'regexp)
> 
> Why is this a defcustom? what alternative could a user possibly want?

Just for symmetry with 'eshell-number-regexp'. Maybe that shouldn't be a 
defcustom either though; if someone wanted to change 
'eshell-number-regexp', that's probably a sign that there's a bug, and 
the default value should be improved.

Either way we decide about 'eshell-number-regexp', I can turn 
'eshell-integer-regexp' into a regular defvar. (The only thing I can 
think of that a person would customize it to would be to allow a "+" at 
the start of an integer, like "+123".)





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#60999: 30.0.50; [PATCH] Add support for negative indices and index ranges in Eshell
  2023-01-22  8:13   ` Jim Porter
@ 2023-01-27  1:23     ` Jim Porter
  2023-01-27  7:38       ` Eli Zaretskii
  0 siblings, 1 reply; 8+ messages in thread
From: Jim Porter @ 2023-01-27  1:23 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 60999

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

On 1/22/2023 12:13 AM, Jim Porter wrote:
> Either way we decide about 'eshell-number-regexp', I can turn 
> 'eshell-integer-regexp' into a regular defvar. (The only thing I can 
> think of that a person would customize it to would be to allow a "+" at 
> the start of an integer, like "+123".)

Ok, I've updated my patch to that 'eshell-integer-regexp' is just a 
regular defvar. In the second patch, I also converted 
'eshell-number-regexp' to a defvar, and improved the regexp to match 
more valid numbers. I think with those improvements, there's no real 
reason for 'eshell-number-regexp' to be customizable anymore.

Note: I haven't done anything with the range syntax though. If you feel 
strongly that it should be a closed range like in Bash (instead of 
half-open like it is in the current patch), then I don't mind changing 
it. Personally though, I have a soft preference for half-open since it's 
more consistent with the rest of Emacs Lisp.

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

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

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

* 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           |   3 +
 lisp/eshell/esh-var.el            | 136 +++++++++++++++++++-----------
 test/lisp/eshell/esh-var-tests.el | 102 +++++++++++++++++-----
 5 files changed, 192 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 5b8ab06086c..e0175bacfdf 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -149,6 +149,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..8b522449762 100644
--- a/lisp/eshell/esh-util.el
+++ b/lisp/eshell/esh-util.el
@@ -111,6 +111,9 @@ eshell-ange-ls-uids
 
 ;;; Internal Variables:
 
+(defvar eshell-integer-regexp (rx (? "-") (+ digit))
+  "Regular expression used to match integer arguments.")
+
 (defvar eshell-group-names nil
   "A cache to hold the names of groups.")
 
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


[-- Attachment #3: 0002-Make-eshell-number-regexp-into-a-regular-defvar.patch --]
[-- Type: text/plain, Size: 6782 bytes --]

From 924603d09aeb9e8af080c2ff3b1b88aea7225cfc Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Thu, 26 Jan 2023 13:11:15 -0800
Subject: [PATCH 2/2] Make 'eshell-number-regexp' into a regular defvar

This isn't a very useful thing to customize, since it needs to detect
numbers that can successfully be parsed by 'string-to-number'.
Changes to this variable would therefore likely requiring adjusting
'eshell-convert-to-number' as well.

* lisp/eshell/esh-util.el (eshell-number-regexp): Make into a defvar
and improve the regexp to support more numbers (including infinity and
NaN).

* test/lisp/eshell/esh-util-tests.el
(esh-util-test/eshell-convert-to-number/floating-point)
(esh-util-test/eshell-convert-to-number/floating-point-exponent)
(esh-util-test/eshell-convert-to-number/non-numeric)
(esh-util-test/eshell-convert-to-number/no-convert): New tests.
---
 lisp/eshell/esh-util.el            | 20 +++++----
 test/lisp/eshell/esh-util-tests.el | 65 ++++++++++++++++++++++++++++++
 2 files changed, 78 insertions(+), 7 deletions(-)

diff --git a/lisp/eshell/esh-util.el b/lisp/eshell/esh-util.el
index 8b522449762..9549e7f1a10 100644
--- a/lisp/eshell/esh-util.el
+++ b/lisp/eshell/esh-util.el
@@ -94,13 +94,6 @@ eshell-convert-numeric-arguments
 argument matches `eshell-number-regexp'."
   :type 'boolean)
 
-(defcustom eshell-number-regexp "-?\\([0-9]*\\.\\)?[0-9]+\\(e[-0-9.]+\\)?"
-  "Regular expression used to match numeric arguments.
-If `eshell-convert-numeric-arguments' is non-nil, and an argument
-matches this regexp, it will be converted to a Lisp number, using the
-function `string-to-number'."
-  :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"
@@ -111,6 +104,19 @@ eshell-ange-ls-uids
 
 ;;; Internal Variables:
 
+(defvar eshell-number-regexp
+  (rx (? "-")
+      (or (seq (+ digit) (? "." (* digit)))
+          (seq (* digit) "." (+ digit)))
+      ;; Optional exponent
+      (? (or "e" "E")
+         (or "+INF" "+NaN"
+             (seq (? (or "+" "-")) (+ digit)))))
+  "Regular expression used to match numeric arguments.
+If `eshell-convert-numeric-arguments' is non-nil, and an argument
+matches this regexp, it will be converted to a Lisp number, using the
+function `string-to-number'.")
+
 (defvar eshell-integer-regexp (rx (? "-") (+ digit))
   "Regular expression used to match integer arguments.")
 
diff --git a/test/lisp/eshell/esh-util-tests.el b/test/lisp/eshell/esh-util-tests.el
index afaf1b77f2b..ed841e96c7e 100644
--- a/test/lisp/eshell/esh-util-tests.el
+++ b/test/lisp/eshell/esh-util-tests.el
@@ -54,4 +54,69 @@ esh-util-test/eshell-stringify/complex
   "Test that `eshell-stringify' correctly stringifies complex objects."
   (should (equal (eshell-stringify (list 'quote 'hello)) "'hello")))
 
+(ert-deftest esh-util-test/eshell-convert-to-number/integer ()
+  "Test that `eshell-convert-to-number' correctly converts integers."
+  (should (equal (eshell-convert-to-number "123") 123))
+  (should (equal (eshell-convert-to-number "-123") -123))
+  ;; These are technially integers, since Emacs Lisp requires at least
+  ;; one digit after the "." to be a float:
+  (should (equal (eshell-convert-to-number "123.") 123))
+  (should (equal (eshell-convert-to-number "-123.") -123)))
+
+(ert-deftest esh-util-test/eshell-convert-to-number/floating-point ()
+  "Test that `eshell-convert-to-number' correctly converts floats."
+  (should (equal (eshell-convert-to-number "1.23") 1.23))
+  (should (equal (eshell-convert-to-number "-1.23") -1.23))
+  (should (equal (eshell-convert-to-number ".1") 0.1))
+  (should (equal (eshell-convert-to-number "-.1") -0.1)))
+
+(ert-deftest esh-util-test/eshell-convert-to-number/floating-point-exponent ()
+  "Test that `eshell-convert-to-number' correctly converts exponent notation."
+  ;; Positive exponent:
+  (dolist (exp '("e2" "e+2" "E2" "E+2"))
+    (should (equal (eshell-convert-to-number (concat "123" exp)) 12300.0))
+    (should (equal (eshell-convert-to-number (concat "-123" exp)) -12300.0))
+    (should (equal (eshell-convert-to-number (concat "1.23" exp)) 123.0))
+    (should (equal (eshell-convert-to-number (concat "-1.23" exp)) -123.0))
+    (should (equal (eshell-convert-to-number (concat "1." exp)) 100.0))
+    (should (equal (eshell-convert-to-number (concat "-1." exp)) -100.0))
+    (should (equal (eshell-convert-to-number (concat ".1" exp)) 10.0))
+    (should (equal (eshell-convert-to-number (concat "-.1" exp)) -10.0)))
+  ;; Negative exponent:
+  (dolist (exp '("e-2" "E-2"))
+    (should (equal (eshell-convert-to-number (concat "123" exp)) 1.23))
+    (should (equal (eshell-convert-to-number (concat "-123" exp)) -1.23))
+    (should (equal (eshell-convert-to-number (concat "1.23" exp)) 0.0123))
+    (should (equal (eshell-convert-to-number (concat "-1.23" exp)) -0.0123))
+    (should (equal (eshell-convert-to-number (concat "1." exp)) 0.01))
+    (should (equal (eshell-convert-to-number (concat "-1." exp)) -0.01))
+    (should (equal (eshell-convert-to-number (concat ".1" exp)) 0.001))
+    (should (equal (eshell-convert-to-number (concat "-.1" exp)) -0.001))))
+
+(ert-deftest esh-util-test/eshell-convert-to-number/floating-point/infinite ()
+  "Test that `eshell-convert-to-number' correctly converts infinite floats."
+  (should (equal (eshell-convert-to-number "1.0e+INF") 1.0e+INF))
+  (should (equal (eshell-convert-to-number "2.e+INF") 1.0e+INF))
+  (should (equal (eshell-convert-to-number "-1.0e+INF") -1.0e+INF))
+  (should (equal (eshell-convert-to-number "-2.e+INF") -1.0e+INF)))
+
+(ert-deftest esh-util-test/eshell-convert-to-number/floating-point/nan ()
+  "Test that `eshell-convert-to-number' correctly converts NaNs."
+  (should (equal (eshell-convert-to-number "1.0e+NaN") 1.0e+NaN))
+  (should (equal (eshell-convert-to-number "2.e+NaN") 2.0e+NaN))
+  (should (equal (eshell-convert-to-number "-1.0e+NaN") -1.0e+NaN))
+  (should (equal (eshell-convert-to-number "-2.e+NaN") -2.0e+NaN)))
+
+(ert-deftest esh-util-test/eshell-convert-to-number/non-numeric ()
+  "Test that `eshell-convert-to-number' does nothing to non-numeric values."
+  (should (equal (eshell-convert-to-number "foo") "foo"))
+  (should (equal (eshell-convert-to-number "") ""))
+  (should (equal (eshell-convert-to-number "123foo") "123foo")))
+
+(ert-deftest esh-util-test/eshell-convert-to-number/no-convert ()
+  "Test that `eshell-convert-to-number' does nothing when disabled."
+  (let ((eshell-convert-numeric-arguments nil))
+    (should (equal (eshell-convert-to-number "123") "123"))
+    (should (equal (eshell-convert-to-number "1.23") "1.23"))))
+
 ;;; esh-util-tests.el ends here
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* bug#60999: 30.0.50; [PATCH] Add support for negative indices and index ranges in Eshell
  2023-01-27  1:23     ` Jim Porter
@ 2023-01-27  7:38       ` Eli Zaretskii
  2023-01-27 18:02         ` Jim Porter
  0 siblings, 1 reply; 8+ messages in thread
From: Eli Zaretskii @ 2023-01-27  7:38 UTC (permalink / raw)
  To: Jim Porter; +Cc: 60999

> Date: Thu, 26 Jan 2023 17:23:19 -0800
> From: Jim Porter <jporterbugs@gmail.com>
> Cc: 60999@debbugs.gnu.org
> 
> On 1/22/2023 12:13 AM, Jim Porter wrote:
> > Either way we decide about 'eshell-number-regexp', I can turn 
> > 'eshell-integer-regexp' into a regular defvar. (The only thing I can 
> > think of that a person would customize it to would be to allow a "+" at 
> > the start of an integer, like "+123".)
> 
> Ok, I've updated my patch to that 'eshell-integer-regexp' is just a 
> regular defvar. In the second patch, I also converted 
> 'eshell-number-regexp' to a defvar, and improved the regexp to match 
> more valid numbers. I think with those improvements, there's no real 
> reason for 'eshell-number-regexp' to be customizable anymore.

Thanks.

> Note: I haven't done anything with the range syntax though. If you feel 
> strongly that it should be a closed range like in Bash (instead of 
> half-open like it is in the current patch), then I don't mind changing 
> it. Personally though, I have a soft preference for half-open since it's 
> more consistent with the rest of Emacs Lisp.

I have no strong feelings, as I don't expect to be using this feature.
I just think this could be confusing to people who do use it in other
shells, and a potential source of complaints and bug reports.  But it
is your call.





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#60999: 30.0.50; [PATCH] Add support for negative indices and index ranges in Eshell
  2023-01-27  7:38       ` Eli Zaretskii
@ 2023-01-27 18:02         ` Jim Porter
  2023-01-27 18:57           ` Eli Zaretskii
  0 siblings, 1 reply; 8+ messages in thread
From: Jim Porter @ 2023-01-27 18:02 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 60999

On 1/26/2023 11:38 PM, Eli Zaretskii wrote:
> I have no strong feelings, as I don't expect to be using this feature.
> I just think this could be confusing to people who do use it in other
> shells, and a potential source of complaints and bug reports.  But it
> is your call.

Ok, thanks. Would it make sense to merge this patch as-is then, and 
afterwards I can post a message on emacs-devel soliciting feedback? If 
people clearly prefer the Bash semantics, I don't have any major issues 
with changing the code to match that.





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#60999: 30.0.50; [PATCH] Add support for negative indices and index ranges in Eshell
  2023-01-27 18:02         ` Jim Porter
@ 2023-01-27 18:57           ` Eli Zaretskii
  2023-01-28  2:15             ` Jim Porter
  0 siblings, 1 reply; 8+ messages in thread
From: Eli Zaretskii @ 2023-01-27 18:57 UTC (permalink / raw)
  To: Jim Porter; +Cc: 60999

> Date: Fri, 27 Jan 2023 10:02:30 -0800
> Cc: 60999@debbugs.gnu.org
> From: Jim Porter <jporterbugs@gmail.com>
> 
> On 1/26/2023 11:38 PM, Eli Zaretskii wrote:
> > I have no strong feelings, as I don't expect to be using this feature.
> > I just think this could be confusing to people who do use it in other
> > shells, and a potential source of complaints and bug reports.  But it
> > is your call.
> 
> Ok, thanks. Would it make sense to merge this patch as-is then, and 
> afterwards I can post a message on emacs-devel soliciting feedback?

Feel free.





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#60999: 30.0.50; [PATCH] Add support for negative indices and index ranges in Eshell
  2023-01-27 18:57           ` Eli Zaretskii
@ 2023-01-28  2:15             ` Jim Porter
  0 siblings, 0 replies; 8+ messages in thread
From: Jim Porter @ 2023-01-28  2:15 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 60999-done

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.





^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2023-01-28  2:15 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-22  3:47 bug#60999: 30.0.50; [PATCH] Add support for negative indices and index ranges in Eshell Jim Porter
2023-01-22  6:29 ` Eli Zaretskii
2023-01-22  8:13   ` Jim Porter
2023-01-27  1:23     ` Jim Porter
2023-01-27  7:38       ` Eli Zaretskii
2023-01-27 18:02         ` Jim Porter
2023-01-27 18:57           ` Eli Zaretskii
2023-01-28  2:15             ` Jim Porter

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.