Patchwork D11897: rhg: refactor display_status_paths with a struct for common arguments

login
register
mail settings
Submitter phabricator
Date Dec. 10, 2021, 10:14 p.m.
Message ID <differential-rev-PHID-DREV-yzmpps3m4spfh36uzouf-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/50223/
State New
Headers show

Comments

phabricator - Dec. 10, 2021, 10:14 p.m.
SimonSapin created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D11897

AFFECTED FILES
  rust/rhg/src/commands/status.rs

CHANGE DETAILS




To: SimonSapin, #hg-reviewers
Cc: mercurial-patches, mercurial-devel

Patch

diff --git a/rust/rhg/src/commands/status.rs b/rust/rhg/src/commands/status.rs
--- a/rust/rhg/src/commands/status.rs
+++ b/rust/rhg/src/commands/status.rs
@@ -255,75 +255,36 @@ 
             }
         }
     }
+    let relative_paths = (!ui.plain())
+        && config
+            .get_option(b"commands", b"status.relative")?
+            .unwrap_or(config.get_bool(b"ui", b"relative-paths")?);
+    let output = DisplayStatusPaths {
+        ui,
+        repo,
+        no_status,
+        relative_paths,
+    };
     if display_states.modified {
-        display_status_paths(
-            ui,
-            repo,
-            config,
-            no_status,
-            &mut ds_status.modified,
-            b"M",
-        )?;
+        output.display(b"M", ds_status.modified)?;
     }
     if display_states.added {
-        display_status_paths(
-            ui,
-            repo,
-            config,
-            no_status,
-            &mut ds_status.added,
-            b"A",
-        )?;
+        output.display(b"A", ds_status.added)?;
     }
     if display_states.removed {
-        display_status_paths(
-            ui,
-            repo,
-            config,
-            no_status,
-            &mut ds_status.removed,
-            b"R",
-        )?;
+        output.display(b"R", ds_status.removed)?;
     }
     if display_states.deleted {
-        display_status_paths(
-            ui,
-            repo,
-            config,
-            no_status,
-            &mut ds_status.deleted,
-            b"!",
-        )?;
+        output.display(b"!", ds_status.deleted)?;
     }
     if display_states.unknown {
-        display_status_paths(
-            ui,
-            repo,
-            config,
-            no_status,
-            &mut ds_status.unknown,
-            b"?",
-        )?;
+        output.display(b"?", ds_status.unknown)?;
     }
     if display_states.ignored {
-        display_status_paths(
-            ui,
-            repo,
-            config,
-            no_status,
-            &mut ds_status.ignored,
-            b"I",
-        )?;
+        output.display(b"I", ds_status.ignored)?;
     }
     if display_states.clean {
-        display_status_paths(
-            ui,
-            repo,
-            config,
-            no_status,
-            &mut ds_status.clean,
-            b"C",
-        )?;
+        output.display(b"C", ds_status.clean)?;
     }
 
     let mut dirstate_write_needed = ds_status.dirty;
@@ -416,41 +377,47 @@ 
     ignore_files
 }
 
-// Probably more elegant to use a Deref or Borrow trait rather than
-// harcode HgPathBuf, but probably not really useful at this point
-fn display_status_paths(
-    ui: &Ui,
-    repo: &Repo,
-    config: &Config,
+struct DisplayStatusPaths<'a> {
+    ui: &'a Ui,
+    repo: &'a Repo,
     no_status: bool,
-    paths: &mut [HgPathCow],
-    status_prefix: &[u8],
-) -> Result<(), CommandError> {
-    paths.sort_unstable();
-    let mut relative: bool = config.get_bool(b"ui", b"relative-paths")?;
-    relative = config
-        .get_option(b"commands", b"status.relative")?
-        .unwrap_or(relative);
-    let print_path = |path: &[u8]| {
-        // TODO optim, probably lots of unneeded copies here, especially
-        // if out stream is buffered
-        if no_status {
-            ui.write_stdout(&format_bytes!(b"{}\n", path))
+    relative_paths: bool,
+}
+
+impl DisplayStatusPaths<'_> {
+    // Probably more elegant to use a Deref or Borrow trait rather than
+    // harcode HgPathBuf, but probably not really useful at this point
+    fn display(
+        &self,
+        status_prefix: &[u8],
+        mut paths: Vec<HgPathCow>,
+    ) -> Result<(), CommandError> {
+        paths.sort_unstable();
+        let print_path = |path: &[u8]| {
+            // TODO optim, probably lots of unneeded copies here, especially
+            // if out stream is buffered
+            if self.no_status {
+                self.ui.write_stdout(&format_bytes!(b"{}\n", path))
+            } else {
+                self.ui.write_stdout(&format_bytes!(
+                    b"{} {}\n",
+                    status_prefix,
+                    path
+                ))
+            }
+        };
+
+        if self.relative_paths {
+            relativize_paths(self.repo, paths.iter().map(Ok), |path| {
+                print_path(&path)
+            })?;
         } else {
-            ui.write_stdout(&format_bytes!(b"{} {}\n", status_prefix, path))
+            for path in paths {
+                print_path(path.as_bytes())?
+            }
         }
-    };
-
-    if relative && !ui.plain() {
-        relativize_paths(repo, paths.iter().map(Ok), |path| {
-            print_path(&path)
-        })?;
-    } else {
-        for path in paths {
-            print_path(path.as_bytes())?
-        }
+        Ok(())
     }
-    Ok(())
 }
 
 /// Check if a file is modified by comparing actual repo store and file system.