unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#68863: Add support for using setf with seq-subseq
@ 2024-02-01  3:31 Okamsn via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-02-04 18:33 ` bug#68863: [PATCH] " Okamsn via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-02-08 11:39 ` bug#68863: " Eli Zaretskii
  0 siblings, 2 replies; 9+ messages in thread
From: Okamsn via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-02-01  3:31 UTC (permalink / raw)
  To: 68863

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

Hello,

This patch adds support for using `seq-subseq` with `setf`, as in

     ;; => [0 1 2 10 11]
     (let ((seq (vector 0 1 2 3 4)))
       (setf (seq-subseq seq -2) (list 10 11 12 13 14))
       seq)

The patch adds a generic version which uses the existing `setf` support 
of `seq-elt` and a specialized version for modifying lists.  Both 
versions use `seq-do` to map a function over the values that should 
replace the values in the modified sequence.

To avoid modifying more values than specified, that modifying function 
uses a `when` condition. I'm not sure of a good way to stop `seq-do` 
early when we know that it can stop calling the modifying function. 
Normally, I would use `cl-block` and `cl-return`. Is it OK to use those 
features in `seq.el`? If not, is it worth adding something like a 
`seq-map-while` or a `seq-do-while`?

Thank you.

[-- Attachment #2: 0001-Add-setf-support-for-seq-subseq.patch --]
[-- Type: text/x-patch, Size: 5338 bytes --]

From 0a5fac443cdcbeb9312d7ee68bafdd22e0905828 Mon Sep 17 00:00:00 2001
From: Earl Hyatt <okamsn@protonmail.com>
Date: Sun, 28 Jan 2024 22:48:13 -0500
Subject: [PATCH] Add setf support for seq-subseq

* lisp/emacs-lisp/seq.el (seq-subseq): Add a generic version of
calling setf on seq-subseq and add a specialized version for when the
modified sequence is a list.
* test/lisp/emacs-lisp/seq-tests.el (test-setf-seq-subseq)
(test-setf-seq-subseq-combinations): Add tests for the feature.
---
 lisp/emacs-lisp/seq.el            | 43 +++++++++++++++++
 test/lisp/emacs-lisp/seq-tests.el | 76 +++++++++++++++++++++++++++++++
 2 files changed, 119 insertions(+)

diff --git a/lisp/emacs-lisp/seq.el b/lisp/emacs-lisp/seq.el
index 4c6553972c2..fd971806d87 100644
--- a/lisp/emacs-lisp/seq.el
+++ b/lisp/emacs-lisp/seq.el
@@ -193,6 +193,49 @@ seq-subseq
         (copy-sequence sequence))))
    (t (error "Unsupported sequence: %s" sequence))))
 
+(cl-defgeneric (setf seq-subseq) (store sequence start &optional end)
+  "Modify the elements of SEQUENCE from START to END to be those of STORE.
+
+SEQUENCE is neither lengthened nor shortened."
+  (let* ((len (seq-length sequence))
+         (idx (if (< start 0)
+                  (+ start len)
+                start))
+         (end (cond
+               ((null end) len)
+               ((< end 0)
+                (+ end len))
+               (t (min len end)))))
+    (when (< idx end)
+      (seq-do (lambda (v)
+                (when (< idx end)
+                  (setf (seq-elt sequence idx) v
+                        idx (1+ idx))))
+              store)))
+  store)
+
+(cl-defmethod (setf seq-subseq) (store (sequence list) start &optional end)
+  "Modify the elements of SEQUENCE from START to END to be those of STORE.
+
+SEQUENCE is neither lengthened nor shortened."
+  (let* ((len (seq-length sequence))
+         (idx (if (< start 0)
+                  (+ start len)
+                start))
+         (end (cond
+               ((null end) len)
+               ((< end 0)  (+ end len))
+               (t          (min len end)))))
+    (when (< idx end)
+      (seq-do (let ((replaced (nthcdr idx sequence)))
+                (lambda (v)
+                  (when (< idx end)
+                    (setf (car replaced) v
+                          replaced (cdr replaced)
+                          idx (1+ idx)))))
+              store)))
+  store)
+
 \f
 (cl-defgeneric seq-map (function sequence)
   "Return the result of applying FUNCTION to each element of SEQUENCE."
diff --git a/test/lisp/emacs-lisp/seq-tests.el b/test/lisp/emacs-lisp/seq-tests.el
index c06ceb00bdb..6b8789688d3 100644
--- a/test/lisp/emacs-lisp/seq-tests.el
+++ b/test/lisp/emacs-lisp/seq-tests.el
@@ -312,6 +312,82 @@ test-seq-subseq
                   (:success
                    (should (equal (seq-subseq list start end) res))))))))))))
 
