Skip to content

Commit 5d7dac1

Browse files
max-sixtyclaude
andcommitted
refactor(cli): consolidate --no-verify deprecation into one path (#2868)
The deprecated `--no-verify` flag was declared as a clap arg five times — across `SwitchArgs`, `RemoveArgs`, `CommitArgs`, `SquashArgs`, and `MergeArgs` — under two field names, resolved through two helpers, with the deprecation-warning string hand-duplicated in two places. This collapses the four bool-resolving commands (`switch`, `remove`, `step commit`, `step squash`) onto a single flattened `HookFlags` args struct with one `resolve()` method, and routes every `--no-verify` deprecation warning — merge's included — through one emitter, so the warning text exists in exactly one place. `wt merge` keeps its own flags: its hooks flag is genuinely tri-state (`Option<bool>`, so config `[merge] verify` still applies) with a positive `--verify` override, which `HookFlags`'s `SetFalse`/default-true shape cannot express. It shares only the warning emitter — forcing it into the struct would need a special case. Behavior is unchanged: `--no-verify` still works as a deprecated alias and emits the same warning under the same conditions. One visible `--help` change, a direct consequence of the shared struct: `--no-hooks` for `step commit`/`step squash` moves under the `Automation` heading, matching where `switch`/`remove`/`merge` already place it. Doc mirrors regenerated. > _This was written by Claude Code on behalf of Maximilian Roos_ Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 4fe3f19 commit 5d7dac1

6 files changed

Lines changed: 101 additions & 60 deletions

File tree

docs/content/step.md

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -138,9 +138,6 @@ Usage: <b><span class=c>wt step commit</span></b> <span class=c>[OPTIONS]</span>
138138
<b><span class=c>-b</span></b>, <b><span class=c>--branch</span></b><span class=c> &lt;BRANCH&gt;</span>
139139
Branch to operate on (defaults to current worktree)
140140

141-
<b><span class=c>--no-hooks</span></b>
142-
Skip hooks
143-
144141
<b><span class=c>--stage</span></b><span class=c> &lt;STAGE&gt;</span>
145142
What to stage before committing [default: all]
146143

@@ -156,6 +153,9 @@ Usage: <b><span class=c>wt step commit</span></b> <span class=c>[OPTIONS]</span>
156153
Print help (see a summary with &#39;-h&#39;)
157154

158155
<b><span class=g>Automation:</span></b>
156+
<b><span class=c>--no-hooks</span></b>
157+
Skip hooks
158+
159159
<b><span class=c>--format</span></b><span class=c> &lt;FORMAT&gt;</span>
160160
Output format
161161

@@ -233,9 +233,6 @@ Usage: <b><span class=c>wt step squash</span></b> <span class=c>[OPTIONS]</span>
233233
Defaults to default branch.
234234

235235
<b><span class=g>Options:</span></b>
236-
<b><span class=c>--no-hooks</span></b>
237-
Skip hooks
238-
239236
<b><span class=c>--stage</span></b><span class=c> &lt;STAGE&gt;</span>
240237
What to stage before committing [default: all]
241238

@@ -251,6 +248,9 @@ Usage: <b><span class=c>wt step squash</span></b> <span class=c>[OPTIONS]</span>
251248
Print help (see a summary with &#39;-h&#39;)
252249

253250
<b><span class=g>Automation:</span></b>
251+
<b><span class=c>--no-hooks</span></b>
252+
Skip hooks
253+
254254
<b><span class=c>--format</span></b><span class=c> &lt;FORMAT&gt;</span>
255255
Output format
256256

skills/worktrunk/reference/step.md

Lines changed: 6 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/cli/mod.rs

Lines changed: 37 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,39 @@ pub(crate) struct Cli {
266266
pub command: Option<Commands>,
267267
}
268268

269+
/// Shared `--no-hooks` / `--no-verify` flags for commands that resolve hook
270+
/// skipping to a plain `bool` (`switch`, `remove`, `step commit`,
271+
/// `step squash`).
272+
///
273+
/// `wt merge` does not flatten this struct: its hooks flag is tri-state
274+
/// (`Option<bool>`, so config `[merge] verify` can still apply) and it carries
275+
/// a positive `--verify` override. It declares its own flags but routes the
276+
/// `--no-verify` deprecation through `crate::warn_no_verify_deprecated`, so the
277+
/// warning text lives in exactly one place.
278+
#[derive(Args)]
279+
pub(crate) struct HookFlags {
280+
/// Skip hooks
281+
#[arg(long = "no-hooks", action = clap::ArgAction::SetFalse, default_value_t = true, help_heading = "Automation")]
282+
pub(crate) verify: bool,
283+
284+
/// Skip hooks (deprecated alias for --no-hooks)
285+
#[arg(long = "no-verify", hide = true)]
286+
pub(crate) no_verify_deprecated: bool,
287+
}
288+
289+
impl HookFlags {
290+
/// Resolve to the effective verify value, emitting the deprecation warning
291+
/// once if `--no-verify` was used.
292+
pub(crate) fn resolve(&self) -> bool {
293+
if self.no_verify_deprecated {
294+
crate::warn_no_verify_deprecated();
295+
false
296+
} else {
297+
self.verify
298+
}
299+
}
300+
}
301+
269302
#[derive(Args)]
270303
pub(crate) struct SwitchArgs {
271304
/// Branch name or shortcut
@@ -344,13 +377,8 @@ pub(crate) struct SwitchArgs {
344377
#[arg(long, overrides_with = "no_cd", hide = true)]
345378
pub(crate) cd: bool,
346379

347-
/// Skip hooks
348-
#[arg(long = "no-hooks", action = clap::ArgAction::SetFalse, default_value_t = true, help_heading = "Automation")]
349-
pub(crate) verify: bool,
350-
351-
/// Skip hooks (deprecated alias for --no-hooks)
352-
#[arg(long = "no-verify", hide = true)]
353-
pub(crate) no_verify_deprecated: bool,
380+
#[command(flatten)]
381+
pub(crate) hooks: HookFlags,
354382

355383
/// Output format
356384
///
@@ -420,13 +448,8 @@ pub(crate) struct RemoveArgs {
420448
#[arg(long)]
421449
pub(crate) foreground: bool,
422450

423-
/// Skip hooks
424-
#[arg(long = "no-hooks", action = clap::ArgAction::SetFalse, default_value_t = true, help_heading = "Automation")]
425-
pub(crate) verify: bool,
426-
427-
/// Skip hooks (deprecated alias for --no-hooks)
428-
#[arg(long = "no-verify", hide = true)]
429-
pub(crate) no_verify_deprecated: bool,
451+
#[command(flatten)]
452+
pub(crate) hooks: HookFlags,
430453

431454
/// Force worktree removal
432455
///

src/cli/step.rs

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,8 @@ pub struct CommitArgs {
66
#[arg(short, long, add = crate::completion::worktree_only_completer())]
77
pub(crate) branch: Option<String>,
88

9-
/// Skip hooks
10-
#[arg(long = "no-hooks", action = clap::ArgAction::SetFalse, default_value_t = true)]
11-
pub(crate) verify: bool,
12-
13-
/// Skip hooks (deprecated alias for --no-hooks)
14-
#[arg(long = "no-verify", hide = true)]
15-
pub(crate) no_verify_deprecated: bool,
9+
#[command(flatten)]
10+
pub(crate) hooks: crate::cli::HookFlags,
1611

1712
/// What to stage before committing [default: all]
1813
#[arg(long)]
@@ -41,13 +36,8 @@ pub struct SquashArgs {
4136
#[arg(add = crate::completion::branch_value_completer())]
4237
pub(crate) target: Option<String>,
4338

44-
/// Skip hooks
45-
#[arg(long = "no-hooks", action = clap::ArgAction::SetFalse, default_value_t = true)]
46-
pub(crate) verify: bool,
47-
48-
/// Skip hooks (deprecated alias for --no-hooks)
49-
#[arg(long = "no-verify", hide = true)]
50-
pub(crate) no_verify_deprecated: bool,
39+
#[command(flatten)]
40+
pub(crate) hooks: crate::cli::HookFlags,
5141

5242
/// What to stage before committing [default: all]
5343
#[arg(long)]

src/main.rs

Lines changed: 15 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -144,18 +144,16 @@ fn warn_select_deprecated() {
144144
);
145145
}
146146

147-
/// Resolve the `--no-hooks` / `--no-verify` pair: emit a deprecation warning
148-
/// if the old flag was used, then return the effective verify value.
149-
fn resolve_verify(verify: bool, no_verify_deprecated: bool) -> bool {
150-
if no_verify_deprecated {
151-
eprintln!(
152-
"{}",
153-
warning_message("--no-verify is deprecated; use --no-hooks instead")
154-
);
155-
false
156-
} else {
157-
verify
158-
}
147+
/// Emit the canonical `--no-verify` deprecation warning to stderr.
148+
///
149+
/// Single source of this warning text. `HookFlags::resolve` (switch, remove,
150+
/// step commit, step squash) and `handle_merge_command` both call here, so the
151+
/// message stays identical across every command that accepts `--no-verify`.
152+
pub(crate) fn warn_no_verify_deprecated() {
153+
eprintln!(
154+
"{}",
155+
warning_message("--no-verify is deprecated; use --no-hooks instead")
156+
);
159157
}
160158

161159
fn handle_hook_command(action: HookCommand, yes: bool) -> anyhow::Result<()> {
@@ -201,7 +199,7 @@ fn handle_hook_command(action: HookCommand, yes: bool) -> anyhow::Result<()> {
201199
fn handle_step_command(action: StepCommand, yes: bool) -> anyhow::Result<()> {
202200
match action {
203201
StepCommand::Commit(args) => {
204-
let verify = resolve_verify(args.verify, args.no_verify_deprecated);
202+
let verify = args.hooks.resolve();
205203
let format = args.format;
206204
// `--show-prompt` and `--dry-run` emit raw text (rendered prompt or LLM
207205
// preview), which would corrupt a JSON consumer's stdout. Refuse the
@@ -230,7 +228,7 @@ fn handle_step_command(action: StepCommand, yes: bool) -> anyhow::Result<()> {
230228
Ok(())
231229
}
232230
StepCommand::Squash(args) => {
233-
let verify = resolve_verify(args.verify, args.no_verify_deprecated);
231+
let verify = args.hooks.resolve();
234232
// `--show-prompt` and `--dry-run` emit raw text (rendered prompt or LLM
235233
// preview), which would corrupt a JSON consumer's stdout.
236234
if args.format == SwitchFormat::Json && (args.show_prompt || args.dry_run) {
@@ -670,7 +668,7 @@ fn handle_select_command(_branches: bool, _remotes: bool) -> anyhow::Result<()>
670668
}
671669

672670
fn handle_switch_command(args: SwitchArgs, yes: bool) -> anyhow::Result<()> {
673-
let verify = resolve_verify(args.verify, args.no_verify_deprecated);
671+
let verify = args.hooks.resolve();
674672

675673
// With no branch argument, `wt switch` opens a TUI picker — config
676674
// deprecation warnings would render above the picker and push it down.
@@ -896,7 +894,7 @@ fn validate_remove_targets(
896894
/// or success message. See [`commands::process::run_internal_sweep`].
897895
fn handle_remove_command(args: RemoveArgs, yes: bool) -> anyhow::Result<()> {
898896
let json_mode = args.format == SwitchFormat::Json;
899-
let verify = resolve_verify(args.verify, args.no_verify_deprecated);
897+
let verify = args.hooks.resolve();
900898
UserConfig::load()
901899
.context("Failed to load config")
902900
.and_then(|config| {
@@ -1331,10 +1329,7 @@ fn init_logging(verbose_level: u8) {
13311329

13321330
fn handle_merge_command(args: MergeArgs, yes: bool) -> anyhow::Result<()> {
13331331
if args.no_verify {
1334-
eprintln!(
1335-
"{}",
1336-
warning_message("--no-verify is deprecated; use --no-hooks instead")
1337-
);
1332+
warn_no_verify_deprecated();
13381333
}
13391334
handle_merge(MergeOptions {
13401335
target: args.target.as_deref(),

tests/integration_tests/remove.rs

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2107,6 +2107,39 @@ approved-commands = ["echo 'hook ran' > {}"]
21072107
);
21082108
}
21092109

2110+
#[rstest]
2111+
fn test_remove_no_verify_deprecated_still_works(mut repo: TestRepo) {
2112+
let worktree_path = repo.add_worktree("feature-no-verify");
2113+
2114+
// --no-verify is a deprecated alias for --no-hooks: still works, still
2115+
// emits the shared deprecation warning that points at --no-hooks.
2116+
let output = repo
2117+
.wt_command()
2118+
.args([
2119+
"remove",
2120+
"--foreground",
2121+
"--yes",
2122+
"--no-verify",
2123+
"feature-no-verify",
2124+
])
2125+
.output()
2126+
.unwrap();
2127+
assert!(output.status.success());
2128+
let stderr = String::from_utf8_lossy(&output.stderr);
2129+
assert!(
2130+
stderr.contains("--no-verify is deprecated"),
2131+
"Expected deprecation warning in stderr: {stderr}"
2132+
);
2133+
assert!(
2134+
stderr.contains("--no-hooks"),
2135+
"Expected --no-hooks suggestion in stderr: {stderr}"
2136+
);
2137+
assert!(
2138+
!worktree_path.exists(),
2139+
"Worktree should be removed with --no-verify"
2140+
);
2141+
}
2142+
21102143
///
21112144
/// Even when a worktree is in detached HEAD state (no branch), the pre-remove
21122145
/// hook should still execute.

0 commit comments

Comments
 (0)