Let by return a boolean instead of an integer

crates/typst-library/src/foundations/array.rs

@@ -822,10 +822,14 @@ impl Array {
         key: Option<Func>,
         /// If given, uses this function to compare elements in the array.
         ///
-        /// This function should return an integer, whose sign is used to
+        /// This function should return a boolean: `{true}` indicates that the
-        /// determine the relative order of two given elements: Negative if the
+        /// elements are in order, while `{false}` indicates that they should be
-        /// first element is smaller, positive if the second element is smaller.
+        /// swapped. To keep the sort stable, if the two elements are equal, the
-        /// If `{0}` is returned, the order of the elements is not modified.
+        /// function should return `{true}`.
+        ///
+        /// If this function does not order the elements properly (e.g., by
+        /// returning `{false}` for both `{(x, y)}` and `{(y, x)}`), the
+        /// resulting array will be unspecified order.
         ///
         /// When used together with `key`, `by` will be passed the keys instead
         /// of the elements.
@@ -838,45 +842,73 @@ impl Array {
         ///   "length",
         /// ).sorted(
         ///   key: s => s.len(),
-        ///   by: (x, y) => y - x,
+        ///   by: (l, r) => l >= r,
         /// )
         /// ```
         #[named]
         by: Option<Func>,
     ) -> SourceResult<Array> {
-        let mut result = Ok(());
+        let mut are_in_order = |mut x: Value, mut y: Value| {
-        let mut vec = self.0;
+            if let Some(f) = &key {
-        let mut compare = |x: Value, y: Value| {
+                x = f.call(engine, context, [x])?;
-            let mut key_of = |x: Value| match &key {
+                y = f.call(engine, context, [y])?;
-                // NOTE: We are relying on `comemo`'s memoization of function
+            }
-                // evaluation to not excessively reevaluate the `key`.
-                Some(f) => f.call(engine, context, [x]),
-                None => Ok(x),
-            };
-            let x = key_of(x)?;
-            let y = key_of(y)?;
             match &by {
-                Some(f) => Ok(match f.call(engine, context, [x, y])? {
+                Some(f) => match f.call(engine, context, [x, y])? {
-                    Value::Int(x) => x.cmp(&0),
+                    Value::Bool(b) => Ok(b),
                     x => {
-                        bail!(span, "expected integer from `by` function, got {}", x.ty())
+                        bail!(span, "expected boolean from `by` function, got {}", x.ty())
                     }
-                }),
+                },
-                None => ops::compare(&x, &y).at(span),
+                None => {
+                    // `x` and `y` are in order iff `y` is not strictly greater
+                    // than `y`.
+                    ops::compare(&x, &y).at(span).map(|o| o != Ordering::Greater)
+                }
             }
         };
+        let mut vec = self.0.into_iter().enumerate().collect::<Vec<_>>();
+        let mut result = Ok(());
         // We use `glidesort` instead of the standard library sorting algorithm
         // to prevent panics (see https://github.com/typst/typst/pull/5627).
-        glidesort::sort_by(vec.make_mut(), |a, b| {
+        glidesort::sort_by(&mut vec, |(i, x), (j, y)| {
-            // Until we get `try` blocks :)
+            // Because we use booleans for the comparison function, in order to
-            compare(a.clone(), b.clone()).unwrap_or_else(|err| {
+            // keep the sort stable, we need to compare in the right order.
-                if result.is_ok() {
+            if i < j {
-                    result = Err(err);
+                // If `x` and `y` appear in this order in the original array,
+                // then we should change their order (i.e., return
+                // `Ordering::Greater`) iff `y` is strictly less than `x` (i.e.,
+                // `compare(x, y)` returns `false`). Otherwise, we should keep
+                // them in the same order (i.e., return `Ordering::Less`).
+                match are_in_order(x.clone(), y.clone()) {
+                    Ok(false) => Ordering::Greater,
+                    Ok(true) => Ordering::Less,
+                    Err(err) => {
+                        if result.is_ok() {
+                            result = Err(err);
+                        }
+                        Ordering::Equal
+                    }
                 }
-                Ordering::Equal
+            } else {
-            })
+                // If `x` and `y` appear in the opposite order in the original
+                // array, then we should change their order (i.e., return
+                // `Ordering::Less`) iff `x` is strictly less than `y` (i.e.,
+                // `compare(y, x)` returns `false`). Otherwise, we should keep
+                // them in the same order (i.e., return `Ordering::Less`).
+                match are_in_order(y.clone(), x.clone()) {
+                    Ok(false) => Ordering::Less,
+                    Ok(true) => Ordering::Greater,
+                    Err(err) => {
+                        if result.is_ok() {
+                            result = Err(err);
+                        }
+                        Ordering::Equal
+                    }
+                }
+            }
         });
-        result.map(|_| vec.into())
+        result.map(|_| vec.into_iter().map(|(_, x)| x).collect())
     }
 
     /// Deduplicates all items in the array.

tests/suite/foundations/array.typ

@@ -355,11 +355,11 @@
 #test((2, 1, 3, 10, 5, 8, 6, -7, 2).sorted(), (-7, 1, 2, 2, 3, 5, 6, 8, 10))
 #test((2, 1, 3, -10, -5, 8, 6, -7, 2).sorted(key: x => x), (-10, -7, -5, 1, 2, 2, 3, 6, 8))
 #test((2, 1, 3, -10, -5, 8, 6, -7, 2).sorted(key: x => x * x), (1, 2, 2, 3, -5, 6, -7, 8, -10))
-#test(("I", "the", "hi", "text").sorted(by: (x, y) => x.len() - y.len()), ("I", "hi", "the", "text"))
+#test(("I", "the", "hi", "text").sorted(by: (x, y) => x.len() < y.len()), ("I", "hi", "the", "text"))
-#test(("I", "the", "hi", "text").sorted(key: x => x.len(), by: (x, y) => y - x), ("text", "the", "hi", "I"))
+#test(("I", "the", "hi", "text").sorted(key: x => x.len(), by: (x, y) => y < x), ("text", "the", "hi", "I"))
 
 --- array-sorted-invalid-by-function ---
-// Error: 2-39 expected integer from `by` function, got string
+// Error: 2-39 expected boolean from `by` function, got string
 #(1, 2, 3).sorted(by: (_, _) => "hmm")
 
 --- array-sorted-key-function-positional-1 ---
@@ -451,7 +451,7 @@
 #([Hi], [There]).sorted()
 
 --- array-sorted-uncomparable-lengths ---
-// Error: 2-26 cannot compare 3em with 2pt
+// Error: 2-26 cannot compare 2pt with 3em
 #(1pt, 2pt, 3em).sorted()
 
 --- array-sorted-key-function-positional-2 ---