Commit 8f7626a
authored
refactor(cli): simplify abstractions (#574)
this PR refactors the `cli` crate. it currently has a bunch of issues
that stem from its original implementation in biome, where file/stdin
execution is the main use case.
- a `CommandRunner` trait that tries to abstract a few common code
blocks away.
- a `Dummy` traversal mode which essentially means "do not traverse".
- reports are tightly coupled to execution. we also need to report
output of commands that do not traverse (e.g. `dblint`)
- `execute_mode` does everything: stdin handling + traversal + reporting
+ exit codes. it cannot be re-used by commands that do not traverse
all of these issues cause my brain to have a hard time understanding
what is going on. This PR refactors the crate to gear it more towards
our use-case. I spent a few cycles designing the refactor, and at least
for me it makes much more sense now.
I also added a dummy `dblint` command that will be implemented in
follow-ups. right now, it prints an empty report.
Below is a richer summary of the issues described above.
## Current Problems
### 1. CommandRunner Trait Forces All Commands Through Same Pipeline
```rust
pub(crate) trait CommandRunner: Sized {
fn run(&mut self, session: CliSession, cli_options: &CliOptions) -> Result<()> {
// Forces: config loading → VCS setup → file traversal → reporting
let (execution, paths) = self.configure_workspace(fs, console, workspace, cli_options)?;
execute_mode(execution, session, cli_options, paths)
}
}
```
**Issues:**
- Only `Check` command uses this trait
- Commands like `version`, `clean`, `init` are ad-hoc functions
- No way for commands to skip parts they don't need (e.g., dblint needs
config but not traversal)
- Forces unnecessary complexity on simple commands
### 2. TraversalMode::Dummy Exists For Wrong Reasons
```rust
pub enum TraversalMode {
Dummy, // Only exists because commands are forced through traversal
Check { stdin: Option<Stdin>, vcs_targeted: VcsTargeted },
}
```
Commands that don't need traversal shouldn't have a "dummy" mode - they
just shouldn't traverse.
### 3. Execution Struct Conflates Concerns
```rust
pub struct Execution {
report_mode: ReportMode, // How to report (Terminal/GitHub/GitLab)
traversal_mode: TraversalMode, // How to process files
max_diagnostics: u32,
}
```
**Problem:** Bundles processing config with reporting config. Commands
that don't traverse (like `dblint`) still need reporting but don't need
traversal config.
### 4. execute_mode() is Monolithic
```rust
pub fn execute_mode(execution: Execution, session: CliSession, ...) -> Result<()> {
// Does: stdin handling + traversal + reporting + exit codes
// Can't be reused by commands that don't traverse
}
```
### 5. Scattered and Inconsistent Structure
- `commands/` has mix of trait impls and functions
- `execute/` has traversal logic tightly coupled to reporting
- No clear separation of workspace setup, execution, and reporting
- Commands repeat boilerplate for config loading, setup, reporting1 parent 02af023 commit 8f7626a
File tree
19 files changed
+859
-1066
lines changed- crates/pgt_cli
- src
- commands
- execute
- process_file
- reporter
- tests/snapshots
19 files changed
+859
-1066
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | | - | |
3 | | - | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
4 | 7 | | |
5 | 8 | | |
| 9 | + | |
6 | 10 | | |
7 | | - | |
| 11 | + | |
8 | 12 | | |
9 | 13 | | |
10 | | - | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
11 | 48 | | |
12 | | - | |
13 | | - | |
14 | | - | |
15 | | - | |
16 | | - | |
17 | | - | |
18 | | - | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
19 | 59 | | |
20 | 60 | | |
21 | | - | |
22 | | - | |
23 | | - | |
24 | | - | |
25 | | - | |
26 | | - | |
27 | | - | |
28 | | - | |
29 | | - | |
30 | | - | |
31 | | - | |
32 | | - | |
33 | | - | |
34 | | - | |
35 | | - | |
36 | | - | |
37 | | - | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
38 | 78 | | |
| 79 | + | |
39 | 80 | | |
40 | | - | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
41 | 96 | | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
42 | 103 | | |
43 | | - | |
44 | | - | |
45 | | - | |
46 | | - | |
47 | | - | |
48 | | - | |
49 | | - | |
50 | | - | |
51 | | - | |
52 | | - | |
53 | | - | |
54 | | - | |
55 | | - | |
56 | | - | |
57 | | - | |
| 104 | + | |
| 105 | + | |
58 | 106 | | |
59 | 107 | | |
60 | | - | |
61 | | - | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
62 | 114 | | |
63 | 115 | | |
64 | | - | |
65 | | - | |
66 | | - | |
67 | | - | |
68 | | - | |
69 | | - | |
70 | | - | |
71 | | - | |
72 | | - | |
73 | | - | |
74 | | - | |
| 116 | + | |
| 117 | + | |
75 | 118 | | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
76 | 138 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
0 commit comments