D12168: [WIP] rhg: Colorize `rhg status` output when appropriate

[SimonSapin]

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/D12168

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

CHANGE DETAILS

diff --git a/rust/rhg/src/ui.rs b/rust/rhg/src/ui.rs
--- a/rust/rhg/src/ui.rs
+++ b/rust/rhg/src/ui.rs
@@ -57,11 +57,6 @@
 
     /// Write bytes to stdout
     pub fn write_stdout(&self, bytes: &[u8]) -> Result<(), UiError> {
-        // Hack to silence "unused" warnings
-        if false {
-            return self.write_stdout_labelled(bytes, "");
-        }
-
         let mut stdout = self.stdout.lock();
 
         stdout.write_all(bytes).or_else(handle_stdout_error)?;
@@ -233,7 +228,7 @@
         if let Some(effects) = self.styles.get(label.as_bytes()) {
             let mut iter = effects.iter();
             if let Some(first) = iter.next() {
-                write_bytes!(stream, b"\0o33[{}", first)?;
+                write_bytes!(stream, b"{}[{}", ASCII_ESCAPE, first)?;
                 for next in iter {
                     write_bytes!(stream, b";{}", next)?;
                 }
@@ -250,13 +245,16 @@
     ) -> io::Result<()> {
         if let Some(effects) = self.styles.get(label.as_bytes()) {
             if !effects.is_empty() {
-                write_bytes!(stream, b"\0o33[0m")?;
+                write_bytes!(stream, b"{}[0m", ASCII_ESCAPE)?;
             }
         }
         Ok(())
     }
 }
 
+/// 0x1B == 27 == 0o33
+const ASCII_ESCAPE: &[u8] = b"\x1b";
+
 enum ColorMode {
     // TODO: support other modes
     Ansi,
diff --git a/rust/rhg/src/main.rs b/rust/rhg/src/main.rs
--- a/rust/rhg/src/main.rs
+++ b/rust/rhg/src/main.rs
@@ -29,7 +29,7 @@
     repo: Result<&Repo, &NoRepoInCwdError>,
     config: &Config,
 ) -> Result<(), CommandError> {
-    check_unsupported(config, repo, ui)?;
+    check_unsupported(config, repo)?;
 
     let app = App::new("rhg")
         .global_setting(AppSettings::AllowInvalidUtf8)
@@ -678,7 +678,6 @@
 fn check_unsupported(
     config: &Config,
     repo: Result<&Repo, &NoRepoInCwdError>,
-    ui: &ui::Ui,
 ) -> Result<(), CommandError> {
     check_extensions(config)?;
 
@@ -702,13 +701,5 @@
         Err(CommandError::unsupported("[decode] config"))?
     }
 
-    if let Some(color) = config.get(b"ui", b"color") {
-        if (color == b"always" || color == b"debug")
-            && !ui.plain(Some("color"))
-        {
-            Err(CommandError::unsupported("colored output"))?
-        }
-    }
-
     Ok(())
 }
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
@@ -326,25 +326,25 @@
         },
     };
     if display_states.modified {
-        output.display(b"M", ds_status.modified)?;
+        output.display(b"M", "status.modified", ds_status.modified)?;
     }
     if display_states.added {
-        output.display(b"A", ds_status.added)?;
+        output.display(b"A", "status.added", ds_status.added)?;
     }
     if display_states.removed {
-        output.display(b"R", ds_status.removed)?;
+        output.display(b"R", "status.removed", ds_status.removed)?;
     }
     if display_states.deleted {
-        output.display(b"!", ds_status.deleted)?;
+        output.display(b"!", "status.deleted", ds_status.deleted)?;
     }
     if display_states.unknown {
-        output.display(b"?", ds_status.unknown)?;
+        output.display(b"?", "status.unknown", ds_status.unknown)?;
     }
     if display_states.ignored {
-        output.display(b"I", ds_status.ignored)?;
+        output.display(b"I", "status.ignored", ds_status.ignored)?;
     }
     if display_states.clean {
-        output.display(b"C", ds_status.clean)?;
+        output.display(b"C", "status.clean", ds_status.clean)?;
     }
 
     let mut dirstate_write_needed = ds_status.dirty;
@@ -448,6 +448,7 @@
     fn display(
         &self,
         status_prefix: &[u8],
+        label: &'static str,
         mut paths: Vec<StatusPath<'_>>,
     ) -> Result<(), CommandError> {
         paths.sort_unstable();
@@ -459,22 +460,26 @@
             } else {
                 path.as_bytes()
             };
-            // TODO optim, probably lots of unneeded copies here, especially
-            // if out stream is buffered
+            // TODO: Add a way to use `write_bytes!` instead of `format_bytes!`
+            // in order to stream to stdout instead of allocating an
+            // itermediate `Vec<u8>`.
             if self.no_status {
-                self.ui.write_stdout(&format_bytes!(b"{}\n", path))?
+                self.ui.write_stdout_labelled(
+                    &format_bytes!(b"{}\n", path),
+                    label,
+                )?
             } else {
-                self.ui.write_stdout(&format_bytes!(
-                    b"{} {}\n",
-                    status_prefix,
-                    path
-                ))?
+                self.ui.write_stdout_labelled(
+                    &format_bytes!(b"{} {}\n", status_prefix, path),
+                    label,
+                )?
             }
             if let Some(source) = copy_source {
-                self.ui.write_stdout(&format_bytes!(
-                    b"  {}\n",
-                    source.as_bytes()
-                ))?
+                let label = "status.copied";
+                self.ui.write_stdout_labelled(
+                    &format_bytes!(b"  {}\n", source.as_bytes()),
+                    label,
+                )?
             }
         }
         Ok(())



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