fix: a flag before the workspace spec stopped tab completion - #592
Merged
Conversation
`aid --codex owner/repo<TAB>` offered no completions at all, and neither did `dl --devcontainer robot owner/repo<TAB>`. The completion script found the spec by counting words from the command -- word two for the spec, word three for a verb -- and neither grammar works that way. aid reads its leading flags and calls the first word that is not one the spec (`parse_aid_args`), and dl is clap, which puts options anywhere among the positional words. The script now scans the words before the cursor and counts the positional ones, stepping over a value option and its value and stopping at `--`, so the spec is wherever it actually lands. That needs one distinction the old code could not make: a flag that *is* the command (`--ls`, no workspace follows) against one that modifies a launch (`--rm`, a spec is still to come). The old guard ended completion on any leading `--`, so closing the gap for `--rm` would have opened `dl --ls <TAB>` up to offering workspaces to a command that takes none. The new table is clap's own `what` group, whose members are mutually exclusive because each one is the whole command, and `completion_tables.rs` diffs the two -- both halves, plus aid's. Claude-Session: https://claude.ai/code/session_01GxeUVm3R579Tqai6GwoEmb
The scan added in the previous commit put the default arm on the wrong side. It listed the flags that *end* a line and treated everything else as a launch modifier a spec still follows, and the flags nobody thinks to list are all on the ending side: dl --repos my-workspace "--repos takes no workspace" dl --update-cache my-workspace the same dl --json my-workspace clap error, --json requires --ls dl --yes my-workspace "--yes means nothing for a workspace command" dl --force my-workspace "Unknown workspace '--force'" dl --stop / --autorm retired, refused by name Ten in all: the command group's hidden members, which the `!hidden` filter dropped from the table, the five that need something already on the line, and the two retired spellings. Every one of them completed `[]` before, and the fix offered a workspace name for each -- tab to a name and get refused for something you never typed, which is the bar `test_the_completion_offers_only_names_a_launch_accepts` already holds the profile names to. The table is inverted: `spec_follows` lists the flags a spec may follow and every other flag ends the line, so the default arm is the refusal. That is three flags for dl rather than sixteen, still derived -- every flag, minus clap's `what` group, minus the hidden ones, minus `NOT_OFFERED_FIRST` -- and it also covers an unknown flag, which the listed-endings form got wrong for free. Two more, both caught by mutation: - `test_a_leading_modifier_leaves_the_verb_where_it_was` compared two empty lists and passed with the verb branch mutated to `position == 99`. Anchored, along with the `--` test beside it. - The sameness the report asked for includes whether the cursor is left against the `/`, which COMPREPLY does not show. The agent-flag test now diffs `compopt` too, across five spec shapes. `aid_flag_list` read one of `REMOTE_CONTROL_FLAGS`' two spellings, because the first entry is a named const rather than a literal, and compared the short list against the script -- the silent pass this file exists to prevent. It resolves the const and asserts one flag per entry. Claude-Session: https://claude.ai/code/session_01GxeUVm3R579Tqai6GwoEmb
Types axis: the field was documented as "the flag *is* the command, so no workspace follows it", which is true of --install and reads as though no *word* follows. `dl --install [<rc-file>]` takes an optional path, and nothing completes it. Unchanged by this branch and left alone: the path branch lives in the spec position, so completing it is a second exception rather than a wider table. Claude-Session: https://claude.ai/code/session_01GxeUVm3R579Tqai6GwoEmb
CHANGELOG under [Unreleased], and a docs/workspace-tools.md section under Shell completion. The rationale worth keeping is why the table lists the flags a spec may follow rather than the flags that end the line: the endings are the side nobody thinks to list, and each of them refuses a workspace for a different reason. Claude-Session: https://claude.ai/code/session_01GxeUVm3R579Tqai6GwoEmb
Reviewer's GuideFixes workspace and verb tab completion when aid or dl options precede the workspace spec by scanning positional arguments and using grammar-derived allowlists to distinguish launch modifiers from flags that terminate completion, with extensive regression and drift-detection coverage. Flow diagram for option-aware workspace completionflowchart TD
A["Completion invoked"] --> B["Scan words before cursor"]
B --> C{"Encounter --?"}
C -- Yes --> D["Stop completion"]
C -- No --> E{"Option takes a value?"}
E -- Yes --> F["Skip option and its value"]
E -- No --> G{"Flag in spec_follows?"}
F --> G
G -- No --> H["Refuse workspace and verb completion"]
G -- Yes --> I["Mark launch modifier"]
I --> J{"Count positional words"}
G --> J
J --> K{"Position 0?"}
K -- Yes --> L["Complete flags or workspace spec"]
K -- No --> M{"Position 1 and command is dl?"}
M -- Yes --> N["Complete workspace verb"]
M -- No --> O["No completion"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Merged
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.
aid --codex owner/repo<TAB>offered nothing at all. Neither didaid --claude,aid --gemini, ordl --devcontainer robot owner/repo<TAB>.What was wrong
The completion script found the workspace spec by counting words from the command: word two for the spec, word three for a verb. Neither grammar works that way.
parse_aid_args), so--codexdoes not stand in the spec's place.The fix
The script now scans the words before the cursor and counts the positional ones, stepping over a value option with its value and stopping at
--, so the spec is wherever it actually lands.That needs a distinction the old guard could not make, since it ended completion on any leading
--: a flag a spec may follow against one that ends the line.The direction of that table is the load-bearing part, and it is where the first attempt went wrong. Listing the flags that end a line put ten flags in the wrong arm at once, because the ones nobody thinks to list are all on that side: the command group's hidden members, the five that need something already on the line, and the two retired spellings. Each refuses a workspace, for four different reasons:
All ten completed
[]before, and the first attempt offered a workspace name for each. Tabbing to a name and then being refused for a word you never typed is worse than no completion, which is the bartest_the_completion_offers_only_names_a_launch_acceptsalready holds the profile names to. So the table lists the flags a spec may follow, three fordland nine foraid, and the default arm is the refusal. That also covers an unknown flag for free.Both tables are derived, not hand-judged
Per the standing rule on second copies of a fact,
rust/dl/tests/completion_tables.rsdiffs them:whatgroup, minus the hidden ones, minusNOT_OFFERED_FIRST. Leaves exactly--rm --devcontainer --claude-profile.parse_aid_argsreads past, minus the three aid answers itself.Writing that diff caught a hole in its own parser:
aid_flag_listread one ofREMOTE_CONTROL_FLAGS' two spellings, because the first entry is a named const rather than a literal, and compared the short list against the script. It now resolves the const and asserts one flag per entry.Verification
Built test-first. Every behavioural claim above is a test, and the three interesting mutations are caught: flipping the default arm back, disabling the verb branch, disabling the spec branch. Two of the tests were themselves defective on the first pass and are fixed here, both found by mutation:
position == 99;/, whichCOMPREPLYdoes not show, so the agent-flag test now diffscompopttoo across five spec shapes.cargo test --workspace,clippy -D warnings,cargo fmt, 757 python tests,pixi run lintall green.Known gap, left alone
dl --install [<rc-file>]takes an optional path and nothing completes it, here or before. The path branch lives inside the spec position, so completing it is a second exception rather than a wider table. Noted in the script.🤖 Generated with Claude Code
https://claude.ai/code/session_01GxeUVm3R579Tqai6GwoEmb
Summary by Sourcery
Fix workspace tab completion so it follows parsed positional arguments instead of assuming fixed word positions.
Bug Fixes:
--when the CLI will not accept a workspace in that position.Enhancements:
dlandaidargument grammars to keep completion behavior aligned with command parsing.Documentation:
Tests:
--, and aid prompt handling.