Skip to content

Commit

Permalink
settings: add helper to turn ConfigError::NotFound into Option
Browse files Browse the repository at this point in the history
Now we have 4 callers, I concluded this is common enough to add an
extension method. Still I think it's preferred to define config items in
src/config/*.toml if possible. It will catch typo of config keys.
  • Loading branch information
yuja committed Apr 14, 2023
1 parent 791b821 commit 6a55ae6
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 48 deletions.
14 changes: 14 additions & 0 deletions lib/src/settings.rs
Expand Up @@ -198,3 +198,17 @@ impl JJRng {
}
}
}

pub trait ConfigResultExt<T> {
fn optional(self) -> Result<Option<T>, config::ConfigError>;
}

impl<T> ConfigResultExt<T> for Result<T, config::ConfigError> {
fn optional(self) -> Result<Option<T>, config::ConfigError> {
match self {
Ok(value) => Ok(Some(value)),
Err(config::ConfigError::NotFound(_)) => Ok(None),
Err(err) => Err(err),
}
}
}
56 changes: 24 additions & 32 deletions src/commands/git.rs
Expand Up @@ -7,14 +7,13 @@ use std::sync::Mutex;
use std::time::Instant;

use clap::{ArgGroup, Subcommand};
use config::ConfigError;
use itertools::Itertools;
use jujutsu_lib::backend::ObjectId;
use jujutsu_lib::git::{self, GitFetchError, GitPushError, GitRefUpdate};
use jujutsu_lib::op_store::{BranchTarget, RefTarget};
use jujutsu_lib::refs::{classify_branch_push_action, BranchPushAction, BranchPushUpdate};
use jujutsu_lib::repo::Repo;
use jujutsu_lib::settings::UserSettings;
use jujutsu_lib::settings::{ConfigResultExt as _, UserSettings};
use jujutsu_lib::store::Store;
use jujutsu_lib::view::View;
use jujutsu_lib::workspace::Workspace;
Expand Down Expand Up @@ -341,27 +340,22 @@ fn get_default_fetch_remotes(
) -> Result<Vec<String>, CommandError> {
const KEY: &str = "git.fetch";
let config = command.settings().config();

match config
.get(KEY)
.or_else(|_| config.get_string(KEY).map(|r| vec![r]))
{
if let Ok(remotes) = config.get(KEY) {
Ok(remotes)
} else if let Some(remote) = config.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
Err(ConfigError::NotFound(_)) => {
if let Some(remote) = get_single_remote(git_repo)? {
if remote != DEFAULT_REMOTE {
writeln!(
ui.hint(),
"Fetching from the only existing remote: {}",
remote
)?;
}
Ok(vec![remote])
} else {
Ok(vec![DEFAULT_REMOTE.to_owned()])
}
if remote != DEFAULT_REMOTE {
writeln!(
ui.hint(),
"Fetching from the only existing remote: {}",
remote
)?;
}
r => Ok(r?),
Ok(vec![remote])
} else {
Ok(vec![DEFAULT_REMOTE.to_owned()])
}
}

Expand Down Expand Up @@ -928,19 +922,17 @@ fn get_default_push_remote(
command: &CommandHelper,
git_repo: &git2::Repository,
) -> Result<String, CommandError> {
match command.settings().config().get_string("git.push") {
let config = command.settings().config();
if let Some(remote) = config.get_string("git.push").optional()? {
Ok(remote)
} else if let Some(remote) = get_single_remote(git_repo)? {
// similar to get_default_fetch_remotes
Err(ConfigError::NotFound(_)) => {
if let Some(remote) = get_single_remote(git_repo)? {
if remote != DEFAULT_REMOTE {
writeln!(ui.hint(), "Pushing to the only existing remote: {}", remote)?;
}
Ok(remote)
} else {
Ok(DEFAULT_REMOTE.to_owned())
}
if remote != DEFAULT_REMOTE {
writeln!(ui.hint(), "Pushing to the only existing remote: {}", remote)?;
}
r => Ok(r?),
Ok(remote)
} else {
Ok(DEFAULT_REMOTE.to_owned())
}
}

Expand Down
12 changes: 8 additions & 4 deletions src/config.rs
Expand Up @@ -20,6 +20,7 @@ use std::{env, fmt};

use config::Source;
use itertools::Itertools;
use jujutsu_lib::settings::ConfigResultExt as _;
use thiserror::Error;

#[derive(Error, Debug)]
Expand Down Expand Up @@ -144,10 +145,13 @@ impl LayeredConfigs {
};
for (source, config) in self.sources() {
let top_value = match prefix_key {
Some(ref key) => match config.get(key) {
Err(config::ConfigError::NotFound { .. }) => continue,
val => val?,
},
Some(ref key) => {
if let Some(val) = config.get(key).optional()? {
val
} else {
continue;
}
}
None => config.collect()?.into(),
};
let mut config_stack: Vec<(Vec<&str>, &config::Value)> =
Expand Down
22 changes: 10 additions & 12 deletions src/merge_tools.rs
Expand Up @@ -29,7 +29,7 @@ use jujutsu_lib::conflicts::{
use jujutsu_lib::gitignore::GitIgnoreFile;
use jujutsu_lib::matchers::EverythingMatcher;
use jujutsu_lib::repo_path::RepoPath;
use jujutsu_lib::settings::UserSettings;
use jujutsu_lib::settings::{ConfigResultExt as _, UserSettings};
use jujutsu_lib::store::Store;
use jujutsu_lib::tree::Tree;
use jujutsu_lib::working_copy::{CheckoutError, SnapshotError, TreeState};
Expand Down Expand Up @@ -507,17 +507,15 @@ fn editor_args_from_settings(
) -> Result<CommandNameAndArgs, ExternalToolError> {
// TODO: Make this configuration have a table of possible editors and detect the
// best one here.
match settings.config().get::<CommandNameAndArgs>(key) {
Ok(args) => Ok(args),
Err(config::ConfigError::NotFound(_)) => {
let default_editor = "meld";
writeln!(
ui.hint(),
"Using default editor '{default_editor}'; you can change this by setting {key}"
)?;
Ok(default_editor.into())
}
Err(err) => Err(err.into()),
if let Some(args) = settings.config().get(key).optional()? {
Ok(args)
} else {
let default_editor = "meld";
writeln!(
ui.hint(),
"Using default editor '{default_editor}'; you can change this by setting {key}"
)?;
Ok(default_editor.into())
}
}

Expand Down

0 comments on commit 6a55ae6

Please sign in to comment.