Skip to content

Commit

Permalink
cli: inline write/remove_config_value_to/from_file()
Browse files Browse the repository at this point in the history
They are short, and it wouldn't make much sense to do load, mutate one entry,
and save in one function.

In later patches, "jj config set"/"unset" will be changed to reuse the loaded
ConfigLayer if the layer can be unambiguously selected.
  • Loading branch information
yuja committed Dec 13, 2024
1 parent 49d70e6 commit 53dd738
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 61 deletions.
13 changes: 13 additions & 0 deletions cli/src/commands/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ mod unset;

use std::path::Path;

use jj_lib::config::ConfigFile;
use jj_lib::config::ConfigSource;
use tracing::instrument;

Expand Down Expand Up @@ -83,6 +84,18 @@ impl ConfigLevelArgs {
panic!("No config_level provided")
}
}

fn edit_config_file(&self, config_env: &ConfigEnv) -> Result<ConfigFile, CommandError> {
let path = self.new_config_file_path(config_env)?;
if path.is_dir() {
return Err(user_error(format!(
"Can't set config in path {path} (dirs not supported)",
path = path.display()
)));
}
let source = self.get_source_kind().unwrap();
Ok(ConfigFile::load_or_empty(source, path)?)
}
}

/// Manage config options
Expand Down
16 changes: 6 additions & 10 deletions cli/src/commands/config/set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,10 @@ use tracing::instrument;
use super::ConfigLevelArgs;
use crate::cli_util::CommandHelper;
use crate::cli_util::WorkspaceCommandHelper;
use crate::command_error::user_error;
use crate::command_error::user_error_with_message;
use crate::command_error::CommandError;
use crate::complete;
use crate::config::parse_toml_value_or_bare_string;
use crate::config::write_config_value_to_file;
use crate::ui::Ui;

/// Update config file to set the given option to a given value.
Expand All @@ -53,13 +52,7 @@ pub fn cmd_config_set(
command: &CommandHelper,
args: &ConfigSetArgs,
) -> Result<(), CommandError> {
let config_path = args.level.new_config_file_path(command.config_env())?;
if config_path.is_dir() {
return Err(user_error(format!(
"Can't set config in path {path} (dirs not supported)",
path = config_path.display()
)));
}
let mut file = args.level.edit_config_file(command.config_env())?;

// TODO(#531): Infer types based on schema (w/ --type arg to override).
let value = parse_toml_value_or_bare_string(&args.value);
Expand All @@ -72,7 +65,10 @@ pub fn cmd_config_set(
check_wc_author(ui, command, &value, AuthorChange::Email)?;
};

write_config_value_to_file(&args.name, value, config_path)
file.set_value(&args.name, value)
.map_err(|err| user_error_with_message(format!("Failed to set {}", args.name), err))?;
file.save()?;
Ok(())
}

/// Returns the commit of the working copy if it exists.
Expand Down
18 changes: 9 additions & 9 deletions cli/src/commands/config/unset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ use tracing::instrument;
use super::ConfigLevelArgs;
use crate::cli_util::CommandHelper;
use crate::command_error::user_error;
use crate::command_error::user_error_with_message;
use crate::command_error::CommandError;
use crate::complete;
use crate::config::remove_config_value_from_file;
use crate::ui::Ui;

/// Update config file to unset the given option.
Expand All @@ -39,13 +39,13 @@ pub fn cmd_config_unset(
command: &CommandHelper,
args: &ConfigUnsetArgs,
) -> Result<(), CommandError> {
let config_path = args.level.new_config_file_path(command.config_env())?;
if config_path.is_dir() {
return Err(user_error(format!(
"Can't set config in path {path} (dirs not supported)",
path = config_path.display()
)));
let mut file = args.level.edit_config_file(command.config_env())?;
let old_value = file
.delete_value(&args.name)
.map_err(|err| user_error_with_message(format!("Failed to unset {}", args.name), err))?;
if old_value.is_none() {
return Err(user_error(format!(r#""{}" doesn't exist"#, args.name)));
}

remove_config_value_from_file(&args.name, config_path)
file.save()?;
Ok(())
}
14 changes: 6 additions & 8 deletions cli/src/commands/git/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ pub mod submodule;
use std::path::Path;

use clap::Subcommand;
use jj_lib::config::ConfigNamePathBuf;
use jj_lib::config::ConfigFile;
use jj_lib::config::ConfigSource;

use self::clone::cmd_git_clone;
use self::clone::GitCloneArgs;
Expand All @@ -46,7 +47,6 @@ use crate::cli_util::CommandHelper;
use crate::cli_util::WorkspaceCommandHelper;
use crate::command_error::user_error_with_message;
use crate::command_error::CommandError;
use crate::config::write_config_value_to_file;
use crate::ui::Ui;

/// Commands for working with Git remotes and the underlying Git repo
Expand Down Expand Up @@ -114,12 +114,10 @@ fn write_repository_level_trunk_alias(
remote: &str,
branch: &str,
) -> Result<(), CommandError> {
let config_path = repo_path.join("config.toml");
write_config_value_to_file(
&ConfigNamePathBuf::from_iter(["revset-aliases", "trunk()"]),
format!("{branch}@{remote}").into(),
&config_path,
)?;
let mut file = ConfigFile::load_or_empty(ConfigSource::Repo, repo_path.join("config.toml"))?;
file.set_value(["revset-aliases", "trunk()"], format!("{branch}@{remote}"))
.expect("initial repo config shouldn't have invalid values");
file.save()?;
writeln!(
ui.status(),
r#"Setting the revset alias "trunk()" to "{branch}@{remote}""#,
Expand Down
34 changes: 0 additions & 34 deletions cli/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ use std::path::PathBuf;
use std::process::Command;

use itertools::Itertools;
use jj_lib::config::ConfigFile;
use jj_lib::config::ConfigLayer;
use jj_lib::config::ConfigLoadError;
use jj_lib::config::ConfigNamePathBuf;
Expand All @@ -34,10 +33,6 @@ use regex::Regex;
use thiserror::Error;
use tracing::instrument;

use crate::command_error::user_error;
use crate::command_error::user_error_with_message;
use crate::command_error::CommandError;

// TODO(#879): Consider generating entire schema dynamically vs. static file.
pub const CONFIG_SCHEMA: &str = include_str!("config-schema.json");

Expand Down Expand Up @@ -427,35 +422,6 @@ pub fn parse_config_args(toml_strs: &[ConfigArg]) -> Result<Vec<ConfigLayer>, Co
.try_collect()
}

pub fn write_config_value_to_file(
key: &ConfigNamePathBuf,
value: toml_edit::Value,
path: &Path,
) -> Result<(), CommandError> {
// TODO: Load config layer by caller. Here we use a dummy source for now.
let mut file = ConfigFile::load_or_empty(ConfigSource::User, path)?;
file.set_value(key, value)
.map_err(|err| user_error_with_message(format!("Failed to set {key}"), err))?;
file.save()?;
Ok(())
}

pub fn remove_config_value_from_file(
key: &ConfigNamePathBuf,
path: &Path,
) -> Result<(), CommandError> {
// TODO: Load config layer by caller. Here we use a dummy source for now.
let mut file = ConfigFile::load_or_empty(ConfigSource::User, path)?;
let old_value = file
.delete_value(key)
.map_err(|err| user_error_with_message(format!("Failed to unset {key}"), err))?;
if old_value.is_none() {
return Err(user_error(format!(r#""{key}" doesn't exist"#)));
}
file.save()?;
Ok(())
}

/// Command name and arguments specified by config.
#[derive(Clone, Debug, Eq, PartialEq, serde::Deserialize)]
#[serde(untagged)]
Expand Down

0 comments on commit 53dd738

Please sign in to comment.