This is an archive of the discontinued Mercurial Phabricator instance.

Differential D12166

rhg: Add parsing for the --color global CLI argument
ClosedPublic

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

Details

Reviewers
Alphare
Group Reviewers
hg-reviewers
Commits
rHGd4a5c2197208: rhg: Add parsing for the --color global CLI argument

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

SimonSapin created this revision.Feb 10 2022, 2:18 PM
Alphare accepted this revision.Feb 12 2022, 6:29 PM
Alphare added a subscriber: Alphare.
Alphare added inline comments.
rust/hg-core/src/config/layer.rs
307–308

This TODO is out-of-date now

This revision is now accepted and ready to land.Feb 12 2022, 6:29 PM
SimonSapin added inline comments.Feb 13 2022, 5:30 AM
rust/hg-core/src/config/layer.rs
307–308

Right, I’ll remove the CLI one.

The others may still be relevant. Maybe not defaults if we keep the current pattern of .get(…).unwrap_or(default_value) at use sites instead of centralized defaults definition.

Alphare added inline comments.Feb 13 2022, 5:53 AM
rust/hg-core/src/config/layer.rs
307–308

I think the others are still relevant. We should think about configitems.py at some point, so let's keep it.

SimonSapin updated this revision to Diff 32152.Feb 13 2022, 6:06 AM
SimonSapin added a commit: rHGd4a5c2197208: rhg: Add parsing for the --color global CLI argument.Feb 14 2022, 5:24 AM
Closed by commit rHGd4a5c2197208: rhg: Add parsing for the --color global CLI argument (authored by SimonSapin). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 32158

rust/hg-core/src/config/config.rs