+(cl-defmacro test-setf-seq-subseq-combinations (&key result range init-vals
+                                                     sub-vals)
+  "Produce substitutions tests for `seq-subseq' using `setf'.
+
+- INIT-VALS is a list holding the initial elements.
+- RESULT is what the final value should be after substitution.
+- SUB-VALS is a list holding the elements to be substituted in.
+- RANGE is a list of the `start' and `end' arguments of `seq-subseq'."
+  (let ((tests))
+    (dolist (type1 '(list vector string))
+      (dolist (type2 '(list vector string))
+        (push  `(should (equal (,type1 ,@result)
+                               (let ((seq (,type1 ,@init-vals)))
+                                 (setf (seq-subseq seq ,@range)
+                                       (,type2 ,@sub-vals))
+                                 seq)))
+               tests)))
+    `(progn ,@tests)))
+
+(ert-deftest test-setf-seq-subseq ()
+  "Test using `seq-subseq' with `setf'.
+Any combination of sequences should work."
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2)
+   :sub-vals (10 11 12)
+   :range (0)
+   :result (10 11 12))
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1)
+   :sub-vals (10 11 12)
+   :range (0)
+   :result (10 11))
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1)
+   :sub-vals (10 11 12)
+   :range (0 100)
+   :result (10 11))
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2 3 4)
+   :sub-vals (12 13 14 15)
+   :range (2 100)
+   :result (0 1 12 13 14))
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2 3 4)
+   :sub-vals (12 13 14 15)
+   :range (2 3)
+   :result (0 1 12 3 4))
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2 3 4)
+   :sub-vals (12 13 14 15)
+   :range (2 2)
+   :result (0 1 2 3 4))
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2 3 4 5 6 7 8 9)
+   :sub-vals (10 11 12 13 14)
+   :range (-2)
+   :result (0 1 2 10 11))
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2 3 4 5 6 7 8 9)
+   :sub-vals (10 11 12 13 14)
+   :range (-6 -3)
+   :result (0 1 2 3 10 11 12 7 8 9))
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2 3 4 5 6 7 8 9)
+   :sub-vals (10 11 12 13 14)
+   :range (-6 -10)
+   :result (0 1 2 3 4 5 6 7 8 9)))
+
 (ert-deftest test-seq-concatenate ()
   (with-test-sequences (seq '(2 4 6))
     (should (equal (seq-concatenate 'string seq [8]) (string 2 4 6 8)))
-- 
2.34.1


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

* bug#68863: [PATCH] Add support for using setf with seq-subseq
  2024-02-01  3:31 bug#68863: Add support for using setf with seq-subseq Okamsn via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-02-04 18:33 ` Okamsn via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-02-08 11:39 ` bug#68863: " Eli Zaretskii
  1 sibling, 0 replies; 9+ messages in thread
From: Okamsn via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-02-04 18:33 UTC (permalink / raw)
  To: 68863; +Cc: Nicolas Petton

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

Hello,

I've added the maintainers e-mail address to the discussion and have 
added "[PATCH]" to the subject to make it more clear that there is a 
patch file attached.

I tested a version that uses a `seq-map-while`, but it only made a 
difference when the sequence of values to copy to the existing sequence 
was longer than 100 elements, which I would guess is a less common use case.

Thank you,
Earl

[-- Attachment #2: 0001-Add-setf-support-for-seq-subseq.patch --]
[-- Type: text/x-patch, Size: 5338 bytes --]

From 0a5fac443cdcbeb9312d7ee68bafdd22e0905828 Mon Sep 17 00:00:00 2001
From: Earl Hyatt <okamsn@protonmail.com>
Date: Sun, 28 Jan 2024 22:48:13 -0500
Subject: [PATCH] Add setf support for seq-subseq

* lisp/emacs-lisp/seq.el (seq-subseq): Add a generic version of
calling setf on seq-subseq and add a specialized version for when the
modified sequence is a list.
* test/lisp/emacs-lisp/seq-tests.el (test-setf-seq-subseq)
(test-setf-seq-subseq-combinations): Add tests for the feature.
---
 lisp/emacs-lisp/seq.el            | 43 +++++++++++++++++
 test/lisp/emacs-lisp/seq-tests.el | 76 +++++++++++++++++++++++++++++++
 2 files changed, 119 insertions(+)

diff --git a/lisp/emacs-lisp/seq.el b/lisp/emacs-lisp/seq.el
index 4c6553972c2..fd971806d87 100644
--- a/lisp/emacs-lisp/seq.el
+++ b/lisp/emacs-lisp/seq.el
@@ -193,6 +193,49 @@ seq-subseq
         (copy-sequence sequence))))
    (t (error "Unsupported sequence: %s" sequence))))
 
+(cl-defgeneric (setf seq-subseq) (store sequence start &optional end)
+  "Modify the elements of SEQUENCE from START to END to be those of STORE.
+
+SEQUENCE is neither lengthened nor shortened."
+  (let* ((len (seq-length sequence))
+         (idx (if (< start 0)
+                  (+ start len)
+                start))
+         (end (cond
+               ((null end) len)
+               ((< end 0)
+                (+ end len))
+               (t (min len end)))))
+    (when (< idx end)
+      (seq-do (lambda (v)
+                (when (< idx end)
+                  (setf (seq-elt sequence idx) v
+                        idx (1+ idx))))
+              store)))
+  store)
+
+(cl-defmethod (setf seq-subseq) (store (sequence list) start &optional end)
+  "Modify the elements of SEQUENCE from START to END to be those of STORE.
+
+SEQUENCE is neither lengthened nor shortened."
+  (let* ((len (seq-length sequence))
+         (idx (if (< start 0)
+                  (+ start len)
+                start))
+         (end (cond
+               ((null end) len)
+               ((< end 0)  (+ end len))
+               (t          (min len end)))))
+    (when (< idx end)
+      (seq-do (let ((replaced (nthcdr idx sequence)))
+                (lambda (v)
+                  (when (< idx end)
+                    (setf (car replaced) v
+                          replaced (cdr replaced)
+                          idx (1+ idx)))))
+              store)))
+  store)
+
 \f
 (cl-defgeneric seq-map (function sequence)
   "Return the result of applying FUNCTION to each element of SEQUENCE."
diff --git a/test/lisp/emacs-lisp/seq-tests.el b/test/lisp/emacs-lisp/seq-tests.el
index c06ceb00bdb..6b8789688d3 100644
--- a/test/lisp/emacs-lisp/seq-tests.el
+++ b/test/lisp/emacs-lisp/seq-tests.el
@@ -312,6 +312,82 @@ test-seq-subseq
                   (:success
                    (should (equal (seq-subseq list start end) res))))))))))))
 
