rust: Generalize the `trim_end_newlines` utility of byte strings

… into `trim_end_matches` that takes a callack.

Also add `trim_start_matches`.

Differential Revision: https://phab.mercurial-scm.org/D11388

rust/hg-core/src/repo.rs

@@ -1,315 +1,316
 use crate::config::{Config, ConfigError, ConfigParseError};
 use crate::errors::{HgError, IoErrorContext, IoResultExt};
 use crate::exit_codes;
 use crate::requirements;
 use crate::utils::files::get_path_from_bytes;
 use crate::utils::SliceExt;
 use memmap::{Mmap, MmapOptions};
 use std::collections::HashSet;
 use std::io::ErrorKind;
 use std::path::{Path, PathBuf};
 
 /// A repository on disk
 pub struct Repo {
     working_directory: PathBuf,
     dot_hg: PathBuf,
     store: PathBuf,
     requirements: HashSet<String>,
     config: Config,
 }
 
 #[derive(Debug, derive_more::From)]
 pub enum RepoError {
     NotFound {
         at: PathBuf,
     },
     #[from]
     ConfigParseError(ConfigParseError),
     #[from]
     Other(HgError),
 }
 
 impl From<ConfigError> for RepoError {
     fn from(error: ConfigError) -> Self {
         match error {
             ConfigError::Parse(error) => error.into(),
             ConfigError::Other(error) => error.into(),
         }
     }
 }
 
 /// Filesystem access abstraction for the contents of a given "base" diretory
 #[derive(Clone, Copy)]
 pub struct Vfs<'a> {
     pub(crate) base: &'a Path,
 }
 
 impl Repo {
     /// tries to find nearest repository root in current working directory or
     /// its ancestors
     pub fn find_repo_root() -> Result<PathBuf, RepoError> {
         let current_directory = crate::utils::current_dir()?;
         // ancestors() is inclusive: it first yields `current_directory`
         // as-is.
         for ancestor in current_directory.ancestors() {
             if is_dir(ancestor.join(".hg"))? {
                 return Ok(ancestor.to_path_buf());
             }
         }
         return Err(RepoError::NotFound {
             at: current_directory,
         });
     }
 
     /// Find a repository, either at the given path (which must contain a `.hg`
     /// sub-directory) or by searching the current directory and its
     /// ancestors.
     ///
     /// A method with two very different "modes" like this usually a code smell
     /// to make two methods instead, but in this case an `Option` is what rhg
     /// sub-commands get from Clap for the `-R` / `--repository` CLI argument.
     /// Having two methods would just move that `if` to almost all callers.
     pub fn find(
         config: &Config,
         explicit_path: Option<PathBuf>,
     ) -> Result<Self, RepoError> {
         if let Some(root) = explicit_path {
             if is_dir(root.join(".hg"))? {
                 Self::new_at_path(root.to_owned(), config)
             } else if is_file(&root)? {
                 Err(HgError::unsupported("bundle repository").into())
             } else {
                 Err(RepoError::NotFound {
                     at: root.to_owned(),
                 })
             }
         } else {
             let root = Self::find_repo_root()?;
             Self::new_at_path(root, config)
         }
     }
 
     /// To be called after checking that `.hg` is a sub-directory
     fn new_at_path(
         working_directory: PathBuf,
         config: &Config,
     ) -> Result<Self, RepoError> {
         let dot_hg = working_directory.join(".hg");
 
         let mut repo_config_files = Vec::new();
         repo_config_files.push(dot_hg.join("hgrc"));
         repo_config_files.push(dot_hg.join("hgrc-not-shared"));
 
         let hg_vfs = Vfs { base: &dot_hg };
         let mut reqs = requirements::load_if_exists(hg_vfs)?;
         let relative =
             reqs.contains(requirements::RELATIVE_SHARED_REQUIREMENT);
         let shared =
             reqs.contains(requirements::SHARED_REQUIREMENT) || relative;
 
         // From `mercurial/localrepo.py`:
         //
         // if .hg/requires contains the sharesafe requirement, it means
         // there exists a `.hg/store/requires` too and we should read it
         // NOTE: presence of SHARESAFE_REQUIREMENT imply that store requirement
         // is present. We never write SHARESAFE_REQUIREMENT for a repo if store
         // is not present, refer checkrequirementscompat() for that
         //
         // However, if SHARESAFE_REQUIREMENT is not present, it means that the
         // repository was shared the old way. We check the share source
         // .hg/requires for SHARESAFE_REQUIREMENT to detect whether the
         // current repository needs to be reshared
         let share_safe = reqs.contains(requirements::SHARESAFE_REQUIREMENT);
 
         let store_path;
         if !shared {
             store_path = dot_hg.join("store");
         } else {
             let bytes = hg_vfs.read("sharedpath")?;
             let mut shared_path =
-                get_path_from_bytes(bytes.trim_end_newlines()).to_owned();
+                get_path_from_bytes(bytes.trim_end_matches(|b| b == b'\n'))
+                    .to_owned();
             if relative {
                 shared_path = dot_hg.join(shared_path)
             }
             if !is_dir(&shared_path)? {
                 return Err(HgError::corrupted(format!(
                     ".hg/sharedpath points to nonexistent directory {}",
                     shared_path.display()
                 ))
                 .into());
             }
 
             store_path = shared_path.join("store");
 
             let source_is_share_safe =
                 requirements::load(Vfs { base: &shared_path })?
                     .contains(requirements::SHARESAFE_REQUIREMENT);
 
             if share_safe && !source_is_share_safe {
                 return Err(match config
                     .get(b"share", b"safe-mismatch.source-not-safe")
                 {
                     Some(b"abort") | None => HgError::abort(
                         "abort: share source does not support share-safe requirement\n\
                         (see `hg help config.format.use-share-safe` for more information)",
                         exit_codes::ABORT,
                     ),
                     _ => HgError::unsupported("share-safe downgrade"),
                 }
                 .into());
             } else if source_is_share_safe && !share_safe {
                 return Err(
                     match config.get(b"share", b"safe-mismatch.source-safe") {
                         Some(b"abort") | None => HgError::abort(
                             "abort: version mismatch: source uses share-safe \
                             functionality while the current share does not\n\
                             (see `hg help config.format.use-share-safe` for more information)",
                         exit_codes::ABORT,
                         ),
                         _ => HgError::unsupported("share-safe upgrade"),
                     }
                     .into(),
                 );
             }
 
             if share_safe {
                 repo_config_files.insert(0, shared_path.join("hgrc"))
             }
         }
         if share_safe {
             reqs.extend(requirements::load(Vfs { base: &store_path })?);
         }
 
         let repo_config = if std::env::var_os("HGRCSKIPREPO").is_none() {
             config.combine_with_repo(&repo_config_files)?
         } else {
             config.clone()
         };
 
         let repo = Self {
             requirements: reqs,
             working_directory,
             store: store_path,
             dot_hg,
             config: repo_config,
         };
 
         requirements::check(&repo)?;
 
         Ok(repo)
     }
 
     pub fn working_directory_path(&self) -> &Path {
         &self.working_directory
     }
 
     pub fn requirements(&self) -> &HashSet<String> {
         &self.requirements
     }
 
     pub fn config(&self) -> &Config {
         &self.config
     }
 
     /// For accessing repository files (in `.hg`), except for the store
     /// (`.hg/store`).
     pub fn hg_vfs(&self) -> Vfs<'_> {
         Vfs { base: &self.dot_hg }
     }
 
     /// For accessing repository store files (in `.hg/store`)
     pub fn store_vfs(&self) -> Vfs<'_> {
         Vfs { base: &self.store }
     }
 
     /// For accessing the working copy
     pub fn working_directory_vfs(&self) -> Vfs<'_> {
         Vfs {
             base: &self.working_directory,
         }
     }
 
     pub fn has_dirstate_v2(&self) -> bool {
         self.requirements
             .contains(requirements::DIRSTATE_V2_REQUIREMENT)
     }
 
     pub fn dirstate_parents(
         &self,
     ) -> Result<crate::dirstate::DirstateParents, HgError> {
         let dirstate = self.hg_vfs().mmap_open("dirstate")?;
         if dirstate.is_empty() {
             return Ok(crate::dirstate::DirstateParents::NULL);
         }
         let parents = if self.has_dirstate_v2() {
             crate::dirstate_tree::on_disk::read_docket(&dirstate)?.parents()
         } else {
             crate::dirstate::parsers::parse_dirstate_parents(&dirstate)?
                 .clone()
         };
         Ok(parents)
     }
 }
 
 impl Vfs<'_> {
     pub fn join(&self, relative_path: impl AsRef<Path>) -> PathBuf {
         self.base.join(relative_path)
     }
 
     pub fn read(
         &self,
         relative_path: impl AsRef<Path>,
     ) -> Result<Vec<u8>, HgError> {
         let path = self.join(relative_path);
         std::fs::read(&path).when_reading_file(&path)
     }
 
     pub fn mmap_open(
         &self,
         relative_path: impl AsRef<Path>,
     ) -> Result<Mmap, HgError> {
         let path = self.base.join(relative_path);
         let file = std::fs::File::open(&path).when_reading_file(&path)?;
         // TODO: what are the safety requirements here?
         let mmap = unsafe { MmapOptions::new().map(&file) }
             .when_reading_file(&path)?;
         Ok(mmap)
     }
 
     pub fn rename(
         &self,
         relative_from: impl AsRef<Path>,
         relative_to: impl AsRef<Path>,
     ) -> Result<(), HgError> {
         let from = self.join(relative_from);
         let to = self.join(relative_to);
         std::fs::rename(&from, &to)
             .with_context(|| IoErrorContext::RenamingFile { from, to })
     }
 }
 
 fn fs_metadata(
     path: impl AsRef<Path>,
 ) -> Result<Option<std::fs::Metadata>, HgError> {
     let path = path.as_ref();
     match std::fs::metadata(path) {
         Ok(meta) => Ok(Some(meta)),
         Err(error) => match error.kind() {
             // TODO: when we require a Rust version where `NotADirectory` is
             // stable, invert this logic and return None for it and `NotFound`
             // and propagate any other error.
             ErrorKind::PermissionDenied => Err(error).with_context(|| {
                 IoErrorContext::ReadingMetadata(path.to_owned())
             }),
             _ => Ok(None),
         },
     }
 }
 
 fn is_dir(path: impl AsRef<Path>) -> Result<bool, HgError> {
     Ok(fs_metadata(path)?.map_or(false, |meta| meta.is_dir()))
 }
 
 fn is_file(path: impl AsRef<Path>) -> Result<bool, HgError> {
     Ok(fs_metadata(path)?.map_or(false, |meta| meta.is_file()))
 }

rust/hg-core/src/utils.rs

@@ -1,483 +1,481
 // utils module
 //
 // Copyright 2019 Raphaël Gomès <[e-mail]>
 //
 // This software may be used and distributed according to the terms of the
 // GNU General Public License version 2 or any later version.
 
 //! Contains useful functions, traits, structs, etc. for use in core.
 
 use crate::errors::{HgError, IoErrorContext};
 use crate::utils::hg_path::HgPath;
 use im_rc::ordmap::DiffItem;
 use im_rc::ordmap::OrdMap;
 use std::cell::Cell;
 use std::fmt;
 use std::{io::Write, ops::Deref};
 
 pub mod files;
 pub mod hg_path;
 pub mod path_auditor;
 
 /// Useful until rust/issues/56345 is stable
 ///
 /// # Examples
 ///
 /// ```
 /// use crate::hg::utils::find_slice_in_slice;
 ///
 /// let haystack = b"This is the haystack".to_vec();
 /// assert_eq!(find_slice_in_slice(&haystack, b"the"), Some(8));
 /// assert_eq!(find_slice_in_slice(&haystack, b"not here"), None);
 /// ```
 pub fn find_slice_in_slice<T>(slice: &[T], needle: &[T]) -> Option<usize>
 where
     for<'a> &'a [T]: PartialEq,
 {
     slice
         .windows(needle.len())
         .position(|window| window == needle)
 }
 
 /// Replaces the `from` slice with the `to` slice inside the `buf` slice.
 ///
 /// # Examples
 ///
 /// ```
 /// use crate::hg::utils::replace_slice;
 /// let mut line = b"I hate writing tests!".to_vec();
 /// replace_slice(&mut line, b"hate", b"love");
 /// assert_eq!(
 ///     line,
 ///     b"I love writing tests!".to_vec()
 /// );
 /// ```
 pub fn replace_slice<T>(buf: &mut [T], from: &[T], to: &[T])
 where
     T: Clone + PartialEq,
 {
     if buf.len() < from.len() || from.len() != to.len() {
         return;
     }
     for i in 0..=buf.len() - from.len() {
         if buf[i..].starts_with(from) {
             buf[i..(i + from.len())].clone_from_slice(to);
         }
     }
 }
 
 pub trait SliceExt {
-    fn trim_end_newlines(&self) -> &Self;
     fn trim_end(&self) -> &Self;
     fn trim_start(&self) -> &Self;
+    fn trim_end_matches(&self, f: impl FnMut(u8) -> bool) -> &Self;
+    fn trim_start_matches(&self, f: impl FnMut(u8) -> bool) -> &Self;
     fn trim(&self) -> &Self;
     fn drop_prefix(&self, needle: &Self) -> Option<&Self>;
     fn split_2(&self, separator: u8) -> Option<(&[u8], &[u8])>;
 }
 
-#[allow(clippy::trivially_copy_pass_by_ref)]
-fn is_not_whitespace(c: &u8) -> bool {
-    !(*c as char).is_whitespace()
-}
+impl SliceExt for [u8] {
+    fn trim_end(&self) -> &[u8] {
+        self.trim_end_matches(|byte| byte.is_ascii_whitespace())
+    }
 
-impl SliceExt for [u8] {
-    fn trim_end_newlines(&self) -> &[u8] {
-        if let Some(last) = self.iter().rposition(|&byte| byte != b'\n') {
+    fn trim_start(&self) -> &[u8] {
+        self.trim_start_matches(|byte| byte.is_ascii_whitespace())
+    }
+
+    fn trim_end_matches(&self, mut f: impl FnMut(u8) -> bool) -> &Self {
+        if let Some(last) = self.iter().rposition(|&byte| !f(byte)) {
             &self[..=last]
         } else {
             &[]
         }
     }
-    fn trim_end(&self) -> &[u8] {
-        if let Some(last) = self.iter().rposition(is_not_whitespace) {
-            &self[..=last]
-        } else {
-            &[]
-        }
-    }
-    fn trim_start(&self) -> &[u8] {
-        if let Some(first) = self.iter().position(is_not_whitespace) {
+
+    fn trim_start_matches(&self, mut f: impl FnMut(u8) -> bool) -> &Self {
+        if let Some(first) = self.iter().position(|&byte| !f(byte)) {
             &self[first..]
         } else {
             &[]
         }
     }
 
     /// ```
     /// use hg::utils::SliceExt;
     /// assert_eq!(
     ///     b"  to trim  ".trim(),
     ///     b"to trim"
     /// );
     /// assert_eq!(
     ///     b"to trim  ".trim(),
     ///     b"to trim"
     /// );
     /// assert_eq!(
     ///     b"  to trim".trim(),
     ///     b"to trim"
     /// );
     /// ```
     fn trim(&self) -> &[u8] {
         self.trim_start().trim_end()
     }
 
     fn drop_prefix(&self, needle: &Self) -> Option<&Self> {
         if self.starts_with(needle) {
             Some(&self[needle.len()..])
         } else {
             None
         }
     }
 
     fn split_2(&self, separator: u8) -> Option<(&[u8], &[u8])> {
         let mut iter = self.splitn(2, |&byte| byte == separator);
         let a = iter.next()?;
         let b = iter.next()?;
         Some((a, b))
     }
 }
 
 pub trait Escaped {
     /// Return bytes escaped for display to the user
     fn escaped_bytes(&self) -> Vec<u8>;
 }
 
 impl Escaped for u8 {
     fn escaped_bytes(&self) -> Vec<u8> {
         let mut acc = vec![];
         match self {
             c @ b'\'' | c @ b'\\' => {
                 acc.push(b'\\');
                 acc.push(*c);
             }
             b'\t' => {
                 acc.extend(br"\\t");
             }
             b'\n' => {
                 acc.extend(br"\\n");
             }
             b'\r' => {
                 acc.extend(br"\\r");
             }
             c if (*c < b' ' || *c >= 127) => {
                 write!(acc, "\\x{:x}", self).unwrap();
             }
             c => {
                 acc.push(*c);
             }
         }
         acc
     }
 }
 
 impl<'a, T: Escaped> Escaped for &'a [T] {
     fn escaped_bytes(&self) -> Vec<u8> {
         self.iter().flat_map(Escaped::escaped_bytes).collect()
     }
 }
 
 impl<T: Escaped> Escaped for Vec<T> {
     fn escaped_bytes(&self) -> Vec<u8> {
         self.deref().escaped_bytes()
     }
 }
 
 impl<'a> Escaped for &'a HgPath {
     fn escaped_bytes(&self) -> Vec<u8> {
         self.as_bytes().escaped_bytes()
     }
 }
 
 // TODO: use the str method when we require Rust 1.45
 pub(crate) fn strip_suffix<'a>(s: &'a str, suffix: &str) -> Option<&'a str> {
     if s.ends_with(suffix) {
         Some(&s[..s.len() - suffix.len()])
     } else {
         None
     }
 }
 
 #[cfg(unix)]
 pub fn shell_quote(value: &[u8]) -> Vec<u8> {
     // TODO: Use the `matches!` macro when we require Rust 1.42+
     if value.iter().all(|&byte| match byte {
         b'a'..=b'z'
         | b'A'..=b'Z'
         | b'0'..=b'9'
         | b'.'
         | b'_'
         | b'/'
         | b'+'
         | b'-' => true,
         _ => false,
     }) {
         value.to_owned()
     } else {
         let mut quoted = Vec::with_capacity(value.len() + 2);
         quoted.push(b'\'');
         for &byte in value {
             if byte == b'\'' {
                 quoted.push(b'\\');
             }
             quoted.push(byte);
         }
         quoted.push(b'\'');
         quoted
     }
 }
 
 pub fn current_dir() -> Result<std::path::PathBuf, HgError> {
     std::env::current_dir().map_err(|error| HgError::IoError {
         error,
         context: IoErrorContext::CurrentDir,
     })
 }
 
 pub fn current_exe() -> Result<std::path::PathBuf, HgError> {
     std::env::current_exe().map_err(|error| HgError::IoError {
         error,
         context: IoErrorContext::CurrentExe,
     })
 }
 
 /// Expand `$FOO` and `${FOO}` environment variables in the given byte string
 pub fn expand_vars(s: &[u8]) -> std::borrow::Cow<[u8]> {
     lazy_static::lazy_static! {
         /// https://github.com/python/cpython/blob/3.9/Lib/posixpath.py#L301
         /// The `x` makes whitespace ignored.
         /// `-u` disables the Unicode flag, which makes `\w` like Python with the ASCII flag.
         static ref VAR_RE: regex::bytes::Regex =
             regex::bytes::Regex::new(r"(?x-u)
                 \$
                 (?:
                     (\w+)
                     |
                     \{
                         ([^}]*)
                     \}
                 )
             ").unwrap();
     }
     VAR_RE.replace_all(s, |captures: &regex::bytes::Captures| {
         let var_name = files::get_os_str_from_bytes(
             captures
                 .get(1)
                 .or_else(|| captures.get(2))
                 .expect("either side of `|` must participate in match")
                 .as_bytes(),
         );
         std::env::var_os(var_name)
             .map(files::get_bytes_from_os_str)
             .unwrap_or_else(|| {
                 // Referencing an environment variable that does not exist.
                 // Leave the $FOO reference as-is.
                 captures[0].to_owned()
             })
     })
 }
 
 #[test]
 fn test_expand_vars() {
     // Modifying process-global state in a test isn’t great,
     // but hopefully this won’t collide with anything.
     std::env::set_var("TEST_EXPAND_VAR", "1");
     assert_eq!(
         expand_vars(b"before/$TEST_EXPAND_VAR/after"),
         &b"before/1/after"[..]
     );
     assert_eq!(
         expand_vars(b"before${TEST_EXPAND_VAR}${TEST_EXPAND_VAR}${TEST_EXPAND_VAR}after"),
         &b"before111after"[..]
     );
     let s = b"before $SOME_LONG_NAME_THAT_WE_ASSUME_IS_NOT_AN_ACTUAL_ENV_VAR after";
     assert_eq!(expand_vars(s), &s[..]);
 }
 
 pub(crate) enum MergeResult<V> {
     UseLeftValue,
     UseRightValue,
     UseNewValue(V),
 }
 
 /// Return the union of the two given maps,
 /// calling `merge(key, left_value, right_value)` to resolve keys that exist in
 /// both.
 ///
 /// CC https://github.com/bodil/im-rs/issues/166
 pub(crate) fn ordmap_union_with_merge<K, V>(
     left: OrdMap<K, V>,
     right: OrdMap<K, V>,
     mut merge: impl FnMut(&K, &V, &V) -> MergeResult<V>,
 ) -> OrdMap<K, V>
 where
     K: Clone + Ord,
     V: Clone + PartialEq,
 {
     if left.ptr_eq(&right) {
         // One of the two maps is an unmodified clone of the other
         left
     } else if left.len() / 2 > right.len() {
         // When two maps have different sizes,
         // their size difference is a lower bound on
         // how many keys of the larger map are not also in the smaller map.
         // This in turn is a lower bound on the number of differences in
         // `OrdMap::diff` and the "amount of work" that would be done
         // by `ordmap_union_with_merge_by_diff`.
         //
         // Here `left` is more than twice the size of `right`,
         // so the number of differences is more than the total size of
         // `right`. Therefore an algorithm based on iterating `right`
         // is more efficient.
         //
         // This helps a lot when a tiny (or empty) map is merged
         // with a large one.
         ordmap_union_with_merge_by_iter(left, right, merge)
     } else if left.len() < right.len() / 2 {
         // Same as above but with `left` and `right` swapped
         ordmap_union_with_merge_by_iter(right, left, |key, a, b| {
             // Also swapped in `merge` arguments:
             match merge(key, b, a) {
                 MergeResult::UseNewValue(v) => MergeResult::UseNewValue(v),
                 // … and swap back in `merge` result:
                 MergeResult::UseLeftValue => MergeResult::UseRightValue,
                 MergeResult::UseRightValue => MergeResult::UseLeftValue,
             }
         })
     } else {
         // For maps of similar size, use the algorithm based on `OrdMap::diff`
         ordmap_union_with_merge_by_diff(left, right, merge)
     }
 }
 
 /// Efficient if `right` is much smaller than `left`
 fn ordmap_union_with_merge_by_iter<K, V>(
     mut left: OrdMap<K, V>,
     right: OrdMap<K, V>,
     mut merge: impl FnMut(&K, &V, &V) -> MergeResult<V>,
 ) -> OrdMap<K, V>
 where
     K: Clone + Ord,
     V: Clone,
 {
     for (key, right_value) in right {
         match left.get(&key) {
             None => {
                 left.insert(key, right_value);
             }
             Some(left_value) => match merge(&key, left_value, &right_value) {
                 MergeResult::UseLeftValue => {}
                 MergeResult::UseRightValue => {
                     left.insert(key, right_value);
                 }
                 MergeResult::UseNewValue(new_value) => {
                     left.insert(key, new_value);
                 }
             },
         }
     }
     left
 }
 
 /// Fallback when both maps are of similar size
 fn ordmap_union_with_merge_by_diff<K, V>(
     mut left: OrdMap<K, V>,
     mut right: OrdMap<K, V>,
     mut merge: impl FnMut(&K, &V, &V) -> MergeResult<V>,
 ) -> OrdMap<K, V>
 where
     K: Clone + Ord,
     V: Clone + PartialEq,
 {
     // (key, value) pairs that would need to be inserted in either map
     // in order to turn it into the union.
     //
     // TODO: if/when https://github.com/bodil/im-rs/pull/168 is accepted,
     // change these from `Vec<(K, V)>` to `Vec<(&K, Cow<V>)>`
     // with `left_updates` only borrowing from `right` and `right_updates` from
     // `left`, and with `Cow::Owned` used for `MergeResult::UseNewValue`.
     //
     // This would allow moving all `.clone()` calls to after we’ve decided
     // which of `right_updates` or `left_updates` to use
     // (value ones becoming `Cow::into_owned`),
     // and avoid making clones we don’t end up using.
     let mut left_updates = Vec::new();
     let mut right_updates = Vec::new();
 
     for difference in left.diff(&right) {
         match difference {
             DiffItem::Add(key, value) => {
                 left_updates.push((key.clone(), value.clone()))
             }
             DiffItem::Remove(key, value) => {
                 right_updates.push((key.clone(), value.clone()))
             }
             DiffItem::Update {
                 old: (key, left_value),
                 new: (_, right_value),
             } => match merge(key, left_value, right_value) {
                 MergeResult::UseLeftValue => {
                     right_updates.push((key.clone(), left_value.clone()))
                 }
                 MergeResult::UseRightValue => {
                     left_updates.push((key.clone(), right_value.clone()))
                 }
                 MergeResult::UseNewValue(new_value) => {
                     left_updates.push((key.clone(), new_value.clone()));
                     right_updates.push((key.clone(), new_value))
                 }
             },
         }
     }
     if left_updates.len() < right_updates.len() {
         for (key, value) in left_updates {
             left.insert(key, value);
         }
         left
     } else {
         for (key, value) in right_updates {
             right.insert(key, value);
         }
         right
     }
 }
 
 /// Join items of the iterable with the given separator, similar to Python’s
 /// `separator.join(iter)`.
 ///
 /// Formatting the return value consumes the iterator.
 /// Formatting it again will produce an empty string.
 pub fn join_display(
     iter: impl IntoIterator<Item = impl fmt::Display>,
     separator: impl fmt::Display,
 ) -> impl fmt::Display {
     JoinDisplay {
         iter: Cell::new(Some(iter.into_iter())),
         separator,
     }
 }
 
 struct JoinDisplay<I, S> {
     iter: Cell<Option<I>>,
     separator: S,
 }
 
 impl<I, T, S> fmt::Display for JoinDisplay<I, S>
 where
     I: Iterator<Item = T>,
     T: fmt::Display,
     S: fmt::Display,
 {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         if let Some(mut iter) = self.iter.take() {
             if let Some(first) = iter.next() {
                 first.fmt(f)?;
             }
             for value in iter {
                 self.separator.fmt(f)?;
                 value.fmt(f)?;
             }
         }
         Ok(())
     }
 }