refactor(home): remove dead settings path fallbacks

default_settings_path() and active_settings_path() always return a
value (Home::from_env() never fails), so unwrap_or_else fallbacks to
".fabro/settings.toml" were dead code. Change both functions to return
PathBuf instead of Option<PathBuf> and remove the unreachable branches
in server_client, serve, and user config.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Bryan Helmkamp 2026-04-06 13:59:17 -04:00
parent 7afcb5091a
commit c57bf24410
No known key found for this signature in database
6 changed files with 23 additions and 21 deletions

View file

@ -335,7 +335,9 @@ pub(crate) async fn run_doctor(
title: "Local".to_string(),
checks: vec![
check_config(
settings_config_path.filter(|path| path.exists()),
settings_config_path
.exists()
.then_some(settings_config_path),
&legacy_config_paths,
),
check_legacy_env(legacy_env_path),

View file

@ -55,10 +55,12 @@ pub(crate) async fn dispatch(command: ServerCommand, _globals: &GlobalArgs) -> R
record_path,
serve_args,
}) => {
let active_config_path = serve_args
.config
.clone()
.or_else(|| user_config::active_settings_path(None));
let active_config_path = Some(
serve_args
.config
.clone()
.unwrap_or_else(|| user_config::active_settings_path(None)),
);
let bind_addr = if let Some(s) = serve_args.bind.as_deref() {
bind::parse_bind(s)?
} else {

View file

@ -86,7 +86,9 @@ pub(crate) fn build_run_manifest(input: ManifestBuildInput) -> Result<BuiltManif
type_: types::ManifestConfigType::Project,
});
}
if let Some(path) = active_settings_path(None).filter(|path| path.is_file()) {
let settings_path = active_settings_path(None);
if settings_path.is_file() {
let path = settings_path;
let source = std::fs::read_to_string(&path)
.with_context(|| format!("Failed to read {}", path.display()))?;
configs.push(types::ManifestConfig {

View file

@ -117,8 +117,7 @@ pub(crate) async fn connect_server_only(args: &ServerTargetArgs) -> Result<Serve
let settings = user_config::load_settings()?;
let target = user_config::resolve_server_target(args, &settings)?;
let runtime = LocalServerRuntime {
active_config_path: user_config::active_settings_path(None)
.unwrap_or_else(|| PathBuf::from(".fabro/settings.toml")),
active_config_path: user_config::active_settings_path(None),
storage_dir: settings.storage_dir(),
};
Ok(ServerStoreClient {
@ -127,8 +126,7 @@ pub(crate) async fn connect_server_only(args: &ServerTargetArgs) -> Result<Serve
}
pub(crate) async fn connect_api_client(storage_dir: &Path) -> Result<fabro_api::Client> {
let config_path = user_config::active_settings_path(None)
.unwrap_or_else(|| PathBuf::from(".fabro/settings.toml"));
let config_path = user_config::active_settings_path(None);
let bind = start::ensure_server_running_for_storage(storage_dir, &config_path)
.with_context(|| format!("Failed to start fabro server for {}", storage_dir.display()))?;
match bind {
@ -169,8 +167,7 @@ pub(crate) async fn connect_server_backed_api_client(
let settings = user_config::load_settings()?;
let target = user_config::resolve_server_target(args, &settings)?;
let runtime = LocalServerRuntime {
active_config_path: user_config::active_settings_path(None)
.unwrap_or_else(|| PathBuf::from(".fabro/settings.toml")),
active_config_path: user_config::active_settings_path(None),
storage_dir: settings.storage_dir(),
};
connect_target_api_client(&target, &runtime).await

View file

@ -81,8 +81,8 @@ impl From<ExecConfig> for ExecSettings {
}
}
pub fn default_settings_path() -> Option<PathBuf> {
Some(Home::from_env().user_config())
pub fn default_settings_path() -> PathBuf {
Home::from_env().user_config()
}
pub fn default_socket_path() -> PathBuf {
@ -93,10 +93,10 @@ pub fn legacy_default_storage_root() -> PathBuf {
Home::from_env().root().to_path_buf()
}
pub fn active_settings_path(path: Option<&Path>) -> Option<PathBuf> {
pub fn active_settings_path(path: Option<&Path>) -> PathBuf {
path.map(Path::to_path_buf)
.or_else(|| std::env::var_os(FABRO_CONFIG_ENV).map(PathBuf::from))
.or_else(default_settings_path)
.unwrap_or_else(default_settings_path)
}
pub fn legacy_user_config_path() -> Option<PathBuf> {
@ -146,8 +146,7 @@ pub fn load_settings_config(path: Option<&Path>) -> anyhow::Result<ConfigLayer>
.flatten()
{
if legacy_path.is_file() && should_warn_about_legacy_user_config(&legacy_path) {
let target = default_settings_path()
.unwrap_or_else(|| PathBuf::from(format!("~/.fabro/{SETTINGS_CONFIG_FILENAME}")));
let target = default_settings_path();
eprintln!(
"Warning: ignoring legacy config file {}. Rename it to {}.",
legacy_path.display(),
@ -210,7 +209,7 @@ mod tests {
assert_eq!(
default_settings_path(),
Some(home.join(".fabro").join(SETTINGS_CONFIG_FILENAME))
home.join(".fabro").join(SETTINGS_CONFIG_FILENAME)
);
assert_eq!(default_socket_path(), home.join(".fabro/fabro.sock"));
assert_eq!(
@ -247,6 +246,6 @@ mod tests {
let custom_path = dir.path().join("custom-settings.toml");
let _guard = EnvGuard::set(FABRO_CONFIG_ENV, Some(&custom_path));
assert_eq!(active_settings_path(None), Some(custom_path));
assert_eq!(active_settings_path(None), custom_path);
}
}

View file

@ -61,7 +61,7 @@ fn load_settings(path: Option<&Path>) -> anyhow::Result<Settings> {
}
fn resolved_config_path(path: Option<&Path>) -> PathBuf {
active_settings_path(path).unwrap_or_else(|| PathBuf::from(".fabro/settings.toml"))
active_settings_path(path)
}
fn apply_serve_overrides(base: &Settings, args: &ServeArgs, dry_run_mode: bool) -> Settings {