+(cl-defmacro test-setf-seq-subseq-combinations (&key result range init-vals
+                                                     sub-vals)
+  "Produce substitutions tests for `seq-subseq' using `setf'.
+
+- INIT-VALS is a list holding the initial elements.
+- RESULT is what the final value should be after substitution.
+- SUB-VALS is a list holding the elements to be substituted in.
+- RANGE is a list of the `start' and `end' arguments of `seq-subseq'."
+  (let ((tests))
+    (dolist (type1 '(list vector string))
+      (dolist (type2 '(list vector string))
+        (push  `(should (equal (,type1 ,@result)
+                               (let ((seq (,type1 ,@init-vals)))
+                                 (setf (seq-subseq seq ,@range)
+                                       (,type2 ,@sub-vals))
+                                 seq)))
+               tests)))
+    `(progn ,@tests)))
+
+(ert-deftest test-setf-seq-subseq ()
+  "Test using `seq-subseq' with `setf'.
+Any combination of sequences should work."
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2)
+   :sub-vals (10 11 12)
+   :range (0)
+   :result (10 11 12))
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1)
+   :sub-vals (10 11 12)
+   :range (0)
+   :result (10 11))
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1)
+   :sub-vals (10 11 12)
+   :range (0 100)
+   :result (10 11))
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2 3 4)
+   :sub-vals (12 13 14 15)
+   :range (2 100)
+   :result (0 1 12 13 14))
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2 3 4)
+   :sub-vals (12 13 14 15)
+   :range (2 3)
+   :result (0 1 12 3 4))
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2 3 4)
+   :sub-vals (12 13 14 15)
+   :range (2 2)
+   :result (0 1 2 3 4))
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2 3 4 5 6 7 8 9)
+   :sub-vals (10 11 12 13 14)
+   :range (-2)
+   :result (0 1 2 10 11))
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2 3 4 5 6 7 8 9)
+   :sub-vals (10 11 12 13 14)
+   :range (-6 -3)
+   :result (0 1 2 3 10 11 12 7 8 9))
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2 3 4 5 6 7 8 9)
+   :sub-vals (10 11 12 13 14)
+   :range (-6 -10)
+   :result (0 1 2 3 4 5 6 7 8 9)))
+
 (ert-deftest test-seq-concatenate ()
   (with-test-sequences (seq '(2 4 6))
     (should (equal (seq-concatenate 'string seq [8]) (string 2 4 6 8)))
-- 
2.34.1


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

* bug#68863: Add support for using setf with seq-subseq
  2024-02-01  3:31 bug#68863: Add support for using setf with seq-subseq Okamsn via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-02-04 18:33 ` bug#68863: [PATCH] " Okamsn via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-02-08 11:39 ` Eli Zaretskii
  2024-02-08 14:25   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 1 reply; 9+ messages in thread
From: Eli Zaretskii @ 2024-02-08 11:39 UTC (permalink / raw)
  To: Okamsn, Nicolas Petton, Stefan Monnier; +Cc: 68863

> Date: Thu, 01 Feb 2024 03:31:10 +0000
> From:  Okamsn via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> This patch adds support for using `seq-subseq` with `setf`, as in
> 
>      ;; => [0 1 2 10 11]
>      (let ((seq (vector 0 1 2 3 4)))
>        (setf (seq-subseq seq -2) (list 10 11 12 13 14))
>        seq)
> 
> The patch adds a generic version which uses the existing `setf` support 
> of `seq-elt` and a specialized version for modifying lists.  Both 
> versions use `seq-do` to map a function over the values that should 
> replace the values in the modified sequence.
> 
> To avoid modifying more values than specified, that modifying function 
> uses a `when` condition. I'm not sure of a good way to stop `seq-do` 
> early when we know that it can stop calling the modifying function. 
> Normally, I would use `cl-block` and `cl-return`. Is it OK to use those 
> features in `seq.el`? If not, is it worth adding something like a 
> `seq-map-while` or a `seq-do-while`?

Thanks.

Nicolas, Stefan: any comments?  seq.el is preloaded, so we should
consider whether this addition is important enough to have it in seq
or elsewhere, if we think it's useful.





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

* bug#68863: Add support for using setf with seq-subseq
  2024-02-08 11:39 ` bug#68863: " Eli Zaretskii
@ 2024-02-08 14:25   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-02-09  3:54     ` Okamsn via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 9+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-02-08 14:25 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Okamsn, 68863, Nicolas Petton

>> This patch adds support for using `seq-subseq` with `setf`, as in
>> 
>>      ;; => [0 1 2 10 11]
>>      (let ((seq (vector 0 1 2 3 4)))
>>        (setf (seq-subseq seq -2) (list 10 11 12 13 14))
>>        seq)
>> 
>> The patch adds a generic version which uses the existing `setf` support 
>> of `seq-elt` and a specialized version for modifying lists.  Both 
>> versions use `seq-do` to map a function over the values that should 
>> replace the values in the modified sequence.
> Nicolas, Stefan: any comments?

Fine by me.

>> To avoid modifying more values than specified, that modifying function 
>> uses a `when` condition. I'm not sure of a good way to stop `seq-do` 
>> early when we know that it can stop calling the modifying function. 
>> Normally, I would use `cl-block` and `cl-return`. Is it OK to use those 
>> features in `seq.el`? If not, is it worth adding something like a 
>> `seq-map-while` or a `seq-do-while`?

`seq.el` is used by some parts of the implementation of `cl-lib`, so
the use of `cl-lib` risks introducing a circular dependency.  Maybe using
`cl-block/return` would be OK, but I wouldn't be surprised if it causes
bootstrap trouble.  You can use catch/throw, OTOH.


        Stefan






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

* bug#68863: Add support for using setf with seq-subseq
  2024-02-08 14:25   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-02-09  3:54     ` Okamsn via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-02-14  2:50       ` Okamsn via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 9+ messages in thread
From: Okamsn via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-02-09  3:54 UTC (permalink / raw)
  To: Stefan Monnier, Eli Zaretskii; +Cc: 68863, Nicolas Petton

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

Stefan Monnier wrote:
>>> To avoid modifying more values than specified, that modifying function
>>> uses a `when` condition. I'm not sure of a good way to stop `seq-do`
>>> early when we know that it can stop calling the modifying function.
>>> Normally, I would use `cl-block` and `cl-return`. Is it OK to use those
>>> features in `seq.el`? If not, is it worth adding something like a
>>> `seq-map-while` or a `seq-do-while`?
> 
> `seq.el` is used by some parts of the implementation of `cl-lib`, so
> the use of `cl-lib` risks introducing a circular dependency.  Maybe using
> `cl-block/return` would be OK, but I wouldn't be surprised if it causes
> bootstrap trouble.  You can use catch/throw, OTOH.
> 
> 
>          Stefan
> 

Attached is an updated version using `catch` and `throw`. Thank you for 
pointing those out to me. The patch is also changed to signal 
`args-out-of-range` for the start and end indexes to be more like 
`seq-subseq`.

How does it look?

[-- Attachment #2: v2-0001-Add-setf-support-for-seq-subseq.patch --]
[-- Type: text/x-patch, Size: 10101 bytes --]

From b06db7905f5d6dfa0d33c05fd214ef95d19814b5 Mon Sep 17 00:00:00 2001
From: Earl Hyatt <okamsn@protonmail.com>
Date: Sun, 28 Jan 2024 22:48:13 -0500
Subject: [PATCH v2] Add setf support for seq-subseq.

* lisp/emacs-lisp/seq.el (seq-subseq): Add a generic version of
calling setf on seq-subseq and add a specialized version for when the
modified sequence is a list.
* test/lisp/emacs-lisp/seq-tests.el (test-setf-seq-subseq)
(test-setf-seq-subseq-combinations): Add tests for the feature.

The feature will signal 'args-out-of-range' if the starting index or
ending index (if given) is outside of the range of values from 0
through the length of the sequence or from the negative length of the
sequence through negative 1.  If the starting index is equal to the
length of the sequence, then nothing is changed.  If the starting
index is equal to the ending index, then nothing is changed.  The
feature should signal an error in all cases where using 'seq-subseq'
would signal an error.
---
 lisp/emacs-lisp/seq.el            |  88 ++++++++++++++++++
 test/lisp/emacs-lisp/seq-tests.el | 149 ++++++++++++++++++++++++++++++
 2 files changed, 237 insertions(+)

diff --git a/lisp/emacs-lisp/seq.el b/lisp/emacs-lisp/seq.el
index 4c6553972c2..6a1fd4c35e3 100644
--- a/lisp/emacs-lisp/seq.el
+++ b/lisp/emacs-lisp/seq.el
@@ -193,6 +193,94 @@ seq-subseq
         (copy-sequence sequence))))
    (t (error "Unsupported sequence: %s" sequence))))
 
+(cl-defgeneric (setf seq-subseq) (store sequence start &optional end)
+  "Modify the elements of SEQUENCE from START to END to be those of STORE.
+END is exclusive.
+
+If END is omitted, it defaults to the length of the sequence.  If
+START or END is negative, it counts from the end.  Signal an
+error if START or END are outside of the sequence (i.e too large
+if positive or too small if negative).
+
+SEQUENCE is neither lengthened nor shortened."
+  (let* ((len (seq-length sequence))
+         (signal-fn (lambda ()
+                      (signal 'args-out-of-range
+                              (if end
+                                  (list sequence start end)
+                                (list sequence start)))))
+         (signal-or-val-fn (lambda (val)
+                             (cond
+                              ((> val len)
+                               (funcall signal-fn))
+                              ((< val 0)
+                               (let ((val2 (+ val len)))
+                                 (if (< val2 0)
+                                     (funcall signal-fn)
+                                   val2)))
+                              (t
+                               val))))
+         (idx (funcall signal-or-val-fn start))
+         (idx-end (if (null end)
+                      len
+                    (funcall signal-or-val-fn end)))
+         (tag (gensym)))
+    (if (> idx idx-end)
+        (funcall signal-fn)
+      (catch tag
+        (seq-do (lambda (v)
+                  (if (< idx idx-end)
+                      (setf (seq-elt sequence idx) v
+                            idx (1+ idx))
+                    (throw tag nil)))
+                store))))
+  store)
+
+(cl-defmethod (setf seq-subseq) (store (sequence list) start &optional end)
+  "Modify the elements of SEQUENCE from START to END to be those of STORE.
+END is exclusive.
+
+If END is omitted, it defaults to the length of the sequence.  If
+START or END is negative, it counts from the end.  Signal an
+error if START or END are outside of the sequence (i.e too large
+if positive or too small if negative).
+
+SEQUENCE is neither lengthened nor shortened."
+  (let* ((len (seq-length sequence))
+         (signal-fn (lambda ()
+                      (signal 'args-out-of-range
+                              (if end
+                                  (list sequence start end)
+                                (list sequence start)))))
+         (signal-or-val-fn (lambda (val)
+                             (cond
+                              ((> val len)
+                               (funcall signal-fn))
+                              ((< val 0)
+                               (let ((val2 (+ val len)))
+                                 (if (< val2 0)
+                                     (funcall signal-fn)
+                                   val2)))
+                              (t
+                               val))))
+         (idx (funcall signal-or-val-fn start))
+         (idx-end (if (null end)
+                      len
+                    (funcall signal-or-val-fn end)))
+         (tag (gensym)))
+    (if (> idx idx-end)
+        (funcall signal-fn)
+      (catch tag
+        (seq-do (let ((replaced (nthcdr idx sequence)))
+                  (lambda (v)
+                    (if (< idx idx-end)
+                        (setf (car replaced) v
+                              replaced (cdr replaced)
+                              idx (1+ idx))
+                      (throw tag nil))))
+                store))))
+  store)
+
 \f
 (cl-defgeneric seq-map (function sequence)
   "Return the result of applying FUNCTION to each element of SEQUENCE."
diff --git a/test/lisp/emacs-lisp/seq-tests.el b/test/lisp/emacs-lisp/seq-tests.el
index c06ceb00bdb..d3e46c32f99 100644
--- a/test/lisp/emacs-lisp/seq-tests.el
+++ b/test/lisp/emacs-lisp/seq-tests.el
@@ -312,6 +312,155 @@ test-seq-subseq
                   (:success
                    (should (equal (seq-subseq list start end) res))))))))))))
 
