Make the refused --worktree flag pairs unrepresentable - #167
Merged
Conversation
`--worktree` cannot be combined with `--skip-branch` or `--keep-changes`. Until now three independent `Flag` fields on `OrcaArgs` could hold either pair, so the refusal had to be repeated at runtime: once in `OrcaArgs.parse`, and again in `flow()` for an `OrcaArgs` built by hand. The flags now become one `RunTarget` — `NewBranch(Uncommitted)`, `CurrentBranch(Uncommitted)` or `Worktree`, which carries neither a branch mode nor an `Uncommitted`. The five legal states are the only ones that can be written, so the check in `flow()` is gone. mainargs still needs raw `Flag` fields to parse argv, so those live on `RawArgs`, private to the package. `OrcaArgs.parse` is its only consumer and converts through `RunTarget.from` — the single place a refused pair is worded and rejected. `RunTarget.toArgv` renders the flags back, next to the parser that reads them, so the shell no longer spells them out itself.
`FlowFlags` carried the same three independent booleans as `OrcaArgs`, built at four launch sites, so any of them could hand the flow child a combination it would refuse after the spawn. It now carries a `RunTarget`, which cannot express one. The shell's own `RunTarget` (menu-only, same three destinations) is dropped for the shared type. `orca run` converts its argv flags with `RunTarget.from` and refuses a bad pair before resolving anything, as before — but now that conversion is the only way to reach a launch path, rather than a check a call site could skip.
Review follow-ups on the two commits before this one: - `flow()` asked `args.target == RunTarget.Worktree`; it now matches, so a new case cannot silently take the in-checkout path. - `--skip-branch --keep-changes` is the one combination of two flags orca allows, and it is now its own constructor — so it gets a parse test. - ADR 0018 named the three `OrcaArgs` fields the flags used to become.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
--worktreecannot be combined with--skip-branchor with--keep-changes.Both pairs were refused at runtime, but nothing stopped code from building one:
OrcaArgscarried three independentFlagfields, and the shell'sFlowFlagsthree independent
Booleans, built at four launch sites.The three flags now become one value once argv is parsed:
Worktreecarries neither a branch mode nor anUncommitted, so the five legalcombinations are the only ones that can be written.
What is now impossible
flow()is gone. It was there because anOrcaArgsbuilt by hand had never been through the parser. Such an
OrcaArgscan nolonger hold a refused pair, so there is nothing left to check.
FlowFlagsholds aRunTarget, so the four sites that build one (menu run, resume, flowauthoring,
orca run) cannot spawn a flow child that would refuse its ownargv.
RunTargetfor themenu; it is folded into the shared one rather than left as a second enum
meaning the same thing.
RunTarget.toArgvrenders the flags theshell appends after
--, next to the@argannotations that parse them back.What is still representable, and why
mainargs builds a case class straight from argv, so raw
Flagfields have toexist at the parse boundary. They moved to
RawArgs, private topackage orca, whose only consumer isOrcaArgs.parse.orca runhas the sameboundary — its own mainargs
@mainflags — and converts them the same way.Both conversions go through
RunTarget.from, the single place a refused pair isworded and rejected. So argv can still say
--worktree --skip-branch; nothingpast those two conversions can.
One derived pair of booleans is left:
DirtyTreeFacts(skipBranch, keepChanges),the dirty-tree policy's input, now filled from
target.skipBranch/target.keepChanges. It has noworktreefield, so it cannot express a refusedpair either.
ParserForClass[OrcaArgs]is no longer given, sinceOrcaArgsis no longer theparser's type. Flow scripts keep writing
flow(OrcaArgs(args)).No behaviour change
Same flags, same refusal wording and precedence (
--worktree --keep-changesstill names only those two), same exit codes, same argv passed to the flow
child.
sbt scalafmtAllclean,sbt testgreen: 2238 tests, 0 failures.