mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-08-28 05:27:41 +00:00
refactor(cli): collapse duplicated bootstrap and settings helpers
- Merge prepare_foreground_server_bootstrap and prepare_server_sink_bootstrap
into one prepare_server_bootstrap(config, storage, foreground).
- Drop three one-line settings_layer_* passthroughs from user_config; callers
now use load_settings_with_{storage_dir,config_and_storage_dir} directly.
- Swap underscore-prefixed lock field for #[expect(dead_code, reason=…)] to
document RAII intent explicitly.
- Remove two narrate-what-it-does comments.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
858e8e1270
commit
a256c14a77
7 changed files with 34 additions and 58 deletions
|
|
@ -21,10 +21,8 @@ fn config_layers(
|
|||
Some(path) => workflow_and_project_layers(path, cwd)?,
|
||||
None => (SettingsLayer::default(), load_settings_project(cwd)?),
|
||||
};
|
||||
let user_layer = user_config::settings_layer_with_config_and_storage_dir(
|
||||
Some(ctx.base_config_path()),
|
||||
None,
|
||||
)?;
|
||||
let user_layer =
|
||||
user_config::load_settings_with_config_and_storage_dir(Some(ctx.base_config_path()), None)?;
|
||||
Ok(EffectiveSettingsLayers::new(
|
||||
SettingsLayer::default(),
|
||||
workflow_layer,
|
||||
|
|
|
|||
|
|
@ -7,7 +7,7 @@ use fabro_util::terminal::Styles;
|
|||
use crate::args::RunArgs;
|
||||
use crate::command_context::CommandContext;
|
||||
use crate::shared::print_json_pretty;
|
||||
use crate::user_config::settings_layer_with_storage_dir;
|
||||
use crate::user_config::load_settings_with_storage_dir;
|
||||
|
||||
pub(crate) async fn execute(
|
||||
mut args: RunArgs,
|
||||
|
|
@ -17,7 +17,7 @@ pub(crate) async fn execute(
|
|||
) -> Result<()> {
|
||||
let styles: &'static Styles = Box::leak(Box::new(Styles::detect_stderr()));
|
||||
let ctx = CommandContext::for_target(&args.target, printer, cli.clone(), cli_layer)?;
|
||||
let cli_defaults = settings_layer_with_storage_dir(None)?;
|
||||
let cli_defaults = load_settings_with_storage_dir(None)?;
|
||||
args.verbose = args.verbose || cli.output.verbosity == OutputVerbosity::Verbose;
|
||||
|
||||
let quiet = args.detach;
|
||||
|
|
|
|||
|
|
@ -8,7 +8,7 @@ use crate::args::{AttachArgs, RunCommands, RunWorkerArgs, StartArgs};
|
|||
use crate::command_context::CommandContext;
|
||||
use crate::server_runs::ServerSummaryLookup;
|
||||
use crate::shared::print_json_pretty;
|
||||
use crate::user_config::settings_layer_with_storage_dir;
|
||||
use crate::user_config::load_settings_with_storage_dir;
|
||||
|
||||
pub(crate) mod attach;
|
||||
pub(crate) mod command;
|
||||
|
|
@ -39,7 +39,7 @@ pub(crate) async fn dispatch(
|
|||
RunCommands::Run(args) => Box::pin(command::execute(args, cli, cli_layer, printer)).await,
|
||||
RunCommands::Create(args) => {
|
||||
let styles: &'static Styles = Box::leak(Box::new(Styles::detect_stderr()));
|
||||
let cli_defaults = settings_layer_with_storage_dir(None)?;
|
||||
let cli_defaults = load_settings_with_storage_dir(None)?;
|
||||
let ctx = CommandContext::for_target(&args.target, printer, cli.clone(), cli_layer)?;
|
||||
let created_run = Box::pin(create::create_run(
|
||||
&ctx,
|
||||
|
|
|
|||
|
|
@ -158,6 +158,6 @@ mod tests {
|
|||
write_server_record(&path, &record).unwrap();
|
||||
|
||||
assert!(active_server_record(dir.path()).unwrap().is_none());
|
||||
assert!(!path.exists()); // lazy cleanup removed file
|
||||
assert!(!path.exists());
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -18,7 +18,8 @@ use tokio::time;
|
|||
use super::record;
|
||||
|
||||
pub(crate) struct ForegroundServerLogBootstrap {
|
||||
_lock_file: std::fs::File,
|
||||
#[expect(dead_code, reason = "held for its Drop to release the server lock")]
|
||||
lock_file: std::fs::File,
|
||||
}
|
||||
|
||||
pub(crate) async fn execute(
|
||||
|
|
@ -76,9 +77,7 @@ pub(crate) async fn prepare_foreground_server_log(
|
|||
std::fs::File::create(&log_path)
|
||||
.with_context(|| format!("creating server log file {}", log_path.display()))?;
|
||||
|
||||
Ok(ForegroundServerLogBootstrap {
|
||||
_lock_file: lock_file,
|
||||
})
|
||||
Ok(ForegroundServerLogBootstrap { lock_file })
|
||||
}
|
||||
|
||||
pub(crate) async fn ensure_server_running_for_storage(
|
||||
|
|
@ -364,7 +363,7 @@ async fn execute_daemon(
|
|||
printer: Printer,
|
||||
) -> Result<()> {
|
||||
let lock_file = acquire_lock(storage_dir).await?;
|
||||
let _lock_file = lock_file; // keep alive until function returns
|
||||
let _lock_file = lock_file;
|
||||
|
||||
if let Some(existing) = record::active_server_record(storage_dir)? {
|
||||
if announce {
|
||||
|
|
|
|||
|
|
@ -420,66 +420,58 @@ async fn pre_tracing_bootstrap(command: &Commands) -> Result<PreTracingBootstrap
|
|||
Commands::Server(ServerNamespace {
|
||||
command: ServerCommand::Start(args),
|
||||
}) if args.foreground => {
|
||||
prepare_foreground_server_bootstrap(
|
||||
prepare_server_bootstrap(
|
||||
args.serve_args.config.as_deref(),
|
||||
args.storage_dir.as_deref(),
|
||||
true,
|
||||
)
|
||||
.await
|
||||
}
|
||||
Commands::Server(ServerNamespace {
|
||||
command: ServerCommand::Restart(args),
|
||||
}) if args.foreground => {
|
||||
prepare_foreground_server_bootstrap(
|
||||
prepare_server_bootstrap(
|
||||
args.serve_args.config.as_deref(),
|
||||
args.storage_dir.as_deref(),
|
||||
true,
|
||||
)
|
||||
.await
|
||||
}
|
||||
Commands::Server(ServerNamespace {
|
||||
command: ServerCommand::Serve(args),
|
||||
}) => prepare_server_sink_bootstrap(
|
||||
args.serve_args.config.as_deref(),
|
||||
args.storage_dir.as_deref(),
|
||||
),
|
||||
}) => {
|
||||
prepare_server_bootstrap(
|
||||
args.serve_args.config.as_deref(),
|
||||
args.storage_dir.as_deref(),
|
||||
false,
|
||||
)
|
||||
.await
|
||||
}
|
||||
_ => Ok(PreTracingBootstrap::cli()),
|
||||
}
|
||||
}
|
||||
|
||||
async fn prepare_foreground_server_bootstrap(
|
||||
async fn prepare_server_bootstrap(
|
||||
config_path: Option<&std::path::Path>,
|
||||
storage_dir: Option<&std::path::Path>,
|
||||
foreground: bool,
|
||||
) -> Result<PreTracingBootstrap> {
|
||||
let settings =
|
||||
user_config::load_settings_with_config_and_storage_dir(config_path, storage_dir)?;
|
||||
let storage_dir = user_config::storage_dir(&settings)?;
|
||||
let storage = fabro_config::Storage::new(&storage_dir);
|
||||
let foreground_server_log_bootstrap =
|
||||
commands::server::start::prepare_foreground_server_log(&storage_dir).await?;
|
||||
let foreground_server_log_bootstrap = if foreground {
|
||||
Some(commands::server::start::prepare_foreground_server_log(&storage_dir).await?)
|
||||
} else {
|
||||
None
|
||||
};
|
||||
|
||||
Ok(PreTracingBootstrap {
|
||||
sink: logging::InternalLogSink::Server {
|
||||
path: storage.server_state().log_path(),
|
||||
},
|
||||
config_log_level: server_config_log_level(&settings),
|
||||
foreground_server_log_bootstrap: Some(foreground_server_log_bootstrap),
|
||||
})
|
||||
}
|
||||
|
||||
fn prepare_server_sink_bootstrap(
|
||||
config_path: Option<&std::path::Path>,
|
||||
storage_dir: Option<&std::path::Path>,
|
||||
) -> Result<PreTracingBootstrap> {
|
||||
let settings =
|
||||
user_config::load_settings_with_config_and_storage_dir(config_path, storage_dir)?;
|
||||
let storage_dir = user_config::storage_dir(&settings)?;
|
||||
let storage = fabro_config::Storage::new(&storage_dir);
|
||||
|
||||
Ok(PreTracingBootstrap {
|
||||
sink: logging::InternalLogSink::Server {
|
||||
path: storage.server_state().log_path(),
|
||||
},
|
||||
config_log_level: server_config_log_level(&settings),
|
||||
foreground_server_log_bootstrap: None,
|
||||
foreground_server_log_bootstrap,
|
||||
})
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -22,31 +22,18 @@ pub(crate) fn load_settings() -> anyhow::Result<SettingsLayer> {
|
|||
load_settings_with_config_and_storage_dir(None, None)
|
||||
}
|
||||
|
||||
pub(crate) fn settings_layer_with_config_and_storage_dir(
|
||||
config_path: Option<&Path>,
|
||||
storage_dir: Option<&Path>,
|
||||
) -> anyhow::Result<SettingsLayer> {
|
||||
let layer = load_settings_config(config_path)?;
|
||||
Ok(apply_storage_dir_override(layer, storage_dir))
|
||||
}
|
||||
|
||||
pub(crate) fn settings_layer_with_storage_dir(
|
||||
storage_dir: Option<&Path>,
|
||||
) -> anyhow::Result<SettingsLayer> {
|
||||
settings_layer_with_config_and_storage_dir(None, storage_dir)
|
||||
}
|
||||
|
||||
pub(crate) fn load_settings_with_storage_dir(
|
||||
storage_dir: Option<&Path>,
|
||||
) -> anyhow::Result<SettingsLayer> {
|
||||
settings_layer_with_storage_dir(storage_dir)
|
||||
load_settings_with_config_and_storage_dir(None, storage_dir)
|
||||
}
|
||||
|
||||
pub(crate) fn load_settings_with_config_and_storage_dir(
|
||||
config_path: Option<&Path>,
|
||||
storage_dir: Option<&Path>,
|
||||
) -> anyhow::Result<SettingsLayer> {
|
||||
settings_layer_with_config_and_storage_dir(config_path, storage_dir)
|
||||
let layer = load_settings_config(config_path)?;
|
||||
Ok(apply_storage_dir_override(layer, storage_dir))
|
||||
}
|
||||
|
||||
fn render_resolve_errors(errors: Vec<fabro_config::ResolveError>) -> anyhow::Error {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue