Skip to content

Commit 9e67fc9

Browse files
committed
refactor: address review comments on task runner summary
- Make --last-details exclusive at type level by splitting Command into ParsedCommand (clap) and Command (resolved with RunLastDetails variant) - Use ? to propagate IO errors in show_last_run_details - Replace summary_file_path with write_summary callback to decouple reporter from file paths - Remove ExecutionInfo/ExecutionStats, build TaskSummary directly in leaf reporter's finish() instead of collecting intermediate state - Add .gitignore to oxlint fixtures to prevent fspy from fingerprinting the cache directory, fixing flaky cache-miss snapshots
1 parent ddbbb94 commit 9e67fc9

File tree

13 files changed

+231
-241
lines changed

13 files changed

+231
-241
lines changed

crates/vite_task/src/cli/mod.rs

Lines changed: 73 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -36,36 +36,100 @@ pub struct RunFlags {
3636
pub details: bool,
3737
}
3838

39-
/// Arguments for the `run` subcommand.
39+
// ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
40+
// Clap-parsed types (used only at the parsing boundary)
41+
// ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
42+
43+
/// Arguments for the `run` subcommand as parsed by clap.
44+
///
45+
/// Contains the `--last-details` flag which is resolved into a separate
46+
/// [`Command::RunLastDetails`] variant via [`ParsedCommand::into_command`].
4047
#[derive(Debug, clap::Args)]
41-
pub struct RunCommand {
48+
pub struct ParsedRunCommand {
4249
/// `packageName#taskName` or `taskName`. If omitted, lists all available tasks.
43-
pub task_specifier: Option<TaskSpecifier>,
50+
task_specifier: Option<TaskSpecifier>,
4451

4552
#[clap(flatten)]
46-
pub flags: RunFlags,
53+
flags: RunFlags,
4754

4855
/// Additional arguments to pass to the tasks
4956
#[clap(trailing_var_arg = true, allow_hyphen_values = true)]
50-
pub additional_args: Vec<Str>,
57+
additional_args: Vec<Str>,
5158

5259
/// Display the detailed summary of the last run.
5360
#[clap(long, exclusive = true)]
54-
pub last_details: bool,
61+
last_details: bool,
5562
}
5663

57-
/// vite task CLI subcommands
64+
/// vite task CLI subcommands as parsed by clap.
65+
///
66+
/// Use [`ParsedCommand::into_command`] to resolve into the dispatched [`Command`]
67+
/// enum, which makes `--last-details` exclusive at the type level.
5868
#[derive(Debug, Parser)]
59-
pub enum Command {
69+
pub enum ParsedCommand {
6070
/// Run tasks
61-
Run(RunCommand),
71+
Run(ParsedRunCommand),
6272
/// Manage the task cache
6373
Cache {
6474
#[clap(subcommand)]
6575
subcmd: CacheSubcommand,
6676
},
6777
}
6878

79+
impl ParsedCommand {
80+
/// Resolve the clap-parsed command into the dispatched [`Command`] enum.
81+
///
82+
/// When `--last-details` is set on the `run` subcommand, this produces
83+
/// [`Command::RunLastDetails`] instead of [`Command::Run`], making the
84+
/// exclusivity enforced at the type level.
85+
#[must_use]
86+
pub fn into_command(self) -> Command {
87+
match self {
88+
Self::Run(run) if run.last_details => Command::RunLastDetails,
89+
Self::Run(run) => Command::Run(RunCommand {
90+
task_specifier: run.task_specifier,
91+
flags: run.flags,
92+
additional_args: run.additional_args,
93+
}),
94+
Self::Cache { subcmd } => Command::Cache { subcmd },
95+
}
96+
}
97+
}
98+
99+
// ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
100+
// Resolved types (used for dispatch — `--last-details` is a separate variant)
101+
// ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
102+
103+
/// Resolved CLI command for dispatch.
104+
///
105+
/// Unlike [`ParsedCommand`], this enum makes `--last-details` a separate variant
106+
/// ([`Command::RunLastDetails`]) so that it is exclusive at the type level —
107+
/// there is no way to combine it with task execution fields.
108+
#[derive(Debug)]
109+
pub enum Command {
110+
/// Run tasks with the given parameters.
111+
Run(RunCommand),
112+
/// Display the saved detailed summary of the last run (`--last-details`).
113+
RunLastDetails,
114+
/// Manage the task cache.
115+
Cache { subcmd: CacheSubcommand },
116+
}
117+
118+
/// Resolved arguments for executing tasks.
119+
///
120+
/// Does not contain `last_details` — that case is represented by
121+
/// [`Command::RunLastDetails`] instead.
122+
#[derive(Debug)]
123+
pub struct RunCommand {
124+
/// `packageName#taskName` or `taskName`. If omitted, lists all available tasks.
125+
pub task_specifier: Option<TaskSpecifier>,
126+
127+
pub flags: RunFlags,
128+
129+
/// Additional arguments to pass to the tasks.
130+
pub additional_args: Vec<Str>,
131+
}
132+
69133
#[derive(thiserror::Error, Debug)]
70134
pub enum CLITaskQueryError {
71135
#[error("no task specifier provided")]
@@ -93,7 +157,6 @@ impl RunCommand {
93157
task_specifier,
94158
flags: RunFlags { recursive, transitive, ignore_depends_on, .. },
95159
additional_args,
96-
..
97160
} = self;
98161

99162
let task_specifier = task_specifier.ok_or(CLITaskQueryError::MissingTaskSpecifier)?;

crates/vite_task/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ mod maybe_str;
44
pub mod session;
55

66
// Public exports for vite_task_bin
7-
pub use cli::{CacheSubcommand, Command, RunCommand, RunFlags};
7+
pub use cli::{CacheSubcommand, Command, ParsedCommand, RunCommand, RunFlags};
88
pub use session::{CommandHandler, ExitStatus, HandledCommand, Session, SessionCallbacks};
99
pub use vite_task_graph::{
1010
config::{

crates/vite_task/src/session/mod.rs

Lines changed: 25 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -101,9 +101,11 @@ impl vite_task_plan::PlanRequestParser for PlanRequestParser<'_> {
101101
match self.command_handler.handle_command(command).await? {
102102
HandledCommand::Synthesized(synthetic) => Ok(Some(PlanRequest::Synthetic(synthetic))),
103103
HandledCommand::ViteTaskCommand(cli_command) => match cli_command {
104-
Command::Cache { .. } => Ok(Some(PlanRequest::Synthetic(
105-
command.to_synthetic_plan_request(UserCacheConfig::disabled()),
106-
))),
104+
Command::Cache { .. } | Command::RunLastDetails => {
105+
Ok(Some(PlanRequest::Synthetic(
106+
command.to_synthetic_plan_request(UserCacheConfig::disabled()),
107+
)))
108+
}
107109
Command::Run(run_command) => {
108110
match run_command.into_query_plan_request(&command.cwd) {
109111
Ok(query_plan_request) => Ok(Some(PlanRequest::Query(query_plan_request))),
@@ -227,12 +229,8 @@ impl<'a> Session<'a> {
227229
pub async fn main(mut self, command: Command) -> anyhow::Result<ExitStatus> {
228230
match command {
229231
Command::Cache { ref subcmd } => self.handle_cache_command(subcmd),
232+
Command::RunLastDetails => self.show_last_run_details(),
230233
Command::Run(run_command) => {
231-
// --last-details: display saved summary and exit (exclusive flag)
232-
if run_command.last_details {
233-
return self.show_last_run_details();
234-
}
235-
236234
let cwd = Arc::clone(&self.cwd);
237235
let is_interactive =
238236
std::io::stdin().is_terminal() && std::io::stdout().is_terminal();
@@ -259,7 +257,7 @@ impl<'a> Session<'a> {
259257
self.workspace_path(),
260258
Box::new(tokio::io::stdout()),
261259
show_details,
262-
Some(self.summary_file_path()),
260+
Some(self.make_summary_writer()),
263261
);
264262
Ok(self
265263
.execute_graph(graph, Box::new(builder))
@@ -401,20 +399,16 @@ impl<'a> Session<'a> {
401399
let selected_label = &select_items[selected_index].label;
402400
let task_specifier = TaskSpecifier::parse_raw(selected_label);
403401
let show_details = flags.details;
404-
let run_command = RunCommand {
405-
task_specifier: Some(task_specifier),
406-
flags,
407-
additional_args,
408-
last_details: false,
409-
};
402+
let run_command =
403+
RunCommand { task_specifier: Some(task_specifier), flags, additional_args };
410404

411405
let cwd = Arc::clone(&self.cwd);
412406
let graph = self.plan_from_cli_run(cwd, run_command).await?;
413407
let builder = LabeledReporterBuilder::new(
414408
self.workspace_path(),
415409
Box::new(tokio::io::stdout()),
416410
show_details,
417-
Some(self.summary_file_path()),
411+
Some(self.make_summary_writer()),
418412
);
419413
Ok(self.execute_graph(graph, Box::new(builder)).await.err().unwrap_or(ExitStatus::SUCCESS))
420414
}
@@ -439,6 +433,19 @@ impl<'a> Session<'a> {
439433
self.cache_path.join("last-summary.json")
440434
}
441435

436+
/// Create a callback that persists the summary to `last-summary.json`.
437+
///
438+
/// The returned closure captures the file path and handles errors internally
439+
/// (logging failures without propagating).
440+
fn make_summary_writer(&self) -> Box<dyn FnOnce(&LastRunSummary)> {
441+
let path = self.summary_file_path();
442+
Box::new(move |summary: &LastRunSummary| {
443+
if let Err(err) = summary.write_atomic(&path) {
444+
tracing::warn!("Failed to write summary to {path:?}: {err}");
445+
}
446+
})
447+
}
448+
442449
/// Display the saved summary from the last run (`--last-details`).
443450
#[expect(
444451
clippy::print_stderr,
@@ -452,8 +459,8 @@ impl<'a> Session<'a> {
452459
{
453460
use std::io::Write;
454461
let mut stdout = std::io::stdout().lock();
455-
let _ = stdout.write_all(&buf);
456-
let _ = stdout.flush();
462+
stdout.write_all(&buf)?;
463+
stdout.flush()?;
457464
}
458465
Ok(ExitStatus(summary.exit_code))
459466
}

0 commit comments

Comments
 (0)