( )⚙ D12168 rhg: Colorize `rhg status` output when appropriate

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D12168

rhg: Colorize `rhg status` output when appropriate
ClosedPublic

Authored by SimonSapin on Feb 10 2022, 2:18 PM.

Details

Reviewers
Alphare
Group Reviewers
hg-reviewers
Commits
rHG3e2b4bb286e7: rhg: Colorize `rhg status` output when appropriate

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

SimonSapin created this revision.Feb 10 2022, 2:18 PM
SimonSapin added a comment.Feb 10 2022, 2:21 PM

This will likely still need minor changes to make a test pass: https://foss.heptapod.net/mercurial/mercurial-devel/-/jobs/340039
This produces output that looks correct when rendered by a terminal, but escape sequences are not exactly where the test expects them.

SimonSapin updated this revision to Diff 32146.Feb 11 2022, 6:42 AM
SimonSapin retitled this revision from [WIP] rhg: Colorize `rhg status` output when appropriate to rhg: Colorize `rhg status` output when appropriate.Feb 11 2022, 11:35 AM
SimonSapin updated this revision to Diff 32149.
Alphare accepted this revision.Feb 12 2022, 7:05 PM
Alphare added a subscriber: Alphare.
Alphare added inline comments.
tests/test-status-color.t
315 ↗(On Diff #32149)

Arguably a feature to not spam the user multiple times

This revision is now accepted and ready to land.Feb 12 2022, 7:05 PM
SimonSapin updated this revision to Diff 32154.Feb 13 2022, 6:06 AM
SimonSapin added a commit: rHG3e2b4bb286e7: rhg: Colorize `rhg status` output when appropriate.Feb 14 2022, 5:25 AM
Closed by commit rHG3e2b4bb286e7: rhg: Colorize `rhg status` output when appropriate (authored by SimonSapin). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mrust/rhg/src/commands/status.rs (42 lines)
Mrust/rhg/src/main.rs (11 lines)
Mrust/rhg/src/ui.rs (5 lines)

Diff 32146

rust/rhg/src/commands/status.rs

no_status, no_status,
relativize: if relative_paths { relativize: if relative_paths {
Some(RelativizePaths::new(repo)?) Some(RelativizePaths::new(repo)?)
} else { } else {
None None
}, },
}; };
if display_states.modified { if display_states.modified {
output.display(b"M", ds_status.modified)?; output.display(b"M ", "status.modified", ds_status.modified)?;
} }
if display_states.added { if display_states.added {
output.display(b"A", ds_status.added)?; output.display(b"A ", "status.added", ds_status.added)?;
} }
if display_states.removed { if display_states.removed {
output.display(b"R", ds_status.removed)?; output.display(b"R ", "status.removed", ds_status.removed)?;
} }
if display_states.deleted { if display_states.deleted {
output.display(b"!", ds_status.deleted)?; output.display(b"! ", "status.deleted", ds_status.deleted)?;
} }
if display_states.unknown { if display_states.unknown {
output.display(b"?", ds_status.unknown)?; output.display(b"? ", "status.unknown", ds_status.unknown)?;
} }
if display_states.ignored { if display_states.ignored {
output.display(b"I", ds_status.ignored)?; output.display(b"I ", "status.ignored", ds_status.ignored)?;
} }
if display_states.clean { 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; let mut dirstate_write_needed = ds_status.dirty;
let filesystem_time_at_status_start = let filesystem_time_at_status_start =
ds_status.filesystem_time_at_status_start; ds_status.filesystem_time_at_status_start;
if (fixup.is_empty() || filesystem_time_at_status_start.is_none()) if (fixup.is_empty() || filesystem_time_at_status_start.is_none())
&& !dirstate_write_needed && !dirstate_write_needed
} }
impl DisplayStatusPaths<'_> { impl DisplayStatusPaths<'_> {
// Probably more elegant to use a Deref or Borrow trait rather than // Probably more elegant to use a Deref or Borrow trait rather than
// harcode HgPathBuf, but probably not really useful at this point // harcode HgPathBuf, but probably not really useful at this point
fn display( fn display(
&self, &self,
status_prefix: &[u8], status_prefix: &[u8],
label: &'static str,
mut paths: Vec<StatusPath<'_>>, mut paths: Vec<StatusPath<'_>>,
) -> Result<(), CommandError> { ) -> Result<(), CommandError> {
paths.sort_unstable(); paths.sort_unstable();
// TODO:!get the stdout lock once for the whole loop instead of in each write
for StatusPath { path, copy_source } in paths { for StatusPath { path, copy_source } in paths {
let relative; let relative;
let path = if let Some(relativize) = &self.relativize { let path = if let Some(relativize) = &self.relativize {
relative = relativize.relativize(&path); relative = relativize.relativize(&path);
&*relative &*relative
} else { } else {
path.as_bytes() path.as_bytes()
}; };
// TODO optim, probably lots of unneeded copies here, especially // TODO: Add a way to use `write_bytes!` instead of `format_bytes!`
// if out stream is buffered // in order to stream to stdout instead of allocating an
if self.no_status { // itermediate `Vec<u8>`.
self.ui.write_stdout(&format_bytes!(b"{}\n", path))? if !self.no_status {
} else { self.ui.write_stdout_labelled(status_prefix, label)?
self.ui.write_stdout(&format_bytes!(
b"{} {}\n",
status_prefix,
path
))?
} }
self.ui
.write_stdout_labelled(&format_bytes!(b"{}\n", path), label)?;
if let Some(source) = copy_source { if let Some(source) = copy_source {
self.ui.write_stdout(&format_bytes!( let label = "status.copied";
b" {}\n", self.ui.write_stdout_labelled(
source.as_bytes() &format_bytes!(b" {}\n", source.as_bytes()),
))? label,
)?
} }
} }
Ok(()) Ok(())
} }
} }
/// Check if a file is modified by comparing actual repo store and file system. /// Check if a file is modified by comparing actual repo store and file system.
/// ///