config.add_trusted_file(&path)? config.add_trusted_file(&path)?
} }
} }
} }
} }
Ok(config) Ok(config)
} }
pub fn load_cli_args_config( pub fn load_cli_args(
&mut self, &mut self,
cli_config_args: impl IntoIterator<Item = impl AsRef<[u8]>>, cli_config_args: impl IntoIterator<Item = impl AsRef<[u8]>>,
color_arg: Option<Vec<u8>>,
) -> Result<(), ConfigError> { ) -> Result<(), ConfigError> {
if let Some(layer) = ConfigLayer::parse_cli_args(cli_config_args)? { if let Some(layer) = ConfigLayer::parse_cli_args(cli_config_args)? {
self.layers.push(layer) self.layers.push(layer)
} }
if let Some(arg) = color_arg {
let mut layer = ConfigLayer::new(ConfigOrigin::CommandLineColor);
layer.add(b"ui"[..].into(), b"color"[..].into(), arg, None);
self.layers.push(layer)
}
Ok(()) Ok(())
} }
fn add_trusted_dir(&mut self, path: &Path) -> Result<(), ConfigError> { fn add_trusted_dir(&mut self, path: &Path) -> Result<(), ConfigError> {
if let Some(entries) = std::fs::read_dir(path) if let Some(entries) = std::fs::read_dir(path)
.when_reading_file(path) .when_reading_file(path)
.io_not_found_as_none()? .io_not_found_as_none()?
{ {

rust/hg-core/src/config/layer.rs

} }
#[derive(Clone, Debug)] #[derive(Clone, Debug)]
pub enum ConfigOrigin { pub enum ConfigOrigin {
/// From a configuration file /// From a configuration file
File(PathBuf), File(PathBuf),
/// From a `--config` CLI argument /// From a `--config` CLI argument
CommandLine, CommandLine,
/// From a `--color` CLI argument
CommandLineColor,
/// From environment variables like `$PAGER` or `$EDITOR` /// From environment variables like `$PAGER` or `$EDITOR`
Environment(Vec<u8>), Environment(Vec<u8>),
/* TODO cli /* TODO defaults (configitems.py)
AlphareUnsubmitted
Not Done

This TODO is out-of-date now

Alphare: This TODO is out-of-date now
SimonSapinAuthorUnsubmitted
Done

Right, I’ll remove the CLI one.

The others may still be relevant. Maybe not defaults if we keep the current pattern of .get(…).unwrap_or(default_value) at use sites instead of centralized defaults definition.

SimonSapin: Right, I’ll remove the CLI one. The others may still be relevant. Maybe not defaults if we…
AlphareUnsubmitted
Not Done

I think the others are still relevant. We should think about configitems.py at some point, so let's keep it.

Alphare: I think the others are still relevant. We should think about `configitems.py` at some point, so…
* TODO defaults (configitems.py)
* TODO extensions * TODO extensions
* TODO Python resources? * TODO Python resources?
* Others? */ * Others? */
} }
impl DisplayBytes for ConfigOrigin { impl DisplayBytes for ConfigOrigin {
fn display_bytes( fn display_bytes(
&self, &self,
out: &mut dyn std::io::Write, out: &mut dyn std::io::Write,
) -> std::io::Result<()> { ) -> std::io::Result<()> {
match self { match self {
ConfigOrigin::File(p) => out.write_all(&get_bytes_from_path(p)), ConfigOrigin::File(p) => out.write_all(&get_bytes_from_path(p)),
ConfigOrigin::CommandLine => out.write_all(b"--config"), ConfigOrigin::CommandLine => out.write_all(b"--config"),
ConfigOrigin::CommandLineColor => out.write_all(b"--color"),
ConfigOrigin::Environment(e) => write_bytes!(out, b"${}", e), ConfigOrigin::Environment(e) => write_bytes!(out, b"${}", e),
} }
} }
} }
#[derive(Debug)] #[derive(Debug)]
pub struct ConfigParseError { pub struct ConfigParseError {
pub origin: ConfigOrigin, pub origin: ConfigOrigin,

rust/rhg/src/main.rs

.arg( .arg(
Arg::with_name("cwd") Arg::with_name("cwd")
.help("change working directory") .help("change working directory")
.long("--cwd") .long("--cwd")
.value_name("DIR") .value_name("DIR")
.takes_value(true) .takes_value(true)
.global(true), .global(true),
) )
.arg(
Arg::with_name("color")
.help("when to colorize (boolean, always, auto, never, or debug)")
.long("--color")
.value_name("TYPE")
.takes_value(true)
.global(true),
)
.version("0.0.1"); .version("0.0.1");
let app = add_subcommand_args(app); let app = add_subcommand_args(app);
let matches = app.clone().get_matches_safe()?; let matches = app.clone().get_matches_safe()?;
let (subcommand_name, subcommand_matches) = matches.subcommand(); let (subcommand_name, subcommand_matches) = matches.subcommand();
// Mercurial allows users to define "defaults" for commands, fallback // Mercurial allows users to define "defaults" for commands, fallback
&Ui::new_infallible(&Config::empty()), &Ui::new_infallible(&Config::empty()),
on_unsupported, on_unsupported,
Err(error.into()), Err(error.into()),
false, false,
) )
}); });
non_repo_config non_repo_config
.load_cli_args_config(early_args.config) .load_cli_args(early_args.config, early_args.color)
.unwrap_or_else(|error| { .unwrap_or_else(|error| {
exit( exit(
&initial_current_dir, &initial_current_dir,
&Ui::new_infallible(&non_repo_config), &Ui::new_infallible(&non_repo_config),
OnUnsupported::from_config(&non_repo_config), OnUnsupported::from_config(&non_repo_config),
Err(error.into()), Err(error.into()),
non_repo_config non_repo_config
.get_bool(b"ui", b"detailed-exit-code") .get_bool(b"ui", b"detailed-exit-code")
/// configuration before using Clap. Ideally we would also use Clap for this, /// configuration before using Clap. Ideally we would also use Clap for this,
/// see <https://github.com/clap-rs/clap/discussions/2366>. /// see <https://github.com/clap-rs/clap/discussions/2366>.
/// ///
/// These arguments are still declared when we do use Clap later, so that Clap /// These arguments are still declared when we do use Clap later, so that Clap
/// does not return an error for their presence. /// does not return an error for their presence.
struct EarlyArgs { struct EarlyArgs {
/// Values of all `--config` arguments. (Possibly none) /// Values of all `--config` arguments. (Possibly none)
config: Vec<Vec<u8>>, config: Vec<Vec<u8>>,
/// Value of all the `--color` argument, if any.
color: Option<Vec<u8>>,
/// Value of the `-R` or `--repository` argument, if any. /// Value of the `-R` or `--repository` argument, if any.
repo: Option<Vec<u8>>, repo: Option<Vec<u8>>,
/// Value of the `--cwd` argument, if any. /// Value of the `--cwd` argument, if any.
cwd: Option<Vec<u8>>, cwd: Option<Vec<u8>>,
} }
impl EarlyArgs { impl EarlyArgs {
fn parse(args: impl IntoIterator<Item = OsString>) -> Self { fn parse(args: impl IntoIterator<Item = OsString>) -> Self {
let mut args = args.into_iter().map(get_bytes_from_os_str); let mut args = args.into_iter().map(get_bytes_from_os_str);
let mut config = Vec::new(); let mut config = Vec::new();
let mut color = None;
let mut repo = None; let mut repo = None;
let mut cwd = None; let mut cwd = None;
// Use `while let` instead of `for` so that we can also call // Use `while let` instead of `for` so that we can also call
// `args.next()` inside the loop. // `args.next()` inside the loop.
while let Some(arg) = args.next() { while let Some(arg) = args.next() {
if arg == b"--config" { if arg == b"--config" {
if let Some(value) = args.next() { if let Some(value) = args.next() {
config.push(value) config.push(value)
} }
} else if let Some(value) = arg.drop_prefix(b"--config=") { } else if let Some(value) = arg.drop_prefix(b"--config=") {
config.push(value.to_owned()) config.push(value.to_owned())
} }
if arg == b"--color" {
if let Some(value) = args.next() {
color = Some(value)
}
} else if let Some(value) = arg.drop_prefix(b"--color=") {
color = Some(value.to_owned())
}
if arg == b"--cwd" { if arg == b"--cwd" {
if let Some(value) = args.next() { if let Some(value) = args.next() {
cwd = Some(value) cwd = Some(value)
} }
} else if let Some(value) = arg.drop_prefix(b"--cwd=") { } else if let Some(value) = arg.drop_prefix(b"--cwd=") {
cwd = Some(value.to_owned()) cwd = Some(value.to_owned())
} }
if arg == b"--repository" || arg == b"-R" { if arg == b"--repository" || arg == b"-R" {
if let Some(value) = args.next() { if let Some(value) = args.next() {
repo = Some(value) repo = Some(value)
} }
} else if let Some(value) = arg.drop_prefix(b"--repository=") { } else if let Some(value) = arg.drop_prefix(b"--repository=") {
repo = Some(value.to_owned()) repo = Some(value.to_owned())
} else if let Some(value) = arg.drop_prefix(b"-R") { } else if let Some(value) = arg.drop_prefix(b"-R") {
repo = Some(value.to_owned()) repo = Some(value.to_owned())
} }
} }
Self { config, repo, cwd } Self {
config,
color,
repo,
cwd,
}
} }
} }
/// What to do when encountering some unsupported feature. /// What to do when encountering some unsupported feature.
/// ///
/// See `HgError::UnsupportedFeature` and `CommandError::UnsupportedFeature`. /// See `HgError::UnsupportedFeature` and `CommandError::UnsupportedFeature`.
enum OnUnsupported { enum OnUnsupported {
/// Print an error message describing what feature is not supported, /// Print an error message describing what feature is not supported,