Skip to content

Add completion#86

Merged
floitsch merged 15 commits intomainfrom
floitsch/completion
Mar 19, 2026
Merged

Add completion#86
floitsch merged 15 commits intomainfrom
floitsch/completion

Conversation

@floitsch
Copy link
Member

No description provided.

@floitsch floitsch requested a review from kasperl March 18, 2026 22:58
@floitsch
Copy link
Member Author

@CodeRabbit review

@coderabbitai
Copy link

coderabbitai bot commented Mar 18, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai
Copy link

coderabbitai bot commented Mar 18, 2026

Walkthrough

This pull request introduces comprehensive shell completion infrastructure for bash, zsh, and fish shells. It adds a completion engine, script generators, and extends the CLI framework with completion-related APIs including CompletionCandidate and CompletionContext classes. All Option types now support completion callbacks with automatic value suggestions.

Changes

Cohort / File(s) Summary
Core Completion Infrastructure
src/cli.toit, src/completion_.toit, src/completion-scripts_.toit
Implements shell completion engine with command tree traversal, option/subcommand parsing, and candidate generation. Introduces CompletionCandidate and CompletionContext public classes. Adds multi-shell script generators (bash, zsh, fish) and extends all Option subclasses with optional completion-callback parameters for custom completion logic.
Documentation & Examples
README.md, examples/completion.toit
Documents new shell completion feature with setup instructions and usage examples. Adds a complete fleet-management CLI example demonstrating completion callbacks, enums, file paths, and dynamic device suggestions via known-devices map.
Help & Integration
src/help-generator_.toit
Automatically includes "completion" subcommand in help output when not already defined, mirroring the existing help subcommand behavior.
Test Suite
tests/completion_test.toit, tests/health/readme9.toit, tests/health/readme10.toit, tests/help_test.toit
Adds comprehensive completion test coverage (383 lines) validating subcommand completion, option names, values, enums, flags, context propagation, hidden items, and nested commands. Updates existing help tests to reflect new completion command. Includes example test files demonstrating device completion and structured output.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Add completion' is vague and generic, using a non-descriptive term that doesn't convey the specific details of shell completion support being added. Consider a more specific title like 'Add shell completion support for bash, zsh, and fish' to better communicate the scope and nature of the changes.
Description check ❓ Inconclusive No pull request description was provided by the author, making it impossible to assess relatedness to the changeset. Add a pull request description explaining the feature, its purpose, and key implementation details to help reviewers understand the scope.
✅ Passed checks (1 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch floitsch/completion
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Tip

CodeRabbit can suggest fixes for GitHub Check annotations.

Configure the reviews.tools.github-checks setting to adjust the time to wait for GitHub Checks to complete.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/completion_.toit`:
- Around line 74-126: The replay loop that processes args (before invoking
complete-rest_) never records how many positional/rest arguments have already
been consumed, so complete-rest_ always starts from slot 0 and later rest
completers can shadow earlier slots; fix this by tracking the active positional
index during the loop (e.g., maintain an integer like active-positional or
positional-index that you increment each time you consume a positional for
current-command) and pass that index into complete-rest_ (or populate the
seen/consumed state so complete-rest_ can start from that offset); update
references in the loop where you descend into subcommands (current-command,
add-options-for-command_) and where you handle non-option positionals so they
increment the tracked positional counter before calling complete-rest_.
- Around line 213-218: The completion currently always suggests "--help" and
"-h" in completion_.toit regardless of whether those names are already in use;
change the logic that adds CompletionCandidate_ entries so it first checks the
command's available names (the same availability rules used by
src/help-generator_.toit — i.e., whether "help" and "h" are unused) before
calling candidates.add for "--help" or "-h". In other words, gate the two
if-blocks around a check like "help name available" / "h name available" (using
whatever symbol/lookup the codebase provides for option-name availability) so
you only add CompletionCandidate_ "--help" or "-h" when that short/long name is
actually free.
- Around line 112-116: Arg handling currently skips short options
(arg.starts-with "-"), which prevents setting pending-option and marking
non-multi options as seen; update the argv-replay logic to parse short options:
for packed forms (e.g., "-abc") iterate each short letter, look up the
corresponding option by its short alias, mark its seen flag (unless
option.multi), and if a short option expects a value (non-multi) and is the last
letter in the packed token, set pending-option to that option so the next argv
token becomes its value; for simple "-o value" set pending-option to the
looked-up option and mark seen (unless option.multi); remove the continue.repeat
so short flags are handled instead of skipped.
- Around line 136-140: The current logic sets directive from
completions.is-empty, causing explicit value-completers (e.g., device/host) that
return no matches to fall back to file completion; instead, base the directive
on whether the pending option actually has a value completer (i.e., whether
pending-option.complete is present), not on completions.is-empty. Change the
directive expression in the CompletionResult_ construction that uses
pending-option.complete and completions (the lines setting directive := ... and
returning CompletionResult_ with to-candidate_ mapping) to emit
DIRECTIVE-FILE-COMPLETION_ only when no completer exists
(pending-option.complete is null/absent) and otherwise emit
DIRECTIVE-NO-FILE-COMPLETION_ even if completions.is-empty; apply the same fix
to the other analogous block around the 160-166 region.

In `@src/completion-scripts_.toit`:
- Around line 33-34: The scripts use "head -n -1" which is non-portable on
BSD/macOS; replace those uses with a portable command that removes the last line
(e.g., use sed to delete the final line) wherever you build the directive and
the remaining lines: specifically update the bash block that assigns
directive=$(echo "$completions" | tail -n 1) and completions=$(echo
"$completions" | head -n -1) and the zsh block that assigns directive=$(echo
"$output" | tail -n 1) and lines=("${(`@f`)$(echo "$output" | head -n -1)}") so
they instead extract the last line for directive with tail and obtain the rest
with a portable "remove last line" command (sed '$d' or equivalent) to ensure
compatibility on macOS/BSD.
- Around line 19-21: The generated shell function name currently only replaces
"-" with "_" (func-name) and leaves spaces, dots, quotes and leading digits
which produce invalid bash/zsh/fish identifiers; update func-name generation
(and any helper used by the bash/zsh/fish generators) to: 1) replace any
non-alphanumeric characters with "_" (e.g. [^A-Za-z0-9] -> "_"), 2) ensure the
resulting name starts with a letter or underscore (prefix with "_" if it starts
with a digit), and 3) collapse consecutive "_" if desired; additionally, quote
all variable interpolations of program-path and program-name used in command
substitutions and shell directives (everywhere program-path and program-name are
inserted into generated bash/zsh/fish code) so command substitutions use
"$program-path" and directives use "$program-name" to handle spaces and special
characters across all three generators.
- Around line 115-125: In the __$(func-name)_completions function the parsed
variable directive is never used and the complete invocation always includes -f
(enabling file completion); update the logic to inspect the directive variable
(as extracted into directive) and conditionally add the -f flag only when
directive equals the file-completion value (e.g. "4") and omit -f when directive
equals the no-file-completion value (e.g. "1"), so the call to complete -c
$program-name uses -f only when directive indicates file completion and
otherwise does not include -f.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5dfe1646-3a75-42e0-996a-c2f533a26430

📥 Commits

Reviewing files that changed from the base of the PR and between 5a8211e and 749808c.

📒 Files selected for processing (10)
  • README.md
  • examples/completion.toit
  • src/cli.toit
  • src/completion-scripts_.toit
  • src/completion_.toit
  • src/help-generator_.toit
  • tests/completion_test.toit
  • tests/health/readme10.toit
  • tests/health/readme9.toit
  • tests/help_test.toit

@floitsch
Copy link
Member Author

TBR.

@floitsch floitsch merged commit ee404b2 into main Mar 19, 2026
7 checks passed
@floitsch floitsch deleted the floitsch/completion branch March 19, 2026 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant