Skip to content

Commit

Permalink
settings: pass around &UserSettings instead of &config::Config consis…
Browse files Browse the repository at this point in the history
…tently

Ui::with_config() is unchanged because I'm not sure if UserSettings should be
constructed earlier. I assume UserSettings will hold an immutable copy of
LayerdConfigs object, whereas Ui has to be initialized before all config layers
get loaded.
  • Loading branch information
yuja committed Nov 23, 2024
1 parent 3d3761b commit 15c3a59
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 16 deletions.
6 changes: 3 additions & 3 deletions cli/src/cli_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -658,10 +658,10 @@ struct AdvanceBookmarksSettings {
}

impl AdvanceBookmarksSettings {
fn from_config(config: &config::Config) -> Result<Self, CommandError> {
fn from_settings(settings: &UserSettings) -> Result<Self, CommandError> {
let get_setting = |setting_key| {
let setting = format!("experimental-advance-branches.{setting_key}");
match config.get::<Vec<String>>(&setting).optional()? {
match settings.config().get::<Vec<String>>(&setting).optional()? {
Some(patterns) => patterns
.into_iter()
.map(|s| {
Expand Down Expand Up @@ -2180,7 +2180,7 @@ Then run `jj squash` to move the resolution into the conflicted commit."#,
&self,
from: impl IntoIterator<Item = &'a CommitId>,
) -> Result<Vec<AdvanceableBookmark>, CommandError> {
let ab_settings = AdvanceBookmarksSettings::from_config(self.settings().config())?;
let ab_settings = AdvanceBookmarksSettings::from_settings(self.settings())?;
if !ab_settings.feature_enabled() {
// Return early if we know that there's no work to do.
return Ok(Vec::new());
Expand Down
6 changes: 4 additions & 2 deletions cli/src/commands/fix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ use jj_lib::repo_path::RepoPathBuf;
use jj_lib::repo_path::RepoPathUiConverter;
use jj_lib::revset::RevsetExpression;
use jj_lib::revset::RevsetIteratorExt;
use jj_lib::settings::UserSettings;
use jj_lib::store::Store;
use jj_lib::tree::Tree;
use pollster::FutureExt;
Expand Down Expand Up @@ -144,7 +145,7 @@ pub(crate) fn cmd_fix(
args: &FixArgs,
) -> Result<(), CommandError> {
let mut workspace_command = command.workspace_helper(ui)?;
let tools_config = get_tools_config(ui, command.settings().config())?;
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")?;
workspace_command.parse_revset(ui, &RevisionArg::from(revs))?
Expand Down Expand Up @@ -445,7 +446,8 @@ struct RawToolConfig {
/// Fails if any of the commands or patterns are obviously unusable, but does
/// 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, config: &config::Config) -> Result<ToolsConfig, CommandError> {
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.
Expand Down
10 changes: 5 additions & 5 deletions lib/src/fsmonitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,9 @@

use std::path::PathBuf;

use config::Config;

use crate::config::ConfigError;
use crate::settings::ConfigResultExt;
use crate::settings::UserSettings;

/// Config for Watchman filesystem monitor (<https://facebook.github.io/watchman/>).
#[derive(Default, Eq, PartialEq, Clone, Debug)]
Expand Down Expand Up @@ -59,11 +58,12 @@ pub enum FsmonitorSettings {

impl FsmonitorSettings {
/// Creates an `FsmonitorSettings` from a `config`.
pub fn from_config(config: &Config) -> Result<FsmonitorSettings, ConfigError> {
match config.get_string("core.fsmonitor") {
pub fn from_settings(settings: &UserSettings) -> Result<FsmonitorSettings, ConfigError> {
match settings.config().get_string("core.fsmonitor") {
Ok(s) => match s.as_str() {
"watchman" => Ok(Self::Watchman(WatchmanConfig {
register_trigger: config
register_trigger: settings
.config()
.get_bool("core.watchman.register_snapshot_trigger")
.optional()?
.unwrap_or_default(),
Expand Down
14 changes: 8 additions & 6 deletions lib/src/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,14 @@ pub struct GitSettings {
}

impl GitSettings {
pub fn from_config(config: &config::Config) -> Self {
let auto_local_bookmark = config
pub fn from_settings(settings: &UserSettings) -> Self {
let auto_local_bookmark = settings
.config()
.get_bool("git.auto-local-bookmark")
.or_else(|_| config.get_bool("git.auto-local-branch"))
.or_else(|_| settings.config().get_bool("git.auto-local-branch"))
.unwrap_or(false);
let abandon_unreachable_commits = config
let abandon_unreachable_commits = settings
.config()
.get_bool("git.abandon-unreachable-commits")
.unwrap_or(true);
GitSettings {
Expand Down Expand Up @@ -169,7 +171,7 @@ impl UserSettings {
}

pub fn fsmonitor_settings(&self) -> Result<FsmonitorSettings, ConfigError> {
FsmonitorSettings::from_config(&self.config)
FsmonitorSettings::from_settings(self)
}

// Must not be changed to avoid git pushing older commits with no set email
Expand Down Expand Up @@ -236,7 +238,7 @@ impl UserSettings {
}

pub fn git_settings(&self) -> GitSettings {
GitSettings::from_config(&self.config)
GitSettings::from_settings(self)
}

pub fn max_new_file_size(&self) -> Result<u64, ConfigError> {
Expand Down

0 comments on commit 15c3a59

Please sign in to comment.