Skip to content

Commit adf7816

Browse files
drainpixiej178
andauthored
Fallback to manual stage for hooks specified directly in command line (#1185)
* feat(run): assume stage based on selected hooks * fix(run): don't clone eagerly * Remove fallback to `--all-files`, add tests * Generate cli docs * Improve wording --------- Co-authored-by: Jo <[email protected]>
1 parent 20dbb99 commit adf7816

File tree

6 files changed

+136
-15
lines changed

6 files changed

+136
-15
lines changed

docs/cli.md

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -204,8 +204,9 @@ prek run [OPTIONS] [HOOK|PROJECT]...
204204
</dd><dt id="prek-run--files"><a href="#prek-run--files"><code>--files</code></a> <i>files</i></dt><dd><p>Specific filenames to run hooks on</p>
205205
</dd><dt id="prek-run--from-ref"><a href="#prek-run--from-ref"><code>--from-ref</code></a>, <code>--source</code>, <code>-s</code> <i>from-ref</i></dt><dd><p>The original ref in a <code>&lt;from_ref&gt;...&lt;to_ref&gt;</code> diff expression. Files changed in this diff will be run through the hooks</p>
206206
</dd><dt id="prek-run--help"><a href="#prek-run--help"><code>--help</code></a>, <code>-h</code></dt><dd><p>Display the concise help for this command</p>
207-
</dd><dt id="prek-run--hook-stage"><a href="#prek-run--hook-stage"><code>--hook-stage</code></a> <i>hook-stage</i></dt><dd><p>The stage during which the hook is fired</p>
208-
<p>[default: pre-commit]</p><p>Possible values:</p>
207+
</dd><dt id="prek-run--hook-stage"><a href="#prek-run--hook-stage"><code>--hook-stage</code></a> <i>hook-stage</i></dt><dd><p>The stage during which the hook is fired.</p>
208+
<p>When specified, only hooks configured for that stage (for example <code>manual</code>, <code>pre-commit</code>, or <code>pre-commit</code>) will run. Defaults to <code>pre-commit</code> if not specified. For hooks specified directly in the command line, fallback to <code>manual</code> stage if no hooks found for <code>pre-commit</code> stage.</p>
209+
<p>Possible values:</p>
209210
<ul>
210211
<li><code>manual</code></li>
211212
<li><code>commit-msg</code></li>
@@ -761,8 +762,9 @@ prek try-repo [OPTIONS] <REPO> [HOOK|PROJECT]...
761762
</dd><dt id="prek-try-repo--files"><a href="#prek-try-repo--files"><code>--files</code></a> <i>files</i></dt><dd><p>Specific filenames to run hooks on</p>
762763
</dd><dt id="prek-try-repo--from-ref"><a href="#prek-try-repo--from-ref"><code>--from-ref</code></a>, <code>--source</code>, <code>-s</code> <i>from-ref</i></dt><dd><p>The original ref in a <code>&lt;from_ref&gt;...&lt;to_ref&gt;</code> diff expression. Files changed in this diff will be run through the hooks</p>
763764
</dd><dt id="prek-try-repo--help"><a href="#prek-try-repo--help"><code>--help</code></a>, <code>-h</code></dt><dd><p>Display the concise help for this command</p>
764-
</dd><dt id="prek-try-repo--hook-stage"><a href="#prek-try-repo--hook-stage"><code>--hook-stage</code></a> <i>hook-stage</i></dt><dd><p>The stage during which the hook is fired</p>
765-
<p>[default: pre-commit]</p><p>Possible values:</p>
765+
</dd><dt id="prek-try-repo--hook-stage"><a href="#prek-try-repo--hook-stage"><code>--hook-stage</code></a> <i>hook-stage</i></dt><dd><p>The stage during which the hook is fired.</p>
766+
<p>When specified, only hooks configured for that stage (for example <code>manual</code>, <code>pre-commit</code>, or <code>pre-commit</code>) will run. Defaults to <code>pre-commit</code> if not specified. For hooks specified directly in the command line, fallback to <code>manual</code> stage if no hooks found for <code>pre-commit</code> stage.</p>
767+
<p>Possible values:</p>
766768
<ul>
767769
<li><code>manual</code></li>
768770
<li><code>commit-msg</code></li>

src/cli/hook_impl.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ pub(crate) async fn hook_impl(
112112
config,
113113
includes,
114114
skips,
115-
hook_type.into(),
115+
Some(hook_type.into()),
116116
run_args.from_ref,
117117
run_args.to_ref,
118118
run_args.all_files,

src/cli/mod.rs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -445,8 +445,13 @@ pub(crate) struct RunArgs {
445445
pub(crate) last_commit: bool,
446446

447447
/// The stage during which the hook is fired.
448-
#[arg(long, default_value_t = Stage::PreCommit, value_enum)]
449-
pub(crate) hook_stage: Stage,
448+
///
449+
/// When specified, only hooks configured for that stage (for example `manual`,
450+
/// `pre-commit`, or `pre-commit`) will run.
451+
/// Defaults to `pre-commit` if not specified.
452+
/// For hooks specified directly in the command line, fallback to `manual` stage if no hooks found for `pre-commit` stage.
453+
#[arg(long, value_enum)]
454+
pub(crate) hook_stage: Option<Stage>,
450455

451456
/// When hooks fail, run `git diff` directly afterward.
452457
#[arg(long)]

src/cli/run/run.rs

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ pub(crate) async fn run(
3636
config: Option<PathBuf>,
3737
includes: Vec<String>,
3838
skips: Vec<String>,
39-
hook_stage: Stage,
39+
hook_stage: Option<Stage>,
4040
from_ref: Option<String>,
4141
to_ref: Option<String>,
4242
all_files: bool,
@@ -59,7 +59,7 @@ pub(crate) async fn run(
5959
};
6060

6161
// Prevent recursive post-checkout hooks.
62-
if hook_stage == Stage::PostCheckout
62+
if hook_stage == Some(Stage::PostCheckout)
6363
&& EnvVars::is_set(EnvVars::PREK_INTERNAL__SKIP_POST_CHECKOUT)
6464
{
6565
return Ok(ExitStatus::Success);
@@ -91,15 +91,15 @@ pub(crate) async fn run(
9191
.init_hooks(store, Some(&reporter))
9292
.await
9393
.context("Failed to init hooks")?;
94-
let filtered_hooks: Vec<_> = hooks
94+
let selected_hooks: Vec<_> = hooks
9595
.into_iter()
9696
.filter(|h| selectors.matches_hook(h))
9797
.map(Arc::new)
9898
.collect();
9999

100100
selectors.report_unused();
101101

102-
if filtered_hooks.is_empty() {
102+
if selected_hooks.is_empty() {
103103
writeln!(
104104
printer.stderr(),
105105
"{}: No hooks found after filtering with the given selectors",
@@ -117,10 +117,32 @@ pub(crate) async fn run(
117117
return Ok(ExitStatus::Failure);
118118
}
119119

120-
let filtered_hooks = filtered_hooks
121-
.into_iter()
122-
.filter(|h| h.stages.contains(hook_stage))
123-
.collect::<Vec<_>>();
120+
let (filtered_hooks, hook_stage) = if let Some(hook_stage) = hook_stage {
121+
let hooks = selected_hooks
122+
.iter()
123+
.filter(|h| h.stages.contains(hook_stage))
124+
.cloned()
125+
.collect::<Vec<_>>();
126+
(hooks, hook_stage)
127+
} else {
128+
// Try filtering by `pre-commit` stage first.
129+
let mut hook_stage = Stage::PreCommit;
130+
let mut hooks = selected_hooks
131+
.iter()
132+
.filter(|h| h.stages.contains(Stage::PreCommit))
133+
.cloned()
134+
.collect::<Vec<_>>();
135+
if hooks.is_empty() && selectors.includes_only_hook_targets() {
136+
// If no hooks found for `pre-commit` stage, try fallback to `manual` stage for hooks specified directly.
137+
hook_stage = Stage::Manual;
138+
hooks = selected_hooks
139+
.iter()
140+
.filter(|h| h.stages.contains(Stage::Manual))
141+
.cloned()
142+
.collect();
143+
}
144+
(hooks, hook_stage)
145+
};
124146

125147
if filtered_hooks.is_empty() {
126148
writeln!(

src/cli/run/selector.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,16 @@ impl Selectors {
202202
})
203203
}
204204

205+
pub(crate) fn includes_only_hook_targets(&self) -> bool {
206+
!self.includes.is_empty()
207+
&& self.includes.iter().all(|s| {
208+
matches!(
209+
s.expr,
210+
SelectorExpr::HookId(_) | SelectorExpr::ProjectHook { .. }
211+
)
212+
})
213+
}
214+
205215
/// Check if a hook matches any of the selection criteria.
206216
pub(crate) fn matches_hook(&self, hook: &Hook) -> bool {
207217
let mut usage = self.usage.lock().unwrap();

tests/run.rs

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -576,6 +576,88 @@ fn stage() {
576576
"#);
577577
}
578578

579+
#[test]
580+
fn fallback_to_manual_stage() {
581+
let context = TestContext::new();
582+
context.init_project();
583+
context.write_pre_commit_config(indoc::indoc! {r"
584+
repos:
585+
- repo: local
586+
hooks:
587+
- id: manual-only
588+
name: manual-only
589+
language: system
590+
entry: echo manual-only
591+
stages: [ manual ]
592+
- id: another-manual
593+
name: another-manual
594+
language: system
595+
entry: echo another-manual
596+
stages: [ manual ]
597+
- id: default-stage
598+
name: default-stage
599+
language: system
600+
entry: echo default-stage
601+
- id: pre-push
602+
name: pre-push
603+
language: system
604+
entry: echo pre-push
605+
stages: [ pre-push ]
606+
"});
607+
context.git_add(".");
608+
609+
// With pre-commit hooks present, default `prek run` stays on pre-commit.
610+
cmd_snapshot!(context.filters(), context.run(), @r"
611+
success: true
612+
exit_code: 0
613+
----- stdout -----
614+
default-stage............................................................Passed
615+
616+
----- stderr -----
617+
");
618+
619+
// Explicit `--hook-stage pre-commit` keeps execution scoped to that stage.
620+
cmd_snapshot!(context.filters(), context.run().arg("--hook-stage").arg("pre-commit").arg("default-stage").arg("manual-only"), @r"
621+
success: true
622+
exit_code: 0
623+
----- stdout -----
624+
default-stage............................................................Passed
625+
626+
----- stderr -----
627+
");
628+
629+
// Selecting manual + pre-commit hooks still runs only the pre-commit ones.
630+
cmd_snapshot!(context.filters(), context.run().arg("manual-only").arg("default-stage"), @r"
631+
success: true
632+
exit_code: 0
633+
----- stdout -----
634+
default-stage............................................................Passed
635+
636+
----- stderr -----
637+
");
638+
639+
// Selecting only manual hooks should still succeed via fallback.
640+
cmd_snapshot!(context.filters(), context.run().arg("manual-only").arg("another-manual"), @r"
641+
success: true
642+
exit_code: 0
643+
----- stdout -----
644+
manual-only..............................................................Passed
645+
another-manual...........................................................Passed
646+
647+
----- stderr -----
648+
");
649+
650+
// Mixing `pre-push` and manual selectors still runs the manual hook via fallback.
651+
cmd_snapshot!(context.filters(), context.run().arg("pre-push").arg("manual-only"), @r"
652+
success: true
653+
exit_code: 0
654+
----- stdout -----
655+
manual-only..............................................................Passed
656+
657+
----- stderr -----
658+
");
659+
}
660+
579661
/// Test global `files`, `exclude`, and hook level `files`, `exclude`.
580662
#[test]
581663
fn files_and_exclude() -> Result<()> {

0 commit comments

Comments
 (0)