bug#81581: [PATCH 1/1] Add a LESSP argument to 'seq-min' and 'seq-max'
Philip Kaludercic <[email protected]>
| Newsgroups | gmane.emacs.bugs |
|---|---|
| Message-ID | <[email protected]> |
Eli Zaretskii <[email protected]> writes: >> From: Philip Kaludercic <[email protected]> >> Cc: Stefan Monnier <[email protected]>, Nicolas Petton >> <[email protected]>, [email protected] >> Date: Sun, 09 Aug 2026 07:28:27 +0000 >> >> Eli Zaretskii <[email protected]> writes: >> >> >> From: Philip Kaludercic <[email protected]> >> >> Date: Sat, 08 Aug 2026 19:59:57 +0000 >> >> >> >> -(cl-defgeneric seq-min (sequence) >> >> +(cl-defgeneric seq-min (sequence &optional lessp) >> >> "Return the smallest element of SEQUENCE. >> >> -SEQUENCE must be a sequence of numbers or markers." >> >> - (apply #'min (seq-into sequence 'list))) >> >> +Values are compared according to the optional parameter LESSP, which >> >> +defaults to `value<'." >> >> + (unless lessp (setq lessp #'value<)) >> >> + (let ((fresh (eval-when-compile (make-symbol "fresh")))) >> >> + (seq-reduce >> >> + (lambda (acc elt) >> >> + (cond >> >> + ((eq acc fresh) elt) >> >> + ((funcall lessp acc elt) acc) >> >> + (t elt))) >> >> + sequence >> >> + fresh))) >> > >> > How about optimizing for the default nil value of LESSP? The original >> > implementation should be faster than the modified one, so how about >> > keeping the original performance for those who don't need a fancy >> > comparison function? >> >> Or did I misunderstand you, and you are advocating for a case >> distinction on LESSP, and dispatch to `min' and `max' if LESSP is `<` >> (instead of nil)? > > Yes, that's what I meant. So something like > I'll also ask why shouldn't the default be '<' anyway, to preserve > backward compatibility? The old implementations would signal an error if you were to call the method with non-numerical values in the list, so would argue that we are not breaking backwards compatibility, unless you think that there are applications that depend on type errors being raised.
(unnamed)
(text/x-patch, 1.8 KB)
diff --git a/lisp/emacs-lisp/seq.el b/lisp/emacs-lisp/seq.el
index b8f35c10def..1d85817d093 100644
--- a/lisp/emacs-lisp/seq.el
+++ b/lisp/emacs-lisp/seq.el
@@ -604,16 +604,41 @@ seq-group-by
(seq-reverse sequence)
nil))
-(cl-defgeneric seq-min (sequence)
+(cl-defgeneric seq-min (sequence &optional lessp)
"Return the smallest element of SEQUENCE.
-SEQUENCE must be a sequence of numbers or markers."
- (apply #'min (seq-into sequence 'list)))
-
-;;;###autoload
-(cl-defgeneric seq-max (sequence)
+Values are compared according to the optional parameter LESSP, which
+defaults to `value<'."
+ (pcase (or lessp #'value<)
+ ('< (apply #'min (seq-into sequence 'list)))
+ ('> (apply #'max (seq-into sequence 'list)))
+ (lessp
+ (let ((fresh (eval-when-compile (make-symbol "fresh"))))
+ (seq-reduce
+ (lambda (acc elt)
+ (cond
+ ((eq acc fresh) elt)
+ ((funcall lessp acc elt) acc)
+ (t elt)))
+ sequence
+ fresh)))))
+
+(cl-defgeneric seq-max (sequence &optional lessp)
"Return the largest element of SEQUENCE.
-SEQUENCE must be a sequence of numbers or markers."
- (apply #'max (seq-into sequence 'list)))
+Values are compared according to the optional parameter LESSP, which
+defaults to `value<'."
+ (pcase (or lessp #'value<)
+ ('< (apply #'max (seq-into sequence 'list)))
+ ('> (apply #'min (seq-into sequence 'list)))
+ (lessp
+ (let ((fresh (eval-when-compile (make-symbol "fresh"))))
+ (seq-reduce
+ (lambda (acc elt)
+ (cond
+ ((eq acc fresh) elt)
+ ((funcall lessp acc elt) elt)
+ (t acc)))
+ sequence
+ fresh)))))
(defun seq--count-successive (pred sequence)
"Count successive elements in SEQUENCE for which PRED returns non-nil."