+(cl-defmacro test-setf-seq-subseq-combinations
+    (&key init-vals sub-vals result range error)
+  "Make a test for each combination of sequence type for `seq-subseq' using `setf'.
+
+- INIT-VALS is a list holding the initial elements.
+- RESULT is what the final value should be after substitution.
+- ERROR is whether the form should signal `args-out-of-range'.
+- SUB-VALS is a list holding the elements to be substituted in.
+- RANGE is a list of the `start' and `end' arguments of `seq-subseq'."
+  (let ((tests))
+    (dolist (type1 '(list vector string))
+      (dolist (type2 '(list vector string))
+        (push  (if error
+                   `(should-error (let ((seq (,type1 ,@init-vals)))
+                                    (setf (seq-subseq seq ,@range)
+                                          (,type2 ,@sub-vals))
+                                    seq)
+                                  :type (quote args-out-of-range))
+                 `(should (equal (,type1 ,@result)
+                                 (let ((seq (,type1 ,@init-vals)))
+                                   (setf (seq-subseq seq ,@range)
+                                         (,type2 ,@sub-vals))
+                                   seq))))
+               tests)))
+    `(progn ,@tests)))
+
+(ert-deftest test-setf-seq-subseq ()
+  "Test using `seq-subseq' with `setf'.
+
+Any combination of sequences should work.
+
+An error should be signalled if the inclusive starting index or
+the exclusive ending index is out of the range from 0 through the
+length of the sequence, or if the starting index is greater than
+the ending index.  If the starting index is equal to the ending
+index, then nothing is changed.  If the starting index is equal
+to the length of the sequence, then nothing is changed.  It
+should signal an error in all the cases that `seq-subseq' signals
+an error."
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2)
+   :sub-vals (10 11 12)
+   :range (0)
+   :result (10 11 12))
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1)
+   :sub-vals (10 11 12)
+   :range (0)
+   :result (10 11))
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2)
+   :sub-vals (10 11 12)
+   :range (1)
+   :result (0 10 11))
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2)
+   :sub-vals (10 11 12)
+   :range (1 3)
+   :result (0 10 11))
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2)
+   :sub-vals (10 11 12)
+   :range (3 1)
+   :error t)
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2)
+   :sub-vals (10 11 12)
+   :range (1 100)
+   :error t)
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2)
+   :sub-vals (10 11 12)
+   :range (7)
+   :error t)
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2 3 4)
+   :sub-vals (12 13 14 15)
+   :range (2 3)
+   :result (0 1 12 3 4))
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2 3 4)
+   :sub-vals (12 13 14 15)
+   :range (2 2)
+   :result (0 1 2 3 4))
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2 3 4)
+   :sub-vals (12 13 14 15)
+   :range (5)
+   :result (0 1 2 3 4))
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2 3 4)
+   :sub-vals (12 13 14 15)
+   :range (6)
+   :error t)
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2 3 4)
+   :sub-vals (12 13 14 15)
+   :range (5 6)
+   :error t)
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2 3 4 5 6 7 8 9)
+   :sub-vals (10 11 12 13 14)
+   :range (-2)
+   :result (0 1 2 3 4 5 6 7 10 11))
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2 3 4 5 6 7 8 9)
+   :sub-vals (10 11 12 13 14)
+   :range (-6 -3)
+   :result (0 1 2 3 10 11 12 7 8 9))
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2 3 4 5 6 7 8 9)
+   :sub-vals (10 11 12 13 14)
+   :range (-6 -10)
+   :error t)
+
+  ;; This range might make sense, but since it would signal an error
+  ;; in `seq-subseq', we also signal an error in the `setf' feature.
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2 3 4 5 6 7 8 9)
+   :sub-vals (10 11 12 13 14)
+   :range (-6 0)
+   :error t)
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2 3 4 5 6 7 8 9)
+   :sub-vals (10 11 12 13 14)
+   :range (100)
+   :error t)
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2 3 4 5 6 7 8 9)
+   :sub-vals (10 11 12 13 14)
+   :range (-100)
+   :error t))
+
 (ert-deftest test-seq-concatenate ()
   (with-test-sequences (seq '(2 4 6))
     (should (equal (seq-concatenate 'string seq [8]) (string 2 4 6 8)))
-- 
2.34.1


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

* bug#68863: Add support for using setf with seq-subseq
  2024-02-09  3:54     ` Okamsn via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-02-14  2:50       ` Okamsn via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-04-18  2:54         ` Okamsn via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 9+ messages in thread
From: Okamsn via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-02-14  2:50 UTC (permalink / raw)
  To: Stefan Monnier, Eli Zaretskii; +Cc: 68863, Nicolas Petton

Okamsn wrote:
> Stefan Monnier wrote:
>>>> To avoid modifying more values than specified, that modifying function
>>>> uses a `when` condition. I'm not sure of a good way to stop `seq-do`
>>>> early when we know that it can stop calling the modifying function.
>>>> Normally, I would use `cl-block` and `cl-return`. Is it OK to use those
>>>> features in `seq.el`? If not, is it worth adding something like a
>>>> `seq-map-while` or a `seq-do-while`?
>>
>> `seq.el` is used by some parts of the implementation of `cl-lib`, so
>> the use of `cl-lib` risks introducing a circular dependency.  Maybe using
>> `cl-block/return` would be OK, but I wouldn't be surprised if it causes
>> bootstrap trouble.  You can use catch/throw, OTOH.
>>
>>
>>           Stefan
>>
> 
> Attached is an updated version using `catch` and `throw`. Thank you for
> pointing those out to me. The patch is also changed to signal
> `args-out-of-range` for the start and end indexes to be more like
> `seq-subseq`.
> 
> How does it look?

Hello,

After testing it more, I see that what I've written does not work as I 
expected in the case

(let ((v    (vector (vector 0 1)
                     (vector 2 3)
                     (vector 4 5))))
   (setf (seq-subseq (seq-subseq (elt v 0) 0) 0)
         [10])
   v)

in which I would expect it to replace the first element of the first 
sub-vector with 10. I will take more time to continue working on this.

Thank you for your patience.








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

* bug#68863: Add support for using setf with seq-subseq
  2024-02-14  2:50       ` Okamsn via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-04-18  2:54         ` Okamsn via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-05-07  1:45           ` Okamsn via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 9+ messages in thread
From: Okamsn via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-04-18  2:54 UTC (permalink / raw)
  To: Stefan Monnier, Eli Zaretskii; +Cc: 68863, Nicolas Petton

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

Okamsn wrote:
> Hello,
> 
> After testing it more, I see that what I've written does not work as I
> expected in the case
> 
> (let ((v    (vector (vector 0 1)
>                       (vector 2 3)
>                       (vector 4 5))))
>     (setf (seq-subseq (seq-subseq (elt v 0) 0) 0)
>           [10])
>     v)
> 
> in which I would expect it to replace the first element of the first
> sub-vector with 10. I will take more time to continue working on this.
> 
> Thank you for your patience.
> 
> 

Hello,

I found a way to work with subplaces, like in the example in my previous 
e-mail message. Instead of creating the generic feature `(setf 
seq-subseq)` like what is done for `seq-elt`, I created a generic 
function `seq-replace`, which is used in a new `gv-expander` for 
`seq-subseq`. This way of doing it is like what is done for `substring`, 
which has the behavior that I wanted.

What do you think about this approach?

Thank you.

[-- Attachment #2: v3-0001-Add-seq-replace-and-setf-support-for-seq-subseq.patch --]
[-- Type: text/x-patch, Size: 16721 bytes --]

From 414c7689ef8735e4d2955e0f97b5ce842120883e Mon Sep 17 00:00:00 2001
From: Earl Hyatt <okamsn@protonmail.com>
Date: Sun, 28 Jan 2024 22:48:13 -0500
Subject: [PATCH v3] Add seq-replace and setf support for seq-subseq.

* lisp/emacs-lisp/seq.el (seq-replace): Add function for
non-destructively replacing the elements of sequence
with those from another sequence.
* lisp/emacs-lisp/seq.el (seq-subseq): Declare the 'gv-expander'
specification using the new 'seq-replace' functions.
* test/lisp/emacs-lisp/seq-tests.el (test-seq-replace)
(test-seq-replace-combinations): Add tests for 'seq-replace'.
* test/lisp/emacs-lisp/seq-tests.el (test-setf-seq-subseq)
(test-setf-seq-subseq-combinations, test-setf-seq-subseq-recursive): Add
tests for the new gv expander.

The feature will signal 'args-out-of-range' if the starting index or
ending index (if given) is outside of the range of values from 0 through
the length of the sequence or from the negative length of the sequence
through negative 1.  If the starting index is equal to the length of the
sequence, then nothing is changed.  If the starting index is equal to
the ending index, then nothing is changed.  The 'seq-replace' and the
new 'setf' support for 'seq-subseq' should signal an error in all cases
where using 'seq-subseq' would signal an error.
---
 lisp/emacs-lisp/seq.el            |  91 +++++++++
 test/lisp/emacs-lisp/seq-tests.el | 324 ++++++++++++++++++++++++++++++
 2 files changed, 415 insertions(+)

diff --git a/lisp/emacs-lisp/seq.el b/lisp/emacs-lisp/seq.el
index a20cff16982..37f73932cd7 100644
--- a/lisp/emacs-lisp/seq.el
+++ b/lisp/emacs-lisp/seq.el
@@ -158,6 +158,84 @@ seq-copy
   "Return a shallow copy of SEQUENCE."
   (copy-sequence sequence))
 
+(cl-defgeneric seq-replace (sequence replacements start &optional end)
+  "Replace elements of SEQUENCE from START to END with elements of REPLACEMENTS.
+END is exclusive."
+  (let* ((len (seq-length sequence))
+         (signal-fn (lambda ()
+                      (signal 'args-out-of-range
+                              (if end
+                                  (list sequence start end)
+                                (list sequence start)))))
+         (signal-or-val-fn (lambda (val)
+                             (cond
+                              ((> val len)
+                               (funcall signal-fn))
+                              ((< val 0)
+                               (let ((val2 (+ val len)))
+                                 (if (< val2 0)
+                                     (funcall signal-fn)
+                                   val2)))
+                              (t
+                               val))))
+         (idx-start (funcall signal-or-val-fn start))
+         (idx-end (if (null end)
+                      len
+                    (funcall signal-or-val-fn end))))
+    (if (> idx-start idx-end)
+        (funcall signal-fn)
+      (let ((replacement-idx 0)
+            (replacement-len (seq-length replacements)))
+        (seq-into (seq-map-indexed (lambda (elem idx)
+                                     (if (and (<= idx-start idx)
+                                              (< idx idx-end)
+                                              (< replacement-idx replacement-len))
+                                         (prog1
+                                             (seq-elt replacements replacement-idx)
+                                           (setq replacement-idx (1+ replacement-idx)))
+                                       elem))
+                                   sequence)
+                  (if (listp sequence)
+                      'list
+                    (type-of sequence)))))))
+
+(cl-defmethod seq-replace (sequence (replacements list) start &optional end)
+  "Replace elements of SEQUENCE from START to END with elements of REPLACEMENTS.
+END is exclusive."
+  (let* ((len (seq-length sequence))
+         (signal-fn (lambda ()
+                      (signal 'args-out-of-range
+                              (if end
+                                  (list sequence start end)
+                                (list sequence start)))))
+         (signal-or-val-fn (lambda (val)
+                             (cond
+                              ((> val len)
+                               (funcall signal-fn))
+                              ((< val 0)
+                               (let ((val2 (+ val len)))
+                                 (if (< val2 0)
+                                     (funcall signal-fn)
+                                   val2)))
+                              (t
+                               val))))
+         (idx-start (funcall signal-or-val-fn start))
+         (idx-end (if (null end)
+                      len
+                    (funcall signal-or-val-fn end))))
+    (if (> idx-start idx-end)
+        (funcall signal-fn)
+      (seq-into (seq-map-indexed (lambda (elem idx)
+                                   (if (and (<= idx-start idx)
+                                            (< idx idx-end)
+                                            replacements)
+                                       (pop replacements)
+                                     elem))
+                                 sequence)
+                (if (listp sequence)
+                    'list
+                  (type-of sequence))))))
+
 ;;;###autoload
 (cl-defgeneric seq-subseq (sequence start &optional end)
   "Return the sequence of elements of SEQUENCE from START to END.
@@ -167,6 +245,19 @@ seq-subseq
 START or END is negative, it counts from the end.  Signal an
 error if START or END are outside of the sequence (i.e too large
 if positive or too small if negative)."
+  (declare
+   (gv-expander
+    (lambda (do)
+      (gv-letplace (getter setter) `(gv-delay-error ,sequence)
+        (macroexp-let2* nil ((start start) (end end))
+          (funcall do
+                   `(seq-subseq ,getter ,start ,end)
+                   (lambda (v)
+                     (macroexp-let2 nil v v
+                       `(progn
+                          ,(funcall setter
+                                    `(seq-replace ,getter ,v ,start ,end))
+                          ,v)))))))))
   (cond
    ((or (stringp sequence) (vectorp sequence)) (substring sequence start end))
    ((listp sequence)
diff --git a/test/lisp/emacs-lisp/seq-tests.el b/test/lisp/emacs-lisp/seq-tests.el
index c06ceb00bdb..44fd5350f72 100644
--- a/test/lisp/emacs-lisp/seq-tests.el
+++ b/test/lisp/emacs-lisp/seq-tests.el
@@ -312,6 +312,330 @@ test-seq-subseq
                   (:success
                    (should (equal (seq-subseq list start end) res))))))))))))
 