rust/rhg/src/main.rs

} }
fn main_with_result( fn main_with_result(
process_start_time: &blackbox::ProcessStartTime, process_start_time: &blackbox::ProcessStartTime,
ui: &ui::Ui, ui: &ui::Ui,
repo: Result<&Repo, &NoRepoInCwdError>, repo: Result<&Repo, &NoRepoInCwdError>,
config: &Config, config: &Config,
) -> Result<(), CommandError> { ) -> Result<(), CommandError> {
check_unsupported(config, repo, ui)?; check_unsupported(config, repo)?;
let app = App::new("rhg") let app = App::new("rhg")
.global_setting(AppSettings::AllowInvalidUtf8) .global_setting(AppSettings::AllowInvalidUtf8)
.global_setting(AppSettings::DisableVersion) .global_setting(AppSettings::DisableVersion)
.setting(AppSettings::SubcommandRequired) .setting(AppSettings::SubcommandRequired)
.setting(AppSettings::VersionlessSubcommands) .setting(AppSettings::VersionlessSubcommands)
.arg( .arg(
Arg::with_name("repository") Arg::with_name("repository")
), ),
}) })
} }
} }
fn check_unsupported( fn check_unsupported(
config: &Config, config: &Config,
repo: Result<&Repo, &NoRepoInCwdError>, repo: Result<&Repo, &NoRepoInCwdError>,
ui: &ui::Ui,
) -> Result<(), CommandError> { ) -> Result<(), CommandError> {
check_extensions(config)?; check_extensions(config)?;
if std::env::var_os("HG_PENDING").is_some() { if std::env::var_os("HG_PENDING").is_some() {
// TODO: only if the value is `== repo.working_directory`? // TODO: only if the value is `== repo.working_directory`?
// What about relative v.s. absolute paths? // What about relative v.s. absolute paths?
Err(CommandError::unsupported("$HG_PENDING"))? Err(CommandError::unsupported("$HG_PENDING"))?
} }
if let Ok(repo) = repo { if let Ok(repo) = repo {
if repo.has_subrepos()? { if repo.has_subrepos()? {
Err(CommandError::unsupported("sub-repositories"))? Err(CommandError::unsupported("sub-repositories"))?
} }
} }
if config.has_non_empty_section(b"encode") { if config.has_non_empty_section(b"encode") {
Err(CommandError::unsupported("[encode] config"))? Err(CommandError::unsupported("[encode] config"))?
} }
if config.has_non_empty_section(b"decode") { if config.has_non_empty_section(b"decode") {
Err(CommandError::unsupported("[decode] config"))? 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(()) Ok(())
} }

rust/rhg/src/ui.rs

/// Returns a buffered handle on stdout for faster batch printing /// Returns a buffered handle on stdout for faster batch printing
/// operations. /// operations.
pub fn stdout_buffer(&self) -> StdoutBuffer<std::io::StdoutLock> { pub fn stdout_buffer(&self) -> StdoutBuffer<std::io::StdoutLock> {
StdoutBuffer::new(self.stdout.lock()) StdoutBuffer::new(self.stdout.lock())
} }
/// Write bytes to stdout /// Write bytes to stdout
pub fn write_stdout(&self, bytes: &[u8]) -> Result<(), UiError> { 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(); let mut stdout = self.stdout.lock();
stdout.write_all(bytes).or_else(handle_stdout_error)?; stdout.write_all(bytes).or_else(handle_stdout_error)?;
stdout.flush().or_else(handle_stdout_error) stdout.flush().or_else(handle_stdout_error)
} }
/// Write bytes to stderr /// Write bytes to stderr