Repository navigation
fix(watch): forward --ignore, --ignore-file, and --print-events to watchexec - #10629
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthrough
ChangesWatch flag forwarding
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThree
Confidence Score: 5/5Safe to merge — the change is a minimal, targeted addition of three missing arg-forwarding blocks, each following a well-established pattern already in the file. The fix is mechanical: three missing forwarding branches added in the correct location, matching every other flag-forwarding block in the function. The watchexec union semantics for repeated --ignore flags mean the new user-supplied ignores compose correctly with the existing auto-derived ones. The regression test covers all three flags end-to-end. No files require special attention. Important Files Changed
Reviews (2): Last reviewed commit: "fix(watch): forward --ignore, --ignore-f..." | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@e2e/cli/test_watch_ignore`:
- Around line 19-23: The watch-ignore test is waiting on any occurrence of
“watchexec,” which can be satisfied by unrelated output and make the test flaky.
Update the polling in the test to wait for the specific constructed debug
command line emitted by the watchexec launch path, using the exact debug
prefix/message from the watch startup flow so the loop only exits once the
actual command is logged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: ee03e2c6-457e-4315-a953-0d7da6ba6f4c
📒 Files selected for processing (2)
e2e/cli/test_watch_ignoresrc/cli/watch.rs
…tchexec `mise watch` parses watchexec flags into its own `WatchexecArgs` struct and then rebuilds a fresh `watchexec ... -- mise run <task>` command line in `Watch::run()`. Each flag must be explicitly re-emitted there, but `--ignore` (`ignore_patterns`), `--ignore-file` (`ignore_files`), and `--print-events` (`print_events`) were parsed and silently dropped, so `mise watch --ignore 'spec/**/*.rb'` had no effect. Add the three missing forwarding branches, mirroring the existing flag-forwarding style (and the jdx#10392 `--wrap-process` fix). The user's `--ignore` patterns are emitted as a separate, additional `--ignore` occurrence; watchexec unions repeated `--ignore` flags, so they combine with the source-derived ignores rather than clobbering them. Adds an e2e regression test that asserts the three flags appear on the constructed watchexec command line. Resolves the issue reported in discussion jdx#7776. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1564137 to
e974731
Compare
Problem
mise watch --ignore 'spec/**/*.rb' ...has no effect — files matching thepattern still trigger a restart (reported in discussion #7776).
mise watchis not a transparent pass-through towatchexec. It declares thewatchexec flags itself (
#[clap(flatten)] watchexec: WatchexecArgs), parsesthem, then rebuilds a fresh
watchexec ... -- mise run <task>command linein
Watch::run(). Every flag has to be explicitly re-emitted there, and threewere parsed but never forwarded:
--ignore→WatchexecArgs::ignore_patterns--ignore-file→WatchexecArgs::ignore_files--print-events→WatchexecArgs::print_eventsThe reporter's own
mise watch -vdebug log confirms it: the constructedwatchexec --restart --exts .rb -- mise run ...line contains--exts/--restart(which have forwarding branches) but not--ignore.Note: the existing
--ignorepush only emits the auto-derived ignorescomputed from task sources (
merge_watch_patterns), not the user'signore_patterns— a name collision that hid the missing forwarding.Fix
Add the three missing forwarding branches in
Watch::run(), matching theexisting flag-forwarding style and the earlier #10392 (
--wrap-process) fix forthe same class of bug. The user's
--ignorepatterns are emitted as anadditional
--ignoreoccurrence; watchexec unions repeated--ignoreflags, sothey combine with the source-derived ignores rather than clobbering them (the
existing auto-ignore block is untouched).
Scope is deliberately limited to the three flags named in the discussion to keep
the change minimal; other unforwarded
WatchexecArgsflags are out of scope.Test
Adds
e2e/cli/test_watch_ignore, which runsmise watchwith the three flagsand asserts they appear on the constructed watchexec command line (the
$ watchexec ...line mise logs underMISE_DEBUG=1).Resolves the issue reported in discussion #7776.
🤖 Generated with Claude Code
Summary by CodeRabbit
mise watchnow properly forwards user-specified watch filtering and debug options to the underlying watcher, including--ignorepatterns,--ignore-file, and--print-events.--ignore,--ignore-file, and--print-events.