+(cl-defmacro test-seq-replace-combinations
+    (&key init-vals sub-vals result range error)
+  "Make a test for each combination of sequence type for `seq-subseq' using `setf'.
+
+- INIT-VALS is a list holding the initial elements.
+- RESULT is what the final value should be after substitution.
+- ERROR is whether the form should signal `args-out-of-range'.
+- SUB-VALS is a list holding the elements to be substituted in.
+- RANGE is a list of the `start' and `end' arguments of `seq-subseq'."
+  (let ((tests))
+    (dolist (type1 '(list vector string))
+      (dolist (type2 '(list vector string))
+        (push  (if error
+                   `(should-error (seq-replace (,type1 ,@init-vals)
+                                               (,type2 ,@sub-vals)
+                                               ,@range)
+                                  :type (quote args-out-of-range))
+                 `(should (equal (,type1 ,@result)
+                                 (seq-replace (,type1 ,@init-vals)
+                                              (,type2 ,@sub-vals)
+                                              ,@range))))
+               tests)))
+    `(progn ,@tests)))
+
+(ert-deftest test-seq-replace ()
+  "Test using `seq-replace' with `setf'.
+
+Any combination of sequences should work.
+
+An error should be signalled if the inclusive starting index or
+the exclusive ending index is out of the range from 0 through the
+length of the sequence, or if the starting index is greater than
+the ending index.  If the starting index is equal to the ending
+index, then nothing is changed.  If the starting index is equal
+to the length of the sequence, then nothing is changed.  It
+should signal an error in all the cases that `seq-replace' signals
+an error."
+  (test-seq-replace-combinations
+   :init-vals (0 1 2)
+   :sub-vals (10 11 12)
+   :range (0)
+   :result (10 11 12))
+
+  (test-seq-replace-combinations
+   :init-vals (0 1)
+   :sub-vals (10 11 12)
+   :range (0)
+   :result (10 11))
+
+  (test-seq-replace-combinations
+   :init-vals (0 1 2)
+   :sub-vals (10 11 12)
+   :range (1)
+   :result (0 10 11))
+
+  (test-seq-replace-combinations
+   :init-vals (0 1 2)
+   :sub-vals (10 11 12)
+   :range (1 3)
+   :result (0 10 11))
+
+  (test-seq-replace-combinations
+   :init-vals (0 1 2)
+   :sub-vals (10 11 12)
+   :range (3 1)
+   :error t)
+
+  (test-seq-replace-combinations
+   :init-vals (0 1 2)
+   :sub-vals (10 11 12)
+   :range (1 100)
+   :error t)
+
+  (test-seq-replace-combinations
+   :init-vals (0 1 2)
+   :sub-vals (10 11 12)
+   :range (7)
+   :error t)
+
+  (test-seq-replace-combinations
+   :init-vals (0 1 2 3 4)
+   :sub-vals (12 13 14 15)
+   :range (2 3)
+   :result (0 1 12 3 4))
+
+  (test-seq-replace-combinations
+   :init-vals (0 1 2 3 4)
+   :sub-vals (12 13 14 15)
+   :range (2 2)
+   :result (0 1 2 3 4))
+
+  (test-seq-replace-combinations
+   :init-vals (0 1 2 3 4)
+   :sub-vals (12 13 14 15)
+   :range (5)
+   :result (0 1 2 3 4))
+
+  (test-seq-replace-combinations
+   :init-vals (0 1 2 3 4)
+   :sub-vals (12 13 14 15)
+   :range (6)
+   :error t)
+
+  (test-seq-replace-combinations
+   :init-vals (0 1 2 3 4)
+   :sub-vals (12 13 14 15)
+   :range (5 6)
+   :error t)
+
+  (test-seq-replace-combinations
+   :init-vals (0 1 2 3 4 5 6 7 8 9)
+   :sub-vals (10 11 12 13 14)
+   :range (-2)
+   :result (0 1 2 3 4 5 6 7 10 11))
+
+  (test-seq-replace-combinations
+   :init-vals (0 1 2 3 4 5 6 7 8 9)
+   :sub-vals (10 11 12 13 14)
+   :range (-6 -3)
+   :result (0 1 2 3 10 11 12 7 8 9))
+
+  (test-seq-replace-combinations
+   :init-vals (0 1 2 3 4 5 6 7 8 9)
+   :sub-vals (10 11 12 13 14)
+   :range (-6 -10)
+   :error t)
+
+  ;; This range might make sense, but since it would signal an error
+  ;; in `seq-subseq', we also signal an error in the `setf' feature.
+  (test-seq-replace-combinations
+   :init-vals (0 1 2 3 4 5 6 7 8 9)
+   :sub-vals (10 11 12 13 14)
+   :range (-6 0)
+   :error t)
+
+  (test-seq-replace-combinations
+   :init-vals (0 1 2 3 4 5 6 7 8 9)
+   :sub-vals (10 11 12 13 14)
+   :range (100)
+   :error t)
+
+  (test-seq-replace-combinations
+   :init-vals (0 1 2 3 4 5 6 7 8 9)
+   :sub-vals (10 11 12 13 14)
+   :range (-100)
+   :error t))
+
+(cl-defmacro test-setf-seq-subseq-combinations
+    (&key init-vals sub-vals result range error)
+  "Make a test for each combination of sequence type for `seq-subseq' using `setf'.
+
+- INIT-VALS is a list holding the initial elements.
+- RESULT is what the final value should be after substitution.
+- ERROR is whether the form should signal `args-out-of-range'.
+- SUB-VALS is a list holding the elements to be substituted in.
+- RANGE is a list of the `start' and `end' arguments of `seq-subseq'."
+  (let ((tests))
+    (dolist (type1 '(list vector string))
+      (dolist (type2 '(list vector string))
+        (push  (if error
+                   `(should-error (let ((seq (,type1 ,@init-vals)))
+                                    (setf (seq-subseq seq ,@range)
+                                          (,type2 ,@sub-vals))
+                                    seq)
+                                  :type (quote args-out-of-range))
+                 `(should (equal (,type1 ,@result)
+                                 (let ((seq (,type1 ,@init-vals)))
+                                   (setf (seq-subseq seq ,@range)
+                                         (,type2 ,@sub-vals))
+                                   seq))))
+               tests)))
+    `(progn ,@tests)))
+
+(ert-deftest test-setf-seq-subseq ()
+  "Test using `seq-subseq' with `setf'.
+
+Any combination of sequences should work.
+
+An error should be signalled if the inclusive starting index or
+the exclusive ending index is out of the range from 0 through the
+length of the sequence, or if the starting index is greater than
+the ending index.  If the starting index is equal to the ending
+index, then nothing is changed.  If the starting index is equal
+to the length of the sequence, then nothing is changed.  It
+should signal an error in all the cases that `seq-subseq' signals
+an error."
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2)
+   :sub-vals (10 11 12)
+   :range (0)
+   :result (10 11 12))
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1)
+   :sub-vals (10 11 12)
+   :range (0)
+   :result (10 11))
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2)
+   :sub-vals (10 11 12)
+   :range (1)
+   :result (0 10 11))
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2)
+   :sub-vals (10 11 12)
+   :range (1 3)
+   :result (0 10 11))
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2)
+   :sub-vals (10 11 12)
+   :range (3 1)
+   :error t)
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2)
+   :sub-vals (10 11 12)
+   :range (1 100)
+   :error t)
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2)
+   :sub-vals (10 11 12)
+   :range (7)
+   :error t)
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2 3 4)
+   :sub-vals (12 13 14 15)
+   :range (2 3)
+   :result (0 1 12 3 4))
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2 3 4)
+   :sub-vals (12 13 14 15)
+   :range (2 2)
+   :result (0 1 2 3 4))
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2 3 4)
+   :sub-vals (12 13 14 15)
+   :range (5)
+   :result (0 1 2 3 4))
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2 3 4)
+   :sub-vals (12 13 14 15)
+   :range (6)
+   :error t)
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2 3 4)
+   :sub-vals (12 13 14 15)
+   :range (5 6)
+   :error t)
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2 3 4 5 6 7 8 9)
+   :sub-vals (10 11 12 13 14)
+   :range (-2)
+   :result (0 1 2 3 4 5 6 7 10 11))
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2 3 4 5 6 7 8 9)
+   :sub-vals (10 11 12 13 14)
+   :range (-6 -3)
+   :result (0 1 2 3 10 11 12 7 8 9))
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2 3 4 5 6 7 8 9)
+   :sub-vals (10 11 12 13 14)
+   :range (-6 -10)
+   :error t)
+
+  ;; This range might make sense, but since it would signal an error
+  ;; in `seq-subseq', we also signal an error in the `setf' feature.
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2 3 4 5 6 7 8 9)
+   :sub-vals (10 11 12 13 14)
+   :range (-6 0)
+   :error t)
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2 3 4 5 6 7 8 9)
+   :sub-vals (10 11 12 13 14)
+   :range (100)
+   :error t)
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2 3 4 5 6 7 8 9)
+   :sub-vals (10 11 12 13 14)
+   :range (-100)
+   :error t))
+
+(ert-deftest test-setf-seq-subseq-recursive ()
+  "Test using `setf' with `seq-subseq' on sub-places.
+Like using `setf' with `substring'."
+  (let ((vect (vector 0 1 2 3 4 5 6)))
+    (setf (seq-subseq (seq-subseq vect 2) 2) [111 222])
+    (should (equal vect (vector 0 1 2 3 111 222 6))))
+
+  (let ((str (string ?a ?b ?c ?d ?e ?f ?g)))
+    (setf (seq-subseq (seq-subseq str 2) 0) (list ?x ?y ?z ?1 ?2 ?3))
+    (should (equal str (string ?a ?b ?x ?y ?z ?1 ?2))))
+
+  (let ((lst (list ?a ?b ?c ?d ?e ?f ?g)))
+    (setf (seq-subseq (seq-subseq (seq-subseq (seq-subseq lst 0)
+                                              0)
+                                  2)
+                      -5
+                      -3)
+          (vector ?x ?y ?z ?1 ?2 ?3))
+    (should (equal lst (list ?a ?b ?x ?y ?e ?f ?g))))
+
+  (let ((lst (list 0 1 2 3 4 5 6 7 8 9)))
+    (setf (seq-subseq (seq-subseq (seq-subseq (seq-subseq lst 1)
+                                              1)
+                                  1)
+                      1)
+          (vector 111 222 333 444 555 666 777 888))
+    (should (equal lst (list 0 1 2 3 111 222 333 444 555 666)))))
+
 (ert-deftest test-seq-concatenate ()
   (with-test-sequences (seq '(2 4 6))
     (should (equal (seq-concatenate 'string seq [8]) (string 2 4 6 8)))
-- 
2.34.1


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

* bug#68863: Add support for using setf with seq-subseq
  2024-04-18  2:54         ` Okamsn via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-05-07  1:45           ` Okamsn via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2024-05-08 21:01             ` Michael Heerdegen via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 1 reply; 9+ messages in thread
From: Okamsn via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-05-07  1:45 UTC (permalink / raw)
  To: Stefan Monnier, Eli Zaretskii, Michael Heerdegen; +Cc: 68863, Nicolas Petton

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

Okamsn wrote:
> Okamsn wrote:
>> Hello,
>>
>> After testing it more, I see that what I've written does not work as I
>> expected in the case
>>
>> (let ((v    (vector (vector 0 1)
>>                        (vector 2 3)
>>                        (vector 4 5))))
>>      (setf (seq-subseq (seq-subseq (elt v 0) 0) 0)
>>            [10])
>>      v)
>>
>> in which I would expect it to replace the first element of the first
>> sub-vector with 10. I will take more time to continue working on this.
>>
>> Thank you for your patience.
>>
>>
> 
> Hello,
> 
> I found a way to work with subplaces, like in the example in my previous
> e-mail message. Instead of creating the generic feature `(setf
> seq-subseq)` like what is done for `seq-elt`, I created a generic
> function `seq-replace`, which is used in a new `gv-expander` for
> `seq-subseq`. This way of doing it is like what is done for `substring`,
> which has the behavior that I wanted.
> 
> What do you think about this approach?
> 
> Thank you.

Hello,

Since supporting sub-places is controversial, would you please review 
version 2 of the patch that I sent, which I have re-attached for 
convenience. This version /does not/ support sub-places.

I have added Michael Heerdegen to the recipients list in case they would 
like to comment.

Thank you.

[-- Attachment #2: v2-0001-Add-setf-support-for-seq-subseq.patch --]
[-- Type: text/x-patch, Size: 10101 bytes --]

From b06db7905f5d6dfa0d33c05fd214ef95d19814b5 Mon Sep 17 00:00:00 2001
From: Earl Hyatt <okamsn@protonmail.com>
Date: Sun, 28 Jan 2024 22:48:13 -0500
Subject: [PATCH v2] Add setf support for seq-subseq.

* lisp/emacs-lisp/seq.el (seq-subseq): Add a generic version of
calling setf on seq-subseq and add a specialized version for when the
modified sequence is a list.
* test/lisp/emacs-lisp/seq-tests.el (test-setf-seq-subseq)
(test-setf-seq-subseq-combinations): Add tests for the feature.

The feature will signal 'args-out-of-range' if the starting index or
ending index (if given) is outside of the range of values from 0
through the length of the sequence or from the negative length of the
sequence through negative 1.  If the starting index is equal to the
length of the sequence, then nothing is changed.  If the starting
index is equal to the ending index, then nothing is changed.  The
feature should signal an error in all cases where using 'seq-subseq'
would signal an error.
---
 lisp/emacs-lisp/seq.el            |  88 ++++++++++++++++++
 test/lisp/emacs-lisp/seq-tests.el | 149 ++++++++++++++++++++++++++++++
 2 files changed, 237 insertions(+)

diff --git a/lisp/emacs-lisp/seq.el b/lisp/emacs-lisp/seq.el
index 4c6553972c2..6a1fd4c35e3 100644
--- a/lisp/emacs-lisp/seq.el
+++ b/lisp/emacs-lisp/seq.el
@@ -193,6 +193,94 @@ seq-subseq
         (copy-sequence sequence))))
    (t (error "Unsupported sequence: %s" sequence))))
 
+(cl-defgeneric (setf seq-subseq) (store sequence start &optional end)
+  "Modify the elements of SEQUENCE from START to END to be those of STORE.
+END is exclusive.
+
+If END is omitted, it defaults to the length of the sequence.  If
+START or END is negative, it counts from the end.  Signal an
+error if START or END are outside of the sequence (i.e too large
+if positive or too small if negative).
+
+SEQUENCE is neither lengthened nor shortened."
+  (let* ((len (seq-length sequence))
+         (signal-fn (lambda ()
+                      (signal 'args-out-of-range
+                              (if end
+                                  (list sequence start end)
+                                (list sequence start)))))
+         (signal-or-val-fn (lambda (val)
+                             (cond
+                              ((> val len)
+                               (funcall signal-fn))
+                              ((< val 0)
+                               (let ((val2 (+ val len)))
+                                 (if (< val2 0)
+                                     (funcall signal-fn)
+                                   val2)))
+                              (t
+                               val))))
+         (idx (funcall signal-or-val-fn start))
+         (idx-end (if (null end)
+                      len
+                    (funcall signal-or-val-fn end)))
+         (tag (gensym)))
+    (if (> idx idx-end)
+        (funcall signal-fn)
+      (catch tag
+        (seq-do (lambda (v)
+                  (if (< idx idx-end)
+                      (setf (seq-elt sequence idx) v
+                            idx (1+ idx))
+                    (throw tag nil)))
+                store))))
+  store)
+
+(cl-defmethod (setf seq-subseq) (store (sequence list) start &optional end)
+  "Modify the elements of SEQUENCE from START to END to be those of STORE.
+END is exclusive.
+
+If END is omitted, it defaults to the length of the sequence.  If
+START or END is negative, it counts from the end.  Signal an
+error if START or END are outside of the sequence (i.e too large
+if positive or too small if negative).
+
+SEQUENCE is neither lengthened nor shortened."
+  (let* ((len (seq-length sequence))
+         (signal-fn (lambda ()
+                      (signal 'args-out-of-range
+                              (if end
+                                  (list sequence start end)
+                                (list sequence start)))))
+         (signal-or-val-fn (lambda (val)
+                             (cond
+                              ((> val len)
+                               (funcall signal-fn))
+                              ((< val 0)
+                               (let ((val2 (+ val len)))
+                                 (if (< val2 0)
+                                     (funcall signal-fn)
+                                   val2)))
+                              (t
+                               val))))
+         (idx (funcall signal-or-val-fn start))
+         (idx-end (if (null end)
+                      len
+                    (funcall signal-or-val-fn end)))
+         (tag (gensym)))
+    (if (> idx idx-end)
+        (funcall signal-fn)
+      (catch tag
+        (seq-do (let ((replaced (nthcdr idx sequence)))
+                  (lambda (v)
+                    (if (< idx idx-end)
+                        (setf (car replaced) v
+                              replaced (cdr replaced)
+                              idx (1+ idx))
+                      (throw tag nil))))
+                store))))
+  store)
+
 \f
 (cl-defgeneric seq-map (function sequence)
   "Return the result of applying FUNCTION to each element of SEQUENCE."
diff --git a/test/lisp/emacs-lisp/seq-tests.el b/test/lisp/emacs-lisp/seq-tests.el
index c06ceb00bdb..d3e46c32f99 100644
--- a/test/lisp/emacs-lisp/seq-tests.el
+++ b/test/lisp/emacs-lisp/seq-tests.el
@@ -312,6 +312,155 @@ test-seq-subseq
                   (:success
                    (should (equal (seq-subseq list start end) res))))))))))))
 
+(cl-defmacro test-setf-seq-subseq-combinations
+    (&key init-vals sub-vals result range error)
+  "Make a test for each combination of sequence type for `seq-subseq' using `setf'.
+
+- INIT-VALS is a list holding the initial elements.
+- RESULT is what the final value should be after substitution.
+- ERROR is whether the form should signal `args-out-of-range'.
+- SUB-VALS is a list holding the elements to be substituted in.
+- RANGE is a list of the `start' and `end' arguments of `seq-subseq'."
+  (let ((tests))
+    (dolist (type1 '(list vector string))
+      (dolist (type2 '(list vector string))
+        (push  (if error
+                   `(should-error (let ((seq (,type1 ,@init-vals)))
+                                    (setf (seq-subseq seq ,@range)
+                                          (,type2 ,@sub-vals))
+                                    seq)
+                                  :type (quote args-out-of-range))
+                 `(should (equal (,type1 ,@result)
+                                 (let ((seq (,type1 ,@init-vals)))
+                                   (setf (seq-subseq seq ,@range)
+                                         (,type2 ,@sub-vals))
+                                   seq))))
+               tests)))
+    `(progn ,@tests)))
+
+(ert-deftest test-setf-seq-subseq ()
+  "Test using `seq-subseq' with `setf'.
+
+Any combination of sequences should work.
+
+An error should be signalled if the inclusive starting index or
+the exclusive ending index is out of the range from 0 through the
+length of the sequence, or if the starting index is greater than
+the ending index.  If the starting index is equal to the ending
+index, then nothing is changed.  If the starting index is equal
+to the length of the sequence, then nothing is changed.  It
+should signal an error in all the cases that `seq-subseq' signals
+an error."
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2)
+   :sub-vals (10 11 12)
+   :range (0)
+   :result (10 11 12))
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1)
+   :sub-vals (10 11 12)
+   :range (0)
+   :result (10 11))
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2)
+   :sub-vals (10 11 12)
+   :range (1)
+   :result (0 10 11))
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2)
+   :sub-vals (10 11 12)
+   :range (1 3)
+   :result (0 10 11))
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2)
+   :sub-vals (10 11 12)
+   :range (3 1)
+   :error t)
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2)
+   :sub-vals (10 11 12)
+   :range (1 100)
+   :error t)
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2)
+   :sub-vals (10 11 12)
+   :range (7)
+   :error t)
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2 3 4)
+   :sub-vals (12 13 14 15)
+   :range (2 3)
+   :result (0 1 12 3 4))
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2 3 4)
+   :sub-vals (12 13 14 15)
+   :range (2 2)
+   :result (0 1 2 3 4))
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2 3 4)
+   :sub-vals (12 13 14 15)
+   :range (5)
+   :result (0 1 2 3 4))
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2 3 4)
+   :sub-vals (12 13 14 15)
+   :range (6)
+   :error t)
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2 3 4)
+   :sub-vals (12 13 14 15)
+   :range (5 6)
+   :error t)
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2 3 4 5 6 7 8 9)
+   :sub-vals (10 11 12 13 14)
+   :range (-2)
+   :result (0 1 2 3 4 5 6 7 10 11))
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2 3 4 5 6 7 8 9)
+   :sub-vals (10 11 12 13 14)
+   :range (-6 -3)
+   :result (0 1 2 3 10 11 12 7 8 9))
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2 3 4 5 6 7 8 9)
+   :sub-vals (10 11 12 13 14)
+   :range (-6 -10)
+   :error t)
+
+  ;; This range might make sense, but since it would signal an error
+  ;; in `seq-subseq', we also signal an error in the `setf' feature.
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2 3 4 5 6 7 8 9)
+   :sub-vals (10 11 12 13 14)
+   :range (-6 0)
+   :error t)
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2 3 4 5 6 7 8 9)
+   :sub-vals (10 11 12 13 14)
+   :range (100)
+   :error t)
+
+  (test-setf-seq-subseq-combinations
+   :init-vals (0 1 2 3 4 5 6 7 8 9)
+   :sub-vals (10 11 12 13 14)
+   :range (-100)
+   :error t))
+
 (ert-deftest test-seq-concatenate ()
   (with-test-sequences (seq '(2 4 6))
     (should (equal (seq-concatenate 'string seq [8]) (string 2 4 6 8)))
-- 
2.34.1


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

* bug#68863: Add support for using setf with seq-subseq
  2024-05-07  1:45           ` Okamsn via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-05-08 21:01             ` Michael Heerdegen via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 0 replies; 9+ messages in thread
From: Michael Heerdegen via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-05-08 21:01 UTC (permalink / raw)
  To: Okamsn; +Cc: 68863, Eli Zaretskii, Nicolas Petton, Stefan Monnier

Okamsn <okamsn@protonmail.com> writes:

> Subject: [PATCH v2] Add setf support for seq-subseq.

Thank you for the update.

I wonder about the implementation for listp STOREs:

> +(cl-defmethod (setf seq-subseq) (store (sequence list) start &optional end)
> +  "Modify the elements of SEQUENCE from START to END to be those of STORE.
> +END is exclusive.
> +
> +If END is omitted, it defaults to the length of the sequence.  If
> +START or END is negative, it counts from the end. [...]

In the case where we don't count from the end:

> +error if START or END are outside of the sequence (i.e too large
> +if positive or too small if negative).
> +
> +SEQUENCE is neither lengthened nor shortened."
> +  (let* ((len (seq-length sequence))

running through the complete list just to get its length which we don't
need is a waste of time - right?

> +      (catch tag
> +        (seq-do (let ((replaced (nthcdr idx sequence)))
> +                  (lambda (v)
> +                    (if (< idx idx-end)
> +                        (setf (car replaced) v
> +                              replaced (cdr replaced)
> +                              idx (1+ idx))
> +                      (throw tag nil))))
> +                store))))
> +  store)

And wouldn't it here be more efficient to use `setcar' and `setcdr'
manipulations to insert the complete sequence (if it's a list, else we
may convert it into one) in one go instead of replacing elements one by
one?


Thanks,

Michael.





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

end of thread, other threads:[~2024-05-08 21:01 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-01  3:31 bug#68863: Add support for using setf with seq-subseq Okamsn via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-02-04 18:33 ` bug#68863: [PATCH] " Okamsn via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-02-08 11:39 ` bug#68863: " Eli Zaretskii
2024-02-08 14:25   ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-02-09  3:54     ` Okamsn via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-02-14  2:50       ` Okamsn via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-04-18  2:54         ` Okamsn via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-05-07  1:45           ` Okamsn via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-05-08 21:01             ` Michael Heerdegen via Bug reports for GNU Emacs, the Swiss army knife of text editors

Code repositories for project(s) associated with this public inbox

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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).