Skip to content

Commit

Permalink
settings: add forwarding getters, replace most of .config() uses
Browse files Browse the repository at this point in the history
.get_table() isn't implemented because it isn't cheap to build a HashMap,
and a table of an abstract Value type wouldn't be useful. Maybe we'll
instead provide an iterator of table keys.

.config() is renamed to .raw_config() to break existing callers.
  • Loading branch information
yuja committed Nov 23, 2024
1 parent 15c3a59 commit ebaabf5
Show file tree
Hide file tree
Showing 25 changed files with 84 additions and 106 deletions.
25 changes: 8 additions & 17 deletions cli/src/cli_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -376,10 +376,7 @@ impl CommandHelper {
Ok(()) => workspace_command,
Err(SnapshotWorkingCopyError::Command(err)) => return Err(err),
Err(SnapshotWorkingCopyError::StaleWorkingCopy(err)) => {
let auto_update_stale = self
.settings()
.config()
.get_bool("snapshot.auto-update-stale")?;
let auto_update_stale = self.settings().get_bool("snapshot.auto-update-stale")?;
if !auto_update_stale {
return Err(err);
}
Expand Down Expand Up @@ -661,7 +658,7 @@ impl AdvanceBookmarksSettings {
fn from_settings(settings: &UserSettings) -> Result<Self, CommandError> {
let get_setting = |setting_key| {
let setting = format!("experimental-advance-branches.{setting_key}");
match settings.config().get::<Vec<String>>(&setting).optional()? {
match settings.get::<Vec<String>>(&setting).optional()? {
Some(patterns) => patterns
.into_iter()
.map(|s| {
Expand Down Expand Up @@ -814,7 +811,6 @@ impl WorkspaceCommandEnvironment {
) -> Result<Option<Rc<UserRevsetExpression>>, CommandError> {
let revset_string = self
.settings()
.config()
.get_string("revsets.short-prefixes")
.unwrap_or_else(|_| self.settings().default_revset());
if revset_string.is_empty() {
Expand Down Expand Up @@ -953,9 +949,8 @@ impl WorkspaceCommandHelper {
loaded_at_head: bool,
) -> Result<Self, CommandError> {
let settings = env.settings();
let commit_summary_template_text =
settings.config().get_string("templates.commit_summary")?;
let op_summary_template_text = settings.config().get_string("templates.op_summary")?;
let commit_summary_template_text = settings.get_string("templates.commit_summary")?;
let op_summary_template_text = settings.get_string("templates.op_summary")?;
let may_update_working_copy =
loaded_at_head && !env.command.global_args().ignore_working_copy;
let working_copy_shared_with_git = is_colocated_git_workspace(&workspace, &repo);
Expand Down Expand Up @@ -1236,7 +1231,7 @@ to the current parents may contain changes from multiple commits.
// empty arguments.
if values.is_empty() {
Ok(FilesetExpression::all())
} else if self.settings().config().get_bool("ui.allow-filesets")? {
} else if self.settings().get_bool("ui.allow-filesets")? {
self.parse_union_filesets(ui, values)
} else {
let expressions = values
Expand Down Expand Up @@ -1265,7 +1260,7 @@ to the current parents may contain changes from multiple commits.

pub fn auto_tracking_matcher(&self, ui: &Ui) -> Result<Box<dyn Matcher>, CommandError> {
let mut diagnostics = FilesetDiagnostics::new();
let pattern = self.settings().config().get_string("snapshot.auto-track")?;
let pattern = self.settings().get_string("snapshot.auto-track")?;
let expression = fileset::parse(
&mut diagnostics,
&pattern,
Expand Down Expand Up @@ -1437,10 +1432,7 @@ to the current parents may contain changes from multiple commits.
let (expression, modifier) = self.parse_revset_with_modifier(ui, revision_arg)?;
let all = match modifier {
Some(RevsetModifier::All) => true,
None => self
.settings()
.config()
.get_bool("ui.always-allow-large-revsets")?,
None => self.settings().get_bool("ui.always-allow-large-revsets")?,
};
if all {
for commit in expression.evaluate_to_commits()? {
Expand Down Expand Up @@ -2689,7 +2681,7 @@ impl LogContentFormat {
pub fn new(ui: &Ui, settings: &UserSettings) -> Result<Self, ConfigError> {
Ok(LogContentFormat {
width: ui.term_width(),
word_wrap: settings.config().get_bool("ui.log-word-wrap")?,
word_wrap: settings.get_bool("ui.log-word-wrap")?,
})
}

Expand Down Expand Up @@ -2748,7 +2740,6 @@ pub fn run_ui_editor(settings: &UserSettings, edit_path: &Path) -> Result<(), Co
// non-Windows): https://github.com/martinvonz/jj/issues/3986
let edit_path = dunce::simplified(edit_path);
let editor: CommandNameAndArgs = settings
.config()
.get("ui.editor")
.map_err(|err| config_error_with_message("Invalid `ui.editor`", err))?;
let mut cmd = editor.to_command();
Expand Down
2 changes: 1 addition & 1 deletion cli/src/commands/bookmark/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ pub fn cmd_bookmark_list(
let language = workspace_command.commit_template_language();
let text = match &args.template {
Some(value) => value.to_owned(),
None => command.settings().config().get("templates.bookmark_list")?,
None => command.settings().get("templates.bookmark_list")?,
};
workspace_command
.parse_template(ui, &language, &text, CommitTemplateLanguage::wrap_ref_name)?
Expand Down
1 change: 0 additions & 1 deletion cli/src/commands/bookmark/track.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@ pub fn cmd_bookmark_track(
let language = workspace_command.commit_template_language();
let text = command
.settings()
.config()
.get::<String>("templates.bookmark_list")?;
workspace_command
.parse_template(ui, &language, &text, CommitTemplateLanguage::wrap_ref_name)?
Expand Down
2 changes: 1 addition & 1 deletion cli/src/commands/config/get.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ pub fn cmd_config_get(
) -> Result<(), CommandError> {
let value = args
.name
.lookup_value(command.settings().config())
.lookup_value(command.settings().raw_config())
.and_then(|value| value.into_string())
.map_err(|err| match err {
ConfigError::Type {
Expand Down
5 changes: 1 addition & 4 deletions cli/src/commands/config/list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,7 @@ pub fn cmd_config_list(
let language = config_template_language();
let text = match &args.template {
Some(value) => value.to_owned(),
None => command
.settings()
.config()
.get_string("templates.config_list")?,
None => command.settings().get_string("templates.config_list")?,
};
command
.parse_template(ui, &language, &text, GenericTemplateLanguage::wrap_self)?
Expand Down
2 changes: 1 addition & 1 deletion cli/src/commands/evolog.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ pub(crate) fn cmd_evolog(
let language = workspace_command.commit_template_language();
let template_string = match &args.template {
Some(value) => value.to_string(),
None => command.settings().config().get_string("templates.log")?,
None => command.settings().get_string("templates.log")?,
};
template = workspace_command
.parse_template(
Expand Down
1 change: 0 additions & 1 deletion cli/src/commands/file/annotate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ pub(crate) fn cmd_file_annotate(

let annotate_commit_summary_text = command
.settings()
.config()
.get_string("templates.annotate_commit_summary")?;
let template = workspace_command.parse_commit_template(ui, &annotate_commit_summary_text)?;

Expand Down
11 changes: 5 additions & 6 deletions cli/src/commands/fix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ pub(crate) fn cmd_fix(
let mut workspace_command = command.workspace_helper(ui)?;
let tools_config = get_tools_config(ui, command.settings())?;
let root_commits: Vec<CommitId> = if args.source.is_empty() {
let revs = command.settings().config().get_string("revsets.fix")?;
let revs = command.settings().get_string("revsets.fix")?;
workspace_command.parse_revset(ui, &RevisionArg::from(revs))?
} else {
workspace_command.parse_union_revsets(ui, &args.source)?
Expand Down Expand Up @@ -447,11 +447,10 @@ struct RawToolConfig {
/// not check for issues that might still occur later like missing executables.
/// This is a place where we could fail earlier in some cases, though.
fn get_tools_config(ui: &mut Ui, settings: &UserSettings) -> Result<ToolsConfig, CommandError> {
let config = settings.config();
let mut tools_config = ToolsConfig { tools: Vec::new() };
// TODO: Remove this block of code and associated documentation after at least
// one release where the feature is marked deprecated.
if let Ok(tool_command) = config.get::<CommandNameAndArgs>("fix.tool-command") {
if let Ok(tool_command) = settings.get::<CommandNameAndArgs>("fix.tool-command") {
// This doesn't change the displayed indices of the `fix.tools` definitions, and
// doesn't have a `name` that could conflict with them. That would matter more
// if we already had better error handling that made use of the `name`.
Expand All @@ -471,10 +470,10 @@ fn get_tools_config(ui: &mut Ui, settings: &UserSettings) -> Result<ToolsConfig,
command = {}
patterns = ["all()"]
"###,
to_toml_value(&config.get::<config::Value>("fix.tool-command").unwrap()).unwrap()
to_toml_value(&settings.get::<config::Value>("fix.tool-command").unwrap()).unwrap()
)?;
}
if let Ok(tools_table) = config.get_table("fix.tools") {
if let Ok(tools_table) = settings.raw_config().get_table("fix.tools") {
// Convert the map into a sorted vector early so errors are deterministic.
let mut tools: Vec<ToolConfig> = tools_table
.into_iter()
Expand Down Expand Up @@ -509,7 +508,7 @@ fn get_tools_config(ui: &mut Ui, settings: &UserSettings) -> Result<ToolsConfig,
if tools_config.tools.is_empty() {
// TODO: This is not a useful message when one or both fields are present but
// have the wrong type. After removing `fix.tool-command`, it will be simpler to
// propagate any errors from `config.get_array("fix.tools")`.
// propagate any errors from `settings.get_array("fix.tools")`.
Err(config_error(
"At least one entry of `fix.tools` or `fix.tool-command` is required.".to_string(),
))
Expand Down
4 changes: 2 additions & 2 deletions cli/src/commands/git/fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,9 @@ fn get_default_fetch_remotes(
git_repo: &git2::Repository,
) -> Result<Vec<String>, CommandError> {
const KEY: &str = "git.fetch";
if let Ok(remotes) = settings.config().get(KEY) {
if let Ok(remotes) = settings.get(KEY) {
Ok(remotes)
} else if let Some(remote) = settings.config().get_string(KEY).optional()? {
} else if let Some(remote) = settings.get_string(KEY).optional()? {
Ok(vec![remote])
} else if let Some(remote) = get_single_remote(git_repo)? {
// if nothing was explicitly configured, try to guess
Expand Down
6 changes: 3 additions & 3 deletions cli/src/commands/git/push.rs
Original file line number Diff line number Diff line change
Expand Up @@ -369,8 +369,8 @@ fn validate_commits_ready_to_push(
.union(workspace_helper.env().immutable_heads_expression())
.range(&RevsetExpression::commits(new_heads));

let config = command.settings().config();
let is_private = if let Ok(revset) = config.get_string("git.private-commits") {
let settings = command.settings();
let is_private = if let Ok(revset) = settings.get_string("git.private-commits") {
workspace_helper
.parse_revset(ui, &RevisionArg::from(revset))?
.evaluate()?
Expand Down Expand Up @@ -483,7 +483,7 @@ fn get_default_push_remote(
settings: &UserSettings,
git_repo: &git2::Repository,
) -> Result<String, CommandError> {
if let Some(remote) = settings.config().get_string("git.push").optional()? {
if let Some(remote) = settings.get_string("git.push").optional()? {
Ok(remote)
} else if let Some(remote) = get_single_remote(git_repo)? {
// similar to get_default_fetch_remotes
Expand Down
8 changes: 2 additions & 6 deletions cli/src/commands/log.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,6 @@ pub(crate) fn cmd_log(

let use_elided_nodes = command
.settings()
.config()
.get_bool("ui.log-synthetic-elided-nodes")?;
let with_content_format = LogContentFormat::new(ui, command.settings())?;

Expand All @@ -152,7 +151,7 @@ pub(crate) fn cmd_log(
let language = workspace_command.commit_template_language();
let template_string = match &args.template {
Some(value) => value.to_string(),
None => command.settings().config().get_string("templates.log")?,
None => command.settings().get_string("templates.log")?,
};
template = workspace_command
.parse_template(
Expand Down Expand Up @@ -333,10 +332,7 @@ pub fn get_node_template(
style: GraphStyle,
settings: &UserSettings,
) -> Result<String, ConfigError> {
let symbol = settings
.config()
.get_string("templates.log_node")
.optional()?;
let symbol = settings.get_string("templates.log_node").optional()?;
let default = if style.is_ascii() {
"builtin_log_node_ascii"
} else {
Expand Down
5 changes: 1 addition & 4 deletions cli/src/commands/operation/diff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,10 +130,7 @@ pub fn cmd_op_diff(
let id_prefix_context = workspace_env.new_id_prefix_context();
let commit_summary_template = {
let language = workspace_env.commit_template_language(merged_repo, &id_prefix_context);
let text = command
.settings()
.config()
.get_string("templates.commit_summary")?;
let text = command.settings().get_string("templates.commit_summary")?;
workspace_env.parse_template(ui, &language, &text, CommitTemplateLanguage::wrap_commit)?
};

Expand Down
9 changes: 3 additions & 6 deletions cli/src/commands/operation/log.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ fn do_op_log(
);
let text = match &args.template {
Some(value) => value.to_owned(),
None => settings.config().get_string("templates.op_log")?,
None => settings.get_string("templates.op_log")?,
};
template = workspace_env
.parse_template(
Expand All @@ -145,7 +145,7 @@ fn do_op_log(

let diff_formats = diff_formats_for_log(settings, &args.diff_format, args.patch)?;
let maybe_show_op_diff = if args.op_diff || !diff_formats.is_empty() {
let template_text = settings.config().get_string("templates.commit_summary")?;
let template_text = settings.get_string("templates.commit_summary")?;
let show = move |ui: &Ui,
formatter: &mut dyn Formatter,
op: &Operation,
Expand Down Expand Up @@ -241,10 +241,7 @@ fn do_op_log(
}

fn get_node_template(style: GraphStyle, settings: &UserSettings) -> Result<String, ConfigError> {
let symbol = settings
.config()
.get_string("templates.op_log_node")
.optional()?;
let symbol = settings.get_string("templates.op_log_node").optional()?;
let default = if style.is_ascii() {
"builtin_op_log_node_ascii"
} else {
Expand Down
7 changes: 2 additions & 5 deletions cli/src/commands/operation/show.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,7 @@ pub fn cmd_op_show(
let id_prefix_context = workspace_env.new_id_prefix_context();
let commit_summary_template = {
let language = workspace_env.commit_template_language(repo.as_ref(), &id_prefix_context);
let text = command
.settings()
.config()
.get_string("templates.commit_summary")?;
let text = command.settings().get_string("templates.commit_summary")?;
workspace_env.parse_template(ui, &language, &text, CommitTemplateLanguage::wrap_commit)?
};

Expand All @@ -81,7 +78,7 @@ pub fn cmd_op_show(

// TODO: Should we make this customizable via clap arg?
let template = {
let text = command.settings().config().get_string("templates.op_log")?;
let text = command.settings().get_string("templates.op_log")?;
workspace_command
.parse_operation_template(ui, &text)?
.labeled("operation")
Expand Down
2 changes: 1 addition & 1 deletion cli/src/commands/show.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ pub(crate) fn cmd_show(
let commit = workspace_command.resolve_single_rev(ui, &args.revision)?;
let template_string = match &args.template {
Some(value) => value.to_string(),
None => command.settings().config().get_string("templates.show")?,
None => command.settings().get_string("templates.show")?,
};
let template = workspace_command.parse_commit_template(ui, &template_string)?;
let diff_renderer = workspace_command.diff_renderer_for(&args.format)?;
Expand Down
2 changes: 1 addition & 1 deletion cli/src/commands/tag.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ fn cmd_tag_list(
let language = workspace_command.commit_template_language();
let text = match &args.template {
Some(value) => value.to_owned(),
None => command.settings().config().get("templates.tag_list")?,
None => command.settings().get("templates.tag_list")?,
};
workspace_command
.parse_template(ui, &language, &text, CommitTemplateLanguage::wrap_ref_name)?
Expand Down
2 changes: 1 addition & 1 deletion cli/src/description_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ pub fn description_template(

// Named as "draft" because the output can contain "JJ: " comment lines.
let template_key = "templates.draft_commit_description";
let template_text = tx.settings().config().get_string(template_key)?;
let template_text = tx.settings().get_string(template_key)?;
let template = tx.parse_commit_template(ui, &template_text)?;

let mut output = Vec::new();
Expand Down
14 changes: 6 additions & 8 deletions cli/src/diff_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,8 +209,7 @@ fn default_diff_format(
settings: &UserSettings,
args: &DiffFormatArgs,
) -> Result<DiffFormat, ConfigError> {
let config = settings.config();
if let Some(args) = config.get("ui.diff.tool").optional()? {
if let Some(args) = settings.get("ui.diff.tool").optional()? {
// External "tool" overrides the internal "format" option.
let tool = if let CommandNameAndArgs::String(name) = &args {
merge_tools::get_external_tool_config(settings, name)?
Expand All @@ -220,9 +219,9 @@ fn default_diff_format(
.unwrap_or_else(|| ExternalMergeTool::with_diff_args(&args));
return Ok(DiffFormat::Tool(Box::new(tool)));
}
let name = if let Some(name) = config.get_string("ui.diff.format").optional()? {
let name = if let Some(name) = settings.get_string("ui.diff.format").optional()? {
name
} else if let Some(name) = config.get_string("diff.format").optional()? {
} else if let Some(name) = settings.get_string("diff.format").optional()? {
name // old config name
} else {
"color-words".to_owned()
Expand Down Expand Up @@ -512,10 +511,9 @@ impl ColorWordsDiffOptions {
settings: &UserSettings,
args: &DiffFormatArgs,
) -> Result<Self, ConfigError> {
let config = settings.config();
let max_inline_alternation = {
let key = "diff.color-words.max-inline-alternation";
match config.get_int(key)? {
match settings.get_int(key)? {
-1 => None, // unlimited
n => Some(
usize::try_from(n)
Expand All @@ -525,7 +523,7 @@ impl ColorWordsDiffOptions {
};
let context = args
.context
.map_or_else(|| config.get("diff.color-words.context"), Ok)?;
.map_or_else(|| settings.get("diff.color-words.context"), Ok)?;
Ok(ColorWordsDiffOptions {
context,
line_diff: LineDiffOptions::from_args(args),
Expand Down Expand Up @@ -1216,7 +1214,7 @@ impl UnifiedDiffOptions {
) -> Result<Self, ConfigError> {
let context = args
.context
.map_or_else(|| settings.config().get("diff.git.context"), Ok)?;
.map_or_else(|| settings.get("diff.git.context"), Ok)?;
Ok(UnifiedDiffOptions {
context,
line_diff: LineDiffOptions::from_args(args),
Expand Down
2 changes: 1 addition & 1 deletion cli/src/graphlog.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ pub enum GraphStyle {

impl GraphStyle {
pub fn from_settings(settings: &UserSettings) -> Result<Self, ConfigError> {
settings.config().get("ui.graph.style")
settings.get("ui.graph.style")
}

pub fn is_ascii(self) -> bool {
Expand Down
Loading

0 comments on commit ebaabf5

Please sign in to comment.