Repository: rust-itertools/itertools
Pull request #704 (2023-06-14 12:38:14)

`MergeJoinBy` also accept functions returning `bool`
Related to #701 . The produced iterator then yields `Either` items instead of `EitherOrBoth` items when the user function returns `Ordering`.

Philippe-Cholet: @jswrenn There is no breaking change, that's the all point of the (private) `OrderingOrBool` trait. The user can still give functions returning `Ordering` (resulting in an iterator of `EitherOrBoth` items). Now the user can also give functions returning `bool` (resulting in an iterator of `Either` items). ... Then we will be able to merge implementations of `MergeJoinBy` and `MergeBy` without breaking anything (which is the purpose of @phimuemue ).
Philippe-Cholet: I just made a last small change: "L/R" for "Left/Right" types instead of "I/J" that stand for iterators.
Should I squash all those commits ?
phimuemue: Yes, that would be nice.

Patch:
diff --git a/.github/dependabot.yml b/.github/dependabot.yml
deleted file mode 100644
index 71607d0..0000000
--- a/.github/dependabot.yml
+++ /dev/null
@@ -1,6 +0,0 @@
-version: 2
-updates:
-- package-ecosystem: github-actions
-  directory: "/"
-  schedule:
-    interval: daily
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 92122cc..143ac2b 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -8,49 +8,32 @@ on:
       - trying
 
 jobs:
-  check:
-    name: check
+  msrv:
+    name: Rust MSRV
     runs-on: ubuntu-latest
-    strategy:
-      matrix:
-        build: [msrv, stable]
-        features:
-          [
-            "",
-            "--no-default-features",
-            "--no-default-features --features use_alloc",
-            "--all-targets --all-features",
-          ]
-        include:
-          - build: msrv
-            rust: 1.62.1
-          - build: stable
-            rust: stable
-        exclude:
-          - build: msrv
-            # we only care about the MSRV with respect to the lib target
-            features: "--all-targets --all-features"
     steps:
-      - uses: actions/checkout@v3
-      - uses: dtolnay/rust-toolchain@master
-        with:
-          toolchain: ${{ matrix.rust }}
-      - run: cargo check ${{ matrix.features }}
+      - uses: actions/checkout@v2
+      - uses: dtolnay/rust-toolchain@1.36.0
+      - run: cargo check --no-default-features
+      - run: cargo check --no-default-features --features "use_alloc"
+      - run: cargo check
 
-  test:
-    name: test
+  stable:
+    name: Rust Stable
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v2
       - uses: dtolnay/rust-toolchain@stable
-      - run: cargo test --all-features
+      - run: cargo check --no-default-features
+      - run: cargo check --no-default-features --features "use_alloc"
+      - run: cargo test
 
   # https://github.com/rust-lang/crater/blob/9ab6f9697c901c4a44025cf0a39b73ad5b37d198/.github/workflows/bors.yml#L125-L149
   end-success:
     name: bors build finished
     if: success()
     runs-on: ubuntu-latest
-    needs: [check, test]
+    needs: [msrv,stable]
 
     steps:
       - name: Mark the job as successful
diff --git a/src/either_or_both.rs b/src/either_or_both.rs
index cf65fe7..c63bc46 100644
--- a/src/either_or_both.rs
+++ b/src/either_or_both.rs
@@ -1,5 +1,3 @@
-use core::ops::{Deref, DerefMut};
-
 use crate::EitherOrBoth::*;
 
 use either::Either;
@@ -16,7 +14,7 @@ pub enum EitherOrBoth<A, B> {
 }
 
 impl<A, B> EitherOrBoth<A, B> {
-    /// If `Left`, or `Both`, return true. Otherwise, return false.
+    /// If `Left`, or `Both`, return true, otherwise, return false.
     pub fn has_left(&self) -> bool {
         self.as_ref().left().is_some()
     }
@@ -26,7 +24,7 @@ impl<A, B> EitherOrBoth<A, B> {
         self.as_ref().right().is_some()
     }
 
-    /// If `Left`, return true. Otherwise, return false.
+    /// If Left, return true otherwise, return false.
     /// Exclusive version of [`has_left`](EitherOrBoth::has_left).
     pub fn is_left(&self) -> bool {
         match *self {
@@ -35,7 +33,7 @@ impl<A, B> EitherOrBoth<A, B> {
         }
     }
 
-    /// If `Right`, return true. Otherwise, return false.
+    /// If Right, return true otherwise, return false.
     /// Exclusive version of [`has_right`](EitherOrBoth::has_right).
     pub fn is_right(&self) -> bool {
         match *self {
@@ -44,12 +42,13 @@ impl<A, B> EitherOrBoth<A, B> {
         }
     }
 
-    /// If `Both`, return true. Otherwise, return false.
+    /// If Right, return true otherwise, return false.
+    /// Equivalent to `self.as_ref().both().is_some()`.
     pub fn is_both(&self) -> bool {
         self.as_ref().both().is_some()
     }
 
-    /// If `Left`, or `Both`, return `Some` with the left value. Otherwise, return `None`.
+    /// If `Left`, or `Both`, return `Some` with the left value, otherwise, return `None`.
     pub fn left(self) -> Option<A> {
         match self {
             Left(left) | Both(left, _) => Some(left),
@@ -57,7 +56,7 @@ impl<A, B> EitherOrBoth<A, B> {
         }
     }
 
-    /// If `Right`, or `Both`, return `Some` with the right value. Otherwise, return `None`.
+    /// If `Right`, or `Both`, return `Some` with the right value, otherwise, return `None`.
     pub fn right(self) -> Option<B> {
         match self {
             Right(right) | Both(_, right) => Some(right),
@@ -65,57 +64,7 @@ impl<A, B> EitherOrBoth<A, B> {
         }
     }
 
-    /// If `Left`, return `Some` with the left value. If `Right` or `Both`, return `None`.
-    ///
-    /// # Examples
-    ///
-    /// ```
-    /// // On the `Left` variant.
-    /// # use itertools::{EitherOrBoth, EitherOrBoth::{Left, Right, Both}};
-    /// let x: EitherOrBoth<_, ()> = Left("bonjour");
-    /// assert_eq!(x.just_left(), Some("bonjour"));
-    ///
-    /// // On the `Right` variant.
-    /// let x: EitherOrBoth<(), _> = Right("hola");
-    /// assert_eq!(x.just_left(), None);
-    ///
-    /// // On the `Both` variant.
-    /// let x = Both("bonjour", "hola");
-    /// assert_eq!(x.just_left(), None);
-    /// ```
-    pub fn just_left(self) -> Option<A> {
-        match self {
-            Left(left) => Some(left),
-            _ => None,
-        }
-    }
-
-    /// If `Right`, return `Some` with the right value. If `Left` or `Both`, return `None`.
-    ///
-    /// # Examples
-    ///
-    /// ```
-    /// // On the `Left` variant.
-    /// # use itertools::{EitherOrBoth::{Left, Right, Both}, EitherOrBoth};
-    /// let x: EitherOrBoth<_, ()> = Left("auf wiedersehen");
-    /// assert_eq!(x.just_left(), Some("auf wiedersehen"));
-    ///
-    /// // On the `Right` variant.
-    /// let x: EitherOrBoth<(), _> = Right("adios");
-    /// assert_eq!(x.just_left(), None);
-    ///
-    /// // On the `Both` variant.
-    /// let x = Both("auf wiedersehen", "adios");
-    /// assert_eq!(x.just_left(), None);
-    /// ```
-    pub fn just_right(self) -> Option<B> {
-        match self {
-            Right(right) => Some(right),
-            _ => None,
-        }
-    }
-
-    /// If `Both`, return `Some` containing the left and right values. Otherwise, return `None`.
+    /// If Both, return `Some` tuple containing left and right.
     pub fn both(self) -> Option<(A, B)> {
         match self {
             Both(a, b) => Some((a, b)),
@@ -123,28 +72,6 @@ impl<A, B> EitherOrBoth<A, B> {
         }
     }
 
-    /// If `Left` or `Both`, return the left value. Otherwise, convert the right value and return it.
-    pub fn into_left(self) -> A
-    where
-        B: Into<A>,
-    {
-        match self {
-            Left(a) | Both(a, _) => a,
-            Right(b) => b.into(),
-        }
-    }
-
-    /// If `Right` or `Both`, return the right value. Otherwise, convert the left value and return it.
-    pub fn into_right(self) -> B
-    where
-        A: Into<B>,
-    {
-        match self {
-            Right(b) | Both(_, b) => b,
-            Left(a) => a.into(),
-        }
-    }
-
     /// Converts from `&EitherOrBoth<A, B>` to `EitherOrBoth<&A, &B>`.
     pub fn as_ref(&self) -> EitherOrBoth<&A, &B> {
         match *self {
@@ -163,32 +90,6 @@ impl<A, B> EitherOrBoth<A, B> {
         }
     }
 
-    /// Converts from `&EitherOrBoth<A, B>` to `EitherOrBoth<&_, &_>` using the [`Deref`] trait.
-    pub fn as_deref(&self) -> EitherOrBoth<&A::Target, &B::Target>
-    where
-        A: Deref,
-        B: Deref,
-    {
-        match *self {
-            Left(ref left) => Left(left),
-            Right(ref right) => Right(right),
-            Both(ref left, ref right) => Both(left, right),
-        }
-    }
-
-    /// Converts from `&mut EitherOrBoth<A, B>` to `EitherOrBoth<&mut _, &mut _>` using the [`DerefMut`] trait.
-    pub fn as_deref_mut(&mut self) -> EitherOrBoth<&mut A::Target, &mut B::Target>
-    where
-        A: DerefMut,
-        B: DerefMut,
-    {
-        match *self {
-            Left(ref mut left) => Left(left),
-            Right(ref mut right) => Right(right),
-            Both(ref mut left, ref mut right) => Both(left, right),
-        }
-    }
-
     /// Convert `EitherOrBoth<A, B>` to `EitherOrBoth<B, A>`.
     pub fn flip(self) -> EitherOrBoth<B, A> {
         match self {
@@ -326,139 +227,6 @@ impl<A, B> EitherOrBoth<A, B> {
             Both(inner_l, inner_r) => (inner_l, inner_r),
         }
     }
-
-    /// Returns a mutable reference to the left value. If the left value is not present,
-    /// it is replaced with `val`.
-    pub fn left_or_insert(&mut self, val: A) -> &mut A {
-        self.left_or_insert_with(|| val)
-    }
-
-    /// Returns a mutable reference to the right value. If the right value is not present,
-    /// it is replaced with `val`.
-    pub fn right_or_insert(&mut self, val: B) -> &mut B {
-        self.right_or_insert_with(|| val)
-    }
-
-    /// If the left value is not present, replace it the value computed by the closure `f`.
-    /// Returns a mutable reference to the now-present left value.
-    pub fn left_or_insert_with<F>(&mut self, f: F) -> &mut A
-    where
-        F: FnOnce() -> A,
-    {
-        match self {
-            Left(left) | Both(left, _) => left,
-            Right(_) => self.insert_left(f()),
-        }
-    }
-
-    /// If the right value is not present, replace it the value computed by the closure `f`.
-    /// Returns a mutable reference to the now-present right value.
-    pub fn right_or_insert_with<F>(&mut self, f: F) -> &mut B
-    where
-        F: FnOnce() -> B,
-    {
-        match self {
-            Right(right) | Both(_, right) => right,
-            Left(_) => self.insert_right(f()),
-        }
-    }
-
-    /// Sets the `left` value of this instance, and returns a mutable reference to it.
-    /// Does not affect the `right` value.
-    ///
-    /// # Examples
-    /// ```
-    /// # use itertools::{EitherOrBoth, EitherOrBoth::{Left, Right, Both}};
-    ///
-    /// // Overwriting a pre-existing value.
-    /// let mut either: EitherOrBoth<_, ()> = Left(0_u32);
-    /// assert_eq!(*either.insert_left(69), 69);
-    ///
-    /// // Inserting a second value.
-    /// let mut either = Right("no");
-    /// assert_eq!(*either.insert_left("yes"), "yes");
-    /// assert_eq!(either, Both("yes", "no"));
-    /// ```
-    pub fn insert_left(&mut self, val: A) -> &mut A {
-        match self {
-            Left(left) | Both(left, _) => {
-                *left = val;
-                left
-            }
-            Right(right) => {
-                // This is like a map in place operation. We move out of the reference,
-                // change the value, and then move back into the reference.
-                unsafe {
-                    // SAFETY: We know this pointer is valid for reading since we got it from a reference.
-                    let right = std::ptr::read(right as *mut _);
-                    // SAFETY: Again, we know the pointer is valid since we got it from a reference.
-                    std::ptr::write(self as *mut _, Both(val, right));
-                }
-
-                if let Both(left, _) = self {
-                    left
-                } else {
-                    // SAFETY: The above pattern will always match, since we just
-                    // set `self` equal to `Both`.
-                    unsafe { std::hint::unreachable_unchecked() }
-                }
-            }
-        }
-    }
-
-    /// Sets the `right` value of this instance, and returns a mutable reference to it.
-    /// Does not affect the `left` value.
-    ///
-    /// # Examples
-    /// ```
-    /// # use itertools::{EitherOrBoth, EitherOrBoth::{Left, Both}};
-    /// // Overwriting a pre-existing value.
-    /// let mut either: EitherOrBoth<_, ()> = Left(0_u32);
-    /// assert_eq!(*either.insert_left(69), 69);
-    ///
-    /// // Inserting a second value.
-    /// let mut either = Left("what's");
-    /// assert_eq!(*either.insert_right(9 + 10), 21 - 2);
-    /// assert_eq!(either, Both("what's", 9+10));
-    /// ```
-    pub fn insert_right(&mut self, val: B) -> &mut B {
-        match self {
-            Right(right) | Both(_, right) => {
-                *right = val;
-                right
-            }
-            Left(left) => {
-                // This is like a map in place operation. We move out of the reference,
-                // change the value, and then move back into the reference.
-                unsafe {
-                    // SAFETY: We know this pointer is valid for reading since we got it from a reference.
-                    let left = std::ptr::read(left as *mut _);
-                    // SAFETY: Again, we know the pointer is valid since we got it from a reference.
-                    std::ptr::write(self as *mut _, Both(left, val));
-                }
-                if let Both(_, right) = self {
-                    right
-                } else {
-                    // SAFETY: The above pattern will always match, since we just
-                    // set `self` equal to `Both`.
-                    unsafe { std::hint::unreachable_unchecked() }
-                }
-            }
-        }
-    }
-
-    /// Set `self` to `Both(..)`, containing the specified left and right values,
-    /// and returns a mutable reference to those values.
-    pub fn insert_both(&mut self, left: A, right: B) -> (&mut A, &mut B) {
-        *self = Both(left, right);
-        if let Both(left, right) = self {
-            (left, right)
-        } else {
-            // SAFETY: The above pattern will always match, since we just
-            // set `self` equal to `Both`.
-            unsafe { std::hint::unreachable_unchecked() }
-        }
-    }
 }
 
 impl<T> EitherOrBoth<T, T> {
diff --git a/src/format.rs b/src/format.rs
index c4cb65d..d87cee9 100644
--- a/src/format.rs
+++ b/src/format.rs
@@ -1,5 +1,5 @@
-use std::cell::Cell;
 use std::fmt;
+use std::cell::RefCell;
 
 /// Format all iterator elements lazily, separated by `sep`.
 ///
@@ -7,10 +7,11 @@ use std::fmt;
 /// exhausted.
 ///
 /// See [`.format_with()`](crate::Itertools::format_with) for more information.
+#[derive(Clone)]
 pub struct FormatWith<'a, I, F> {
     sep: &'a str,
     /// FormatWith uses interior mutability because Display::fmt takes &self.
-    inner: Cell<Option<(I, F)>>,
+    inner: RefCell<Option<(I, F)>>,
 }
 
 /// Format all iterator elements lazily, separated by `sep`.
@@ -20,40 +21,38 @@ pub struct FormatWith<'a, I, F> {
 ///
 /// See [`.format()`](crate::Itertools::format)
 /// for more information.
+#[derive(Clone)]
 pub struct Format<'a, I> {
     sep: &'a str,
     /// Format uses interior mutability because Display::fmt takes &self.
-    inner: Cell<Option<I>>,
+    inner: RefCell<Option<I>>,
 }
 
 pub fn new_format<I, F>(iter: I, separator: &str, f: F) -> FormatWith<'_, I, F>
-where
-    I: Iterator,
-    F: FnMut(I::Item, &mut dyn FnMut(&dyn fmt::Display) -> fmt::Result) -> fmt::Result,
+    where I: Iterator,
+          F: FnMut(I::Item, &mut dyn FnMut(&dyn fmt::Display) -> fmt::Result) -> fmt::Result
 {
     FormatWith {
         sep: separator,
-        inner: Cell::new(Some((iter, f))),
+        inner: RefCell::new(Some((iter, f))),
     }
 }
 
 pub fn new_format_default<I>(iter: I, separator: &str) -> Format<'_, I>
-where
-    I: Iterator,
+    where I: Iterator,
 {
     Format {
         sep: separator,
-        inner: Cell::new(Some(iter)),
+        inner: RefCell::new(Some(iter)),
     }
 }
 
 impl<'a, I, F> fmt::Display for FormatWith<'a, I, F>
-where
-    I: Iterator,
-    F: FnMut(I::Item, &mut dyn FnMut(&dyn fmt::Display) -> fmt::Result) -> fmt::Result,
+    where I: Iterator,
+          F: FnMut(I::Item, &mut dyn  FnMut(&dyn fmt::Display) -> fmt::Result) -> fmt::Result
 {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        let (mut iter, mut format) = match self.inner.take() {
+        let (mut iter, mut format) = match self.inner.borrow_mut().take() {
             Some(t) => t,
             None => panic!("FormatWith: was already formatted once"),
         };
@@ -72,15 +71,12 @@ where
 }
 
 impl<'a, I> Format<'a, I>
-where
-    I: Iterator,
+    where I: Iterator,
 {
-    fn format(
-        &self,
-        f: &mut fmt::Formatter,
-        cb: fn(&I::Item, &mut fmt::Formatter) -> fmt::Result,
-    ) -> fmt::Result {
-        let mut iter = match self.inner.take() {
+    fn format<F>(&self, f: &mut fmt::Formatter, mut cb: F) -> fmt::Result
+        where F: FnMut(&I::Item, &mut fmt::Formatter) -> fmt::Result,
+    {
+        let mut iter = match self.inner.borrow_mut().take() {
             Some(t) => t,
             None => panic!("Format: was already formatted once"),
         };
@@ -113,56 +109,5 @@ macro_rules! impl_format {
     }
 }
 
-impl_format! {Display Debug UpperExp LowerExp UpperHex LowerHex Octal Binary Pointer}
-
-impl<'a, I, F> Clone for FormatWith<'a, I, F>
-where
-    (I, F): Clone,
-{
-    fn clone(&self) -> Self {
-        struct PutBackOnDrop<'r, 'a, I, F> {
-            into: &'r FormatWith<'a, I, F>,
-            inner: Option<(I, F)>,
-        }
-        // This ensures we preserve the state of the original `FormatWith` if `Clone` panics
-        impl<'r, 'a, I, F> Drop for PutBackOnDrop<'r, 'a, I, F> {
-            fn drop(&mut self) {
-                self.into.inner.set(self.inner.take())
-            }
-        }
-        let pbod = PutBackOnDrop {
-            inner: self.inner.take(),
-            into: self,
-        };
-        Self {
-            inner: Cell::new(pbod.inner.clone()),
-            sep: self.sep,
-        }
-    }
-}
-
-impl<'a, I> Clone for Format<'a, I>
-where
-    I: Clone,
-{
-    fn clone(&self) -> Self {
-        struct PutBackOnDrop<'r, 'a, I> {
-            into: &'r Format<'a, I>,
-            inner: Option<I>,
-        }
-        // This ensures we preserve the state of the original `FormatWith` if `Clone` panics
-        impl<'r, 'a, I> Drop for PutBackOnDrop<'r, 'a, I> {
-            fn drop(&mut self) {
-                self.into.inner.set(self.inner.take())
-            }
-        }
-        let pbod = PutBackOnDrop {
-            inner: self.inner.take(),
-            into: self,
-        };
-        Self {
-            inner: Cell::new(pbod.inner.clone()),
-            sep: self.sep,
-        }
-    }
-}
+impl_format!{Display Debug
+             UpperExp LowerExp UpperHex LowerHex Octal Binary Pointer}
diff --git a/src/groupbylazy.rs b/src/groupbylazy.rs
index 80c6f09..a5a321d 100644
--- a/src/groupbylazy.rs
+++ b/src/groupbylazy.rs
@@ -19,7 +19,7 @@ impl<A, K, F: ?Sized> KeyFunction<A> for F
 
 
 /// `ChunkIndex` acts like the grouping key function for `IntoChunks`
-#[derive(Debug, Clone)]
+#[derive(Debug)]
 struct ChunkIndex {
     size: usize,
     index: usize,
@@ -50,7 +50,7 @@ impl<A> KeyFunction<A> for ChunkIndex {
     }
 }
 
-#[derive(Clone)]
+
 struct GroupInner<K, I, F>
     where I: Iterator
 {
@@ -471,13 +471,6 @@ pub struct IntoChunks<I>
     index: Cell<usize>,
 }
 
-impl<I> Clone for IntoChunks<I>
-    where I: Clone + Iterator,
-          I::Item: Clone,
-{
-    clone_fields!(inner, index);
-}
-
 
 impl<I> IntoChunks<I>
     where I: Iterator,
@@ -514,7 +507,6 @@ impl<'a, I> IntoIterator for &'a IntoChunks<I>
 ///
 /// See [`.chunks()`](crate::Itertools::chunks) for more information.
 #[must_use = "iterator adaptors are lazy and do nothing unless consumed"]
-#[derive(Clone)]
 pub struct Chunks<'a, I: 'a>
     where I: Iterator,
           I::Item: 'a,
diff --git a/src/lib.rs b/src/lib.rs
index 751448e..340a041 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -1012,7 +1012,10 @@ pub trait Itertools : Iterator {
     /// Create an iterator that merges items from both this and the specified
     /// iterator in ascending order.
     ///
-    /// It chooses whether to pair elements based on the `Ordering` returned by the
+    /// The function can either return an `Ordering` variant or a boolean.
+    ///
+    /// If `cmp_fn` returns `Ordering`,
+    /// it chooses whether to pair elements based on the `Ordering` returned by the
     /// specified compare function. At any point, inspecting the tip of the
     /// iterators `I` and `J` as items `i` of type `I::Item` and `j` of type
     /// `J::Item` respectively, the resulting iterator will:
@@ -1028,18 +1031,46 @@ pub trait Itertools : Iterator {
     /// use itertools::Itertools;
     /// use itertools::EitherOrBoth::{Left, Right, Both};
     ///
-    /// let multiples_of_2 = (0..10).step_by(2);
-    /// let multiples_of_3 = (0..10).step_by(3);
+    /// let a = vec![0, 2, 4, 6, 1].into_iter();
+    /// let b = (0..10).step_by(3);
     ///
     /// itertools::assert_equal(
-    ///     multiples_of_2.merge_join_by(multiples_of_3, |i, j| i.cmp(j)),
-    ///     vec![Both(0, 0), Left(2), Right(3), Left(4), Both(6, 6), Left(8), Right(9)]
+    ///     a.merge_join_by(b, |i, j| i.cmp(j)),
+    ///     vec![Both(0, 0), Left(2), Right(3), Left(4), Both(6, 6), Left(1), Right(9)]
+    /// );
+    /// ```
+    ///
+    /// If `cmp_fn` returns `bool`,
+    /// it chooses whether to pair elements based on the boolean returned by the
+    /// specified function. At any point, inspecting the tip of the
+    /// iterators `I` and `J` as items `i` of type `I::Item` and `j` of type
+    /// `J::Item` respectively, the resulting iterator will:
+    ///
+    /// - Emit `Either::Left(i)` when `true`,
+    ///   and remove `i` from its source iterator
+    /// - Emit `Either::Right(j)` when `false`,
+    ///   and remove `j` from its source iterator
+    ///
+    /// It is similar to the `Ordering` case if the first argument is considered
+    /// "less" than the second argument.
+    ///
+    /// ```
+    /// use itertools::Itertools;
+    /// use itertools::Either::{Left, Right};
+    ///
+    /// let a = vec![0, 2, 4, 6, 1].into_iter();
+    /// let b = (0..10).step_by(3);
+    ///
+    /// itertools::assert_equal(
+    ///     a.merge_join_by(b, |i, j| i <= j),
+    ///     vec![Left(0), Right(0), Left(2), Right(3), Left(4), Left(6), Left(1), Right(6), Right(9)]
     /// );
     /// ```
     #[inline]
-    fn merge_join_by<J, F>(self, other: J, cmp_fn: F) -> MergeJoinBy<Self, J::IntoIter, F>
+    fn merge_join_by<J, F, T>(self, other: J, cmp_fn: F) -> MergeJoinBy<Self, J::IntoIter, F>
         where J: IntoIterator,
-              F: FnMut(&Self::Item, &J::Item) -> std::cmp::Ordering,
+              F: FnMut(&Self::Item, &J::Item) -> T,
+              T: merge_join::OrderingOrBool<Self::Item, J::Item>,
               Self: Sized
     {
         merge_join_by(self, other, cmp_fn)
@@ -1657,24 +1688,24 @@ pub trait Itertools : Iterator {
         pad_tail::pad_using(self, min, f)
     }
 
-    /// Return an iterator adaptor that combines each element with a `Position` to
+    /// Return an iterator adaptor that wraps each element in a `Position` to
     /// ease special-case handling of the first or last elements.
     ///
     /// Iterator element type is
-    /// [`(Position, Self::Item)`](Position)
+    /// [`Position<Self::Item>`](Position)
     ///
     /// ```
     /// use itertools::{Itertools, Position};
     ///
     /// let it = (0..4).with_position();
     /// itertools::assert_equal(it,
-    ///                         vec![(Position::First, 0),
-    ///                              (Position::Middle, 1),
-    ///                              (Position::Middle, 2),
-    ///                              (Position::Last, 3)]);
+    ///                         vec![Position::First(0),
+    ///                              Position::Middle(1),
+    ///                              Position::Middle(2),
+    ///                              Position::Last(3)]);
     ///
     /// let it = (0..1).with_position();
-    /// itertools::assert_equal(it, vec![(Position::Only, 0)]);
+    /// itertools::assert_equal(it, vec![Position::Only(0)]);
     /// ```
     fn with_position(self) -> WithPosition<Self>
         where Self: Sized,
@@ -2549,8 +2580,6 @@ pub trait Itertools : Iterator {
     /// **Note:** This consumes the entire iterator, uses the
     /// [`slice::sort_unstable`] method and returns the result as a new
     /// iterator that owns its elements.
-    /// 
-    /// This sort is unstable (i.e., may reorder equal elements).
     ///
     /// The sorted iterator, if directly collected to a `Vec`, is converted
     /// without any extra copying or allocation cost.
@@ -2580,8 +2609,6 @@ pub trait Itertools : Iterator {
     /// **Note:** This consumes the entire iterator, uses the
     /// [`slice::sort_unstable_by`] method and returns the result as a new
     /// iterator that owns its elements.
-    /// 
-    /// This sort is unstable (i.e., may reorder equal elements).
     ///
     /// The sorted iterator, if directly collected to a `Vec`, is converted
     /// without any extra copying or allocation cost.
@@ -2615,8 +2642,6 @@ pub trait Itertools : Iterator {
     /// **Note:** This consumes the entire iterator, uses the
     /// [`slice::sort_unstable_by_key`] method and returns the result as a new
     /// iterator that owns its elements.
-    /// 
-    /// This sort is unstable (i.e., may reorder equal elements).
     ///
     /// The sorted iterator, if directly collected to a `Vec`, is converted
     /// without any extra copying or allocation cost.
@@ -2651,8 +2676,6 @@ pub trait Itertools : Iterator {
     /// **Note:** This consumes the entire iterator, uses the
     /// [`slice::sort`] method and returns the result as a new
     /// iterator that owns its elements.
-    /// 
-    /// This sort is stable (i.e., does not reorder equal elements).
     ///
     /// The sorted iterator, if directly collected to a `Vec`, is converted
     /// without any extra copying or allocation cost.
@@ -2682,8 +2705,6 @@ pub trait Itertools : Iterator {
     /// **Note:** This consumes the entire iterator, uses the
     /// [`slice::sort_by`] method and returns the result as a new
     /// iterator that owns its elements.
-    /// 
-    /// This sort is stable (i.e., does not reorder equal elements).
     ///
     /// The sorted iterator, if directly collected to a `Vec`, is converted
     /// without any extra copying or allocation cost.
@@ -2692,7 +2713,7 @@ pub trait Itertools : Iterator {
     /// use itertools::Itertools;
     ///
     /// // sort people in descending order by age
-    /// let people = vec![("Jane", 20), ("John", 18), ("Jill", 30), ("Jack", 30)];
+    /// let people = vec![("Jane", 20), ("John", 18), ("Jill", 30), ("Jack", 27)];
     ///
     /// let oldest_people_first = people
     ///     .into_iter()
@@ -2717,8 +2738,6 @@ pub trait Itertools : Iterator {
     /// **Note:** This consumes the entire iterator, uses the
     /// [`slice::sort_by_key`] method and returns the result as a new
     /// iterator that owns its elements.
-    /// 
-    /// This sort is stable (i.e., does not reorder equal elements).
     ///
     /// The sorted iterator, if directly collected to a `Vec`, is converted
     /// without any extra copying or allocation cost.
@@ -2727,7 +2746,7 @@ pub trait Itertools : Iterator {
     /// use itertools::Itertools;
     ///
     /// // sort people in descending order by age
-    /// let people = vec![("Jane", 20), ("John", 18), ("Jill", 30), ("Jack", 30)];
+    /// let people = vec![("Jane", 20), ("John", 18), ("Jill", 30), ("Jack", 27)];
     ///
     /// let oldest_people_first = people
     ///     .into_iter()
@@ -2754,8 +2773,6 @@ pub trait Itertools : Iterator {
     /// **Note:** This consumes the entire iterator, uses the
     /// [`slice::sort_by_cached_key`] method and returns the result as a new
     /// iterator that owns its elements.
-    /// 
-    /// This sort is stable (i.e., does not reorder equal elements).
     ///
     /// The sorted iterator, if directly collected to a `Vec`, is converted
     /// without any extra copying or allocation cost.
@@ -2764,7 +2781,7 @@ pub trait Itertools : Iterator {
     /// use itertools::Itertools;
     ///
     /// // sort people in descending order by age
-    /// let people = vec![("Jane", 20), ("John", 18), ("Jill", 30), ("Jack", 30)];
+    /// let people = vec![("Jane", 20), ("John", 18), ("Jill", 30), ("Jack", 27)];
     ///
     /// let oldest_people_first = people
     ///     .into_iter()
diff --git a/src/merge_join.rs b/src/merge_join.rs
index f2fbdea..84f7d03 100644
--- a/src/merge_join.rs
+++ b/src/merge_join.rs
@@ -2,19 +2,23 @@ use std::cmp::Ordering;
 use std::iter::Fuse;
 use std::fmt;
 
+use either::Either;
+
 use super::adaptors::{PutBack, put_back};
 use crate::either_or_both::EitherOrBoth;
+use crate::size_hint::{self, SizeHint};
 #[cfg(doc)]
 use crate::Itertools;
 
 /// Return an iterator adaptor that merge-joins items from the two base iterators in ascending order.
 ///
 /// [`IntoIterator`] enabled version of [`Itertools::merge_join_by`].
-pub fn merge_join_by<I, J, F>(left: I, right: J, cmp_fn: F)
+pub fn merge_join_by<I, J, F, T>(left: I, right: J, cmp_fn: F)
     -> MergeJoinBy<I::IntoIter, J::IntoIter, F>
     where I: IntoIterator,
           J: IntoIterator,
-          F: FnMut(&I::Item, &J::Item) -> Ordering
+          F: FnMut(&I::Item, &J::Item) -> T,
+          T: OrderingOrBool<I::Item, J::Item>,
 {
     MergeJoinBy {
         left: put_back(left.into_iter().fuse()),
@@ -30,7 +34,66 @@ pub fn merge_join_by<I, J, F>(left: I, right: J, cmp_fn: F)
 pub struct MergeJoinBy<I: Iterator, J: Iterator, F> {
     left: PutBack<Fuse<I>>,
     right: PutBack<Fuse<J>>,
-    cmp_fn: F
+    cmp_fn: F,
+}
+
+pub trait OrderingOrBool<L, R> {
+    type MergeResult;
+    fn left(left: L) -> Self::MergeResult;
+    fn right(right: R) -> Self::MergeResult;
+    // "merge" never returns (Some(...), Some(...), ...) so Option<Either<I::Item, J::Item>>
+    // is appealing but it is always followed by two put_backs, so we think the compiler is
+    // smart enough to optimize it. Or we could move put_backs into "merge".
+    fn merge(self, left: L, right: R) -> (Option<L>, Option<R>, Self::MergeResult);
+    fn size_hint(left: SizeHint, right: SizeHint) -> SizeHint;
+}
+
+impl<L, R> OrderingOrBool<L, R> for Ordering {
+    type MergeResult = EitherOrBoth<L, R>;
+    fn left(left: L) -> Self::MergeResult {
+        EitherOrBoth::Left(left)
+    }
+    fn right(right: R) -> Self::MergeResult {
+        EitherOrBoth::Right(right)
+    }
+    fn merge(self, left: L, right: R) -> (Option<L>, Option<R>, Self::MergeResult) {
+        match self {
+            Ordering::Equal => (None, None, EitherOrBoth::Both(left, right)),
+            Ordering::Less => (None, Some(right), EitherOrBoth::Left(left)),
+            Ordering::Greater => (Some(left), None, EitherOrBoth::Right(right)),
+        }
+    }
+    fn size_hint(left: SizeHint, right: SizeHint) -> SizeHint {
+        let (a_lower, a_upper) = left;
+        let (b_lower, b_upper) = right;
+        let lower = ::std::cmp::max(a_lower, b_lower);
+        let upper = match (a_upper, b_upper) {
+            (Some(x), Some(y)) => x.checked_add(y),
+            _ => None,
+        };
+        (lower, upper)
+    }
+}
+
+impl<L, R> OrderingOrBool<L, R> for bool {
+    type MergeResult = Either<L, R>;
+    fn left(left: L) -> Self::MergeResult {
+        Either::Left(left)
+    }
+    fn right(right: R) -> Self::MergeResult {
+        Either::Right(right)
+    }
+    fn merge(self, left: L, right: R) -> (Option<L>, Option<R>, Self::MergeResult) {
+        if self {
+            (None, Some(right), Either::Left(left))
+        } else {
+            (Some(left), None, Either::Right(right))
+        }
+    }
+    fn size_hint(left: SizeHint, right: SizeHint) -> SizeHint {
+        // Not ExactSizeIterator because size may be larger than usize
+        size_hint::add(left, right)
+    }
 }
 
 impl<I, J, F> Clone for MergeJoinBy<I, J, F>
@@ -52,49 +115,34 @@ impl<I, J, F> fmt::Debug for MergeJoinBy<I, J, F>
     debug_fmt_fields!(MergeJoinBy, left, right);
 }
 
-impl<I, J, F> Iterator for MergeJoinBy<I, J, F>
+impl<I, J, F, T> Iterator for MergeJoinBy<I, J, F>
     where I: Iterator,
           J: Iterator,
-          F: FnMut(&I::Item, &J::Item) -> Ordering
+          F: FnMut(&I::Item, &J::Item) -> T,
+          T: OrderingOrBool<I::Item, J::Item>,
 {
-    type Item = EitherOrBoth<I::Item, J::Item>;
+    type Item = T::MergeResult;
 
     fn next(&mut self) -> Option<Self::Item> {
         match (self.left.next(), self.right.next()) {
             (None, None) => None,
-            (Some(left), None) =>
-                Some(EitherOrBoth::Left(left)),
-            (None, Some(right)) =>
-                Some(EitherOrBoth::Right(right)),
+            (Some(left), None) => Some(T::left(left)),
+            (None, Some(right)) => Some(T::right(right)),
             (Some(left), Some(right)) => {
-                match (self.cmp_fn)(&left, &right) {
-                    Ordering::Equal =>
-                        Some(EitherOrBoth::Both(left, right)),
-                    Ordering::Less => {
-                        self.right.put_back(right);
-                        Some(EitherOrBoth::Left(left))
-                    },
-                    Ordering::Greater => {
-                        self.left.put_back(left);
-                        Some(EitherOrBoth::Right(right))
-                    }
+                let (left, right, next) = (self.cmp_fn)(&left, &right).merge(left, right);
+                if let Some(left) = left {
+                    self.left.put_back(left);
+                }
+                if let Some(right) = right {
+                    self.right.put_back(right);
                 }
+                Some(next)
             }
         }
     }
 
-    fn size_hint(&self) -> (usize, Option<usize>) {
-        let (a_lower, a_upper) = self.left.size_hint();
-        let (b_lower, b_upper) = self.right.size_hint();
-
-        let lower = ::std::cmp::max(a_lower, b_lower);
-
-        let upper = match (a_upper, b_upper) {
-            (Some(x), Some(y)) => x.checked_add(y),
-            _ => None,
-        };
-
-        (lower, upper)
+    fn size_hint(&self) -> SizeHint {
+        T::size_hint(self.left.size_hint(), self.right.size_hint())
     }
 
     fn count(mut self) -> usize {
@@ -106,10 +154,12 @@ impl<I, J, F> Iterator for MergeJoinBy<I, J, F>
                 (None, Some(_right)) => break count + 1 + self.right.into_parts().1.count(),
                 (Some(left), Some(right)) => {
                     count += 1;
-                    match (self.cmp_fn)(&left, &right) {
-                        Ordering::Equal => {}
-                        Ordering::Less => self.right.put_back(right),
-                        Ordering::Greater => self.left.put_back(left),
+                    let (left, right, _) = (self.cmp_fn)(&left, &right).merge(left, right);
+                    if let Some(left) = left {
+                        self.left.put_back(left);
+                    }
+                    if let Some(right) = right {
+                        self.right.put_back(right);
                     }
                 }
             }
@@ -122,27 +172,24 @@ impl<I, J, F> Iterator for MergeJoinBy<I, J, F>
             match (self.left.next(), self.right.next()) {
                 (None, None) => break previous_element,
                 (Some(left), None) => {
-                    break Some(EitherOrBoth::Left(
+                    break Some(T::left(
                         self.left.into_parts().1.last().unwrap_or(left),
                     ))
                 }
                 (None, Some(right)) => {
-                    break Some(EitherOrBoth::Right(
+                    break Some(T::right(
                         self.right.into_parts().1.last().unwrap_or(right),
                     ))
                 }
                 (Some(left), Some(right)) => {
-                    previous_element = match (self.cmp_fn)(&left, &right) {
-                        Ordering::Equal => Some(EitherOrBoth::Both(left, right)),
-                        Ordering::Less => {
-                            self.right.put_back(right);
-                            Some(EitherOrBoth::Left(left))
-                        }
-                        Ordering::Greater => {
-                            self.left.put_back(left);
-                            Some(EitherOrBoth::Right(right))
-                        }
+                    let (left, right, elem) = (self.cmp_fn)(&left, &right).merge(left, right);
+                    if let Some(left) = left {
+                        self.left.put_back(left);
+                    }
+                    if let Some(right) = right {
+                        self.right.put_back(right);
                     }
+                    previous_element = Some(elem);
                 }
             }
         }
@@ -156,13 +203,17 @@ impl<I, J, F> Iterator for MergeJoinBy<I, J, F>
             n -= 1;
             match (self.left.next(), self.right.next()) {
                 (None, None) => break None,
-                (Some(_left), None) => break self.left.nth(n).map(EitherOrBoth::Left),
-                (None, Some(_right)) => break self.right.nth(n).map(EitherOrBoth::Right),
-                (Some(left), Some(right)) => match (self.cmp_fn)(&left, &right) {
-                    Ordering::Equal => {}
-                    Ordering::Less => self.right.put_back(right),
-                    Ordering::Greater => self.left.put_back(left),
-                },
+                (Some(_left), None) => break self.left.nth(n).map(T::left),
+                (None, Some(_right)) => break self.right.nth(n).map(T::right),
+                (Some(left), Some(right)) => {
+                    let (left, right, _) = (self.cmp_fn)(&left, &right).merge(left, right);
+                    if let Some(left) = left {
+                        self.left.put_back(left);
+                    }
+                    if let Some(right) = right {
+                        self.right.put_back(right);
+                    }
+                }
             }
         }
     }
diff --git a/src/tuple_impl.rs b/src/tuple_impl.rs
index fdf0865..f4488db 100644
--- a/src/tuple_impl.rs
+++ b/src/tuple_impl.rs
@@ -201,7 +201,7 @@ impl<I, T> FusedIterator for TupleWindows<I, T>
 /// See [`.circular_tuple_windows()`](crate::Itertools::circular_tuple_windows) for more
 /// information.
 #[must_use = "iterator adaptors are lazy and do nothing unless consumed"]
-#[derive(Debug, Clone)]
+#[derive(Debug)]
 pub struct CircularTupleWindows<I, T: Clone>
     where I: Iterator<Item = T::Item> + Clone,
           T: TupleCollect + Clone
diff --git a/src/with_position.rs b/src/with_position.rs
index dda9b25..1388503 100644
--- a/src/with_position.rs
+++ b/src/with_position.rs
@@ -2,7 +2,7 @@ use std::iter::{Fuse,Peekable, FusedIterator};
 
 /// An iterator adaptor that wraps each element in an [`Position`].
 ///
-/// Iterator element type is `(Position, I::Item)`.
+/// Iterator element type is `Position<I::Item>`.
 ///
 /// See [`.with_position()`](crate::Itertools::with_position) for more information.
 #[must_use = "iterator adaptors are lazy and do nothing unless consumed"]
@@ -30,24 +30,36 @@ pub fn with_position<I>(iter: I) -> WithPosition<I>
     }
 }
 
-/// The first component of the value yielded by `WithPosition`.
+/// A value yielded by `WithPosition`.
 /// Indicates the position of this element in the iterator results.
 ///
 /// See [`.with_position()`](crate::Itertools::with_position) for more information.
 #[derive(Copy, Clone, Debug, PartialEq)]
-pub enum Position {
+pub enum Position<T> {
     /// This is the first element.
-    First,
+    First(T),
     /// This is neither the first nor the last element.
-    Middle,
+    Middle(T),
     /// This is the last element.
-    Last,
+    Last(T),
     /// This is the only element.
-    Only,
+    Only(T),
+}
+
+impl<T> Position<T> {
+    /// Return the inner value.
+    pub fn into_inner(self) -> T {
+        match self {
+            Position::First(x) |
+            Position::Middle(x) |
+            Position::Last(x) |
+            Position::Only(x) => x,
+        }
+    }
 }
 
 impl<I: Iterator> Iterator for WithPosition<I> {
-    type Item = (Position, I::Item);
+    type Item = Position<I::Item>;
 
     fn next(&mut self) -> Option<Self::Item> {
         match self.peekable.next() {
@@ -58,15 +70,15 @@ impl<I: Iterator> Iterator for WithPosition<I> {
                     // Peek to see if this is also the last item,
                     // in which case tag it as `Only`.
                     match self.peekable.peek() {
-                        Some(_) => Some((Position::First, item)),
-                        None => Some((Position::Only, item)),
+                        Some(_) => Some(Position::First(item)),
+                        None => Some(Position::Only(item)),
                     }
                 } else {
                     // Have seen the first item, and there's something left.
                     // Peek to see if this is the last item.
                     match self.peekable.peek() {
-                        Some(_) => Some((Position::Middle, item)),
-                        None => Some((Position::Last, item)),
+                        Some(_) => Some(Position::Middle(item)),
+                        None => Some(Position::Last(item)),
                     }
                 }
             }


Test patch:
diff --git a/tests/quick.rs b/tests/quick.rs
index b00c73b..914960a 100644
--- a/tests/quick.rs
+++ b/tests/quick.rs
@@ -829,6 +829,31 @@ quickcheck! {
     }
 }
 
+quickcheck! {
+    fn merge_join_by_ordering_vs_bool(a: Vec<u8>, b: Vec<u8>) -> bool {
+        use either::Either;
+        use itertools::free::merge_join_by;
+        let mut has_equal = false;
+        let it_ord = merge_join_by(a.clone(), b.clone(), Ord::cmp).flat_map(|v| match v {
+            EitherOrBoth::Both(l, r) => {
+                has_equal = true;
+                vec![Either::Left(l), Either::Right(r)]
+            }
+            EitherOrBoth::Left(l) => vec![Either::Left(l)],
+            EitherOrBoth::Right(r) => vec![Either::Right(r)],
+        });
+        let it_bool = merge_join_by(a, b, PartialOrd::le);
+        itertools::equal(it_ord, it_bool) || has_equal
+    }
+    fn merge_join_by_bool_unwrapped_is_merge_by(a: Vec<u8>, b: Vec<u8>) -> bool {
+        use either::Either;
+        use itertools::free::merge_join_by;
+        let it = a.clone().into_iter().merge_by(b.clone(), PartialOrd::ge);
+        let it_join = merge_join_by(a, b, PartialOrd::ge).map(Either::into_inner);
+        itertools::equal(it, it_join)
+    }
+}
+
 quickcheck! {
     fn size_tee(a: Vec<u8>) -> bool {
         let (mut t1, mut t2) = a.iter().tee();
@@ -993,17 +1018,6 @@ quickcheck! {
     }
 }
 
-quickcheck! {
-    fn chunk_clone_equal(a: Vec<u8>, size: u8) -> () {
-        let mut size = size;
-        if size == 0 {
-            size += 1;
-        }
-        let it = a.chunks(size as usize);
-        itertools::assert_equal(it.clone(), it);
-    }
-}
-
 quickcheck! {
     fn equal_chunks_lazy(a: Vec<u8>, size: u8) -> bool {
         let mut size = size;
@@ -1021,71 +1035,7 @@ quickcheck! {
     }
 }
 
-// tuple iterators
 quickcheck! {
-    fn equal_circular_tuple_windows_1(a: Vec<u8>) -> bool {
-        let x = a.iter().map(|e| (e,) );
-        let y = a.iter().circular_tuple_windows::<(_,)>();
-        itertools::assert_equal(x,y);
-        true
-    }
-
-    fn equal_circular_tuple_windows_2(a: Vec<u8>) -> bool {
-        let x = (0..a.len()).map(|start_idx| (
-            &a[start_idx],
-            &a[(start_idx + 1) % a.len()],
-        ));
-        let y = a.iter().circular_tuple_windows::<(_, _)>();
-        itertools::assert_equal(x,y);
-        true
-    }
-
-    fn equal_circular_tuple_windows_3(a: Vec<u8>) -> bool {
-        let x = (0..a.len()).map(|start_idx| (
-            &a[start_idx],
-            &a[(start_idx + 1) % a.len()],
-            &a[(start_idx + 2) % a.len()],
-        ));
-        let y = a.iter().circular_tuple_windows::<(_, _, _)>();
-        itertools::assert_equal(x,y);
-        true
-    }
-
-    fn equal_circular_tuple_windows_4(a: Vec<u8>) -> bool {
-        let x = (0..a.len()).map(|start_idx| (
-            &a[start_idx],
-            &a[(start_idx + 1) % a.len()],
-            &a[(start_idx + 2) % a.len()],
-            &a[(start_idx + 3) % a.len()],
-        ));
-        let y = a.iter().circular_tuple_windows::<(_, _, _, _)>();
-        itertools::assert_equal(x,y);
-        true
-    }
-
-    fn equal_cloned_circular_tuple_windows(a: Vec<u8>) -> bool {
-        let x = a.iter().circular_tuple_windows::<(_, _, _, _)>();
-        let y = x.clone();
-        itertools::assert_equal(x,y);
-        true
-    }
-
-    fn equal_cloned_circular_tuple_windows_noninitial(a: Vec<u8>) -> bool {
-        let mut x = a.iter().circular_tuple_windows::<(_, _, _, _)>();
-        let _ = x.next();
-        let y = x.clone();
-        itertools::assert_equal(x,y);
-        true
-    }
-
-    fn equal_cloned_circular_tuple_windows_complete(a: Vec<u8>) -> bool {
-        let mut x = a.iter().circular_tuple_windows::<(_, _, _, _)>();
-        for _ in x.by_ref() {}
-        let y = x.clone();
-        itertools::assert_equal(x,y);
-        true
-    }
-
     fn equal_tuple_windows_1(a: Vec<u8>) -> bool {
         let x = a.windows(1).map(|s| (&s[0], ));
         let y = a.iter().tuple_windows::<(_,)>();