-
Notifications
You must be signed in to change notification settings - Fork 3
feat: Experimental command line completion support #65
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughAdds an experimental "completion" command and YAML generator for the Completely tool, a README describing how to generate/install Bash completions, a constructor flag to enable the command, exposes runner global options, and narrows BetterCommand parent/runner type annotations. All output can be written to stdout or a specified file. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant CLI as CLI App
participant BCR as BetterCommandRunner
participant CC as CompletionCommand
participant UR as UsageRepresentation
participant Gen as CompletelyYamlGenerator
participant FS as File System
User->>CLI: start CLI (args)
CLI->>BCR: new BetterCommandRunner(experimentalCompletionCommand: true)
Note right of BCR: CompletionCommand registered at startup
User->>BCR: run "completion [--target --exec-name --file]"
BCR->>CC: dispatch runWithConfig(config)
CC->>UR: compile(runner, execNameOverride?)
UR-->>CC: CommandUsage tree
alt file provided
CC->>FS: open file for writing
CC->>Gen: generate(out, usage)
Gen-->>FS: write YAML spec
else stdout
CC->>Gen: generate(stdout, usage)
Gen-->>User: print YAML spec
end
sequenceDiagram
autonumber
participant BCR as BetterCommandRunner
participant Reg as Command Registry
BCR->>Reg: addCommand(CompletionCommand)
Note over BCR,Reg: Happens only if experimentalCompletionCommand == true
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
There was a problem hiding this 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
🧹 Nitpick comments (6)
packages/cli_tools/README_completion.md (3)
10-11: Fix markdownlint MD034: wrap the bare URL as a proper link.Prevents linter noise and renders consistently.
-https://github.com/bashly-framework/completely +See the Completely project: <https://github.com/bashly-framework/completely>
8-8: Minor copyedits: proper nouns.Capitalize Ruby and Homebrew.
-This requires the tool `completely`, which also requires ruby to be installed. +This requires the tool `completely`, which also requires Ruby to be installed.-or with homebrew: +or with Homebrew:Also applies to: 16-16
35-37: Optional: add macOS/Linux install paths for Bash completions.Paths differ across distros; adding a macOS example reduces friction.
If useful, I can add a short table with common locations (e.g., macOS Homebrew: /opt/homebrew/etc/bash_completion.d, Debian/Ubuntu: /usr/share/bash-completion/completions).
packages/cli_tools/lib/src/better_command_runner/better_command_runner.dart (1)
182-184: ExposeglobalOptionsas read‑only to callers.Returning the backing list allows external mutation after parsing. Wrap with
UnmodifiableListView.- List<O> get globalOptions => _globalOptions; + List<O> get globalOptions => UnmodifiableListView(_globalOptions);Add import (outside this hunk):
import 'dart:collection' show UnmodifiableListView;packages/cli_tools/lib/src/better_command_runner/completion_command.dart (2)
62-69: Non-blocking: add a default case or assertion for future targets.Prepares for adding more targets without silent no-ops.
switch (target) { case CompletionTarget.completely: CompletelyYamlGenerator().generate(out, usage); break; + default: + throw UnsupportedError('Unsupported completion target: $target'); }
190-197: Note: only first positional (argPos == 0) gets value completions.Callout for future enhancement; acceptable for experimental.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/cli_tools/README_completion.md(1 hunks)packages/cli_tools/lib/src/better_command_runner/better_command.dart(1 hunks)packages/cli_tools/lib/src/better_command_runner/better_command_runner.dart(3 hunks)packages/cli_tools/lib/src/better_command_runner/completion_command.dart(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-07T07:55:17.269Z
Learnt from: christerswahn
PR: serverpod/cli_tools#57
File: packages/config/test/better_command_runner/default_flags_test.dart:1-1
Timestamp: 2025-08-07T07:55:17.269Z
Learning: In the `config` package, `better_command_runner.dart` is intentionally kept as a separate import (`package:config/better_command_runner.dart`) rather than being re-exported through the main `packages/config/lib/config.dart` barrel file. This separation is by design according to the package maintainer christerswahn.
Applied to files:
packages/cli_tools/lib/src/better_command_runner/better_command_runner.dart
🪛 markdownlint-cli2 (0.17.2)
packages/cli_tools/README_completion.md
10-10: Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (7)
packages/cli_tools/lib/src/better_command_runner/better_command_runner.dart (3)
8-9: LGTM: local import of the completion command.The separation keeps the feature self-contained.
151-151: Constructor flag looks good and defaults safe.Non-breaking; feature-gated behind
experimentalCompletionCommand.
177-179: Approve: no duplicate 'completion' subcommand found.
CompletionCommand is implemented at packages/cli_tools/lib/src/better_command_runner/completion_command.dart (String get name => 'completion') and the only registration is the conditional addCommand(CompletionCommand()) in packages/cli_tools/lib/src/better_command_runner/better_command_runner.dart; no other command defines the 'completion' name.packages/cli_tools/lib/src/better_command_runner/completion_command.dart (4)
248-263: Enum values: prefer usingnameonly if that’s what the CLI expects.If your CLI accepts enum serialized values different from
.name, adjust accordingly. Otherwise this is fine.Do your
EnumOption-backed flags accept enum.namestrings or something else (e.g., custom toString/labels)? If unsure, I can audit usages and align the generator.
58-61: Minor: allow overriding the executable name viaexecNamesafely.If
execNamecontains spaces, Completely treats it as separate tokens already. No change needed, just confirming intent.If multi-word exec names are not expected in your CLIs, consider validating and warning when
execNamecontains whitespace.
152-154: Edge case: empty root (no options, no subcommands) prints nothing.Reasonable default; just noting behavior.
146-146: Sanity‑check generated completely.yaml matches Completely README formatEnsure CompletelyYamlGenerator emits a mapping key (e.g.,
mygit:ormygit status:) followed by an indented YAML sequence of list items (lines starting with-) as shown in the Completely README.File: packages/cli_tools/lib/src/better_command_runner/completion_command.dart (CompletelyYamlGenerator, ~line 146)
packages/cli_tools/lib/src/better_command_runner/completion_command.dart
Show resolved
Hide resolved
packages/cli_tools/lib/src/better_command_runner/completion_command.dart
Show resolved
Hide resolved
packages/cli_tools/lib/src/better_command_runner/completion_command.dart
Show resolved
Hide resolved
packages/cli_tools/lib/src/better_command_runner/completion_command.dart
Show resolved
Hide resolved
packages/cli_tools/lib/src/better_command_runner/completion_command.dart
Show resolved
Hide resolved
nielsenko
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is super interesting. I do think we should consider using another tool than completely that supports more shells. I had my eyes on complgen previously, but something like carapace is perhaps a more mature eco-system.
However, I'm fine with enabling this experiment as is. Let's not make perfect the enemy of good.. though fish obviously a superior shell 😉
|
Had a look at carapace, it supports YAML input - terrific! It will be quite easy to write another yaml generator for it! 👍 (If you want to have a go at it, by all means! 😁 ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (2)
packages/cli_tools/lib/src/better_command_runner/completion_command.dart (2)
197-202: Fix indentation for positional argument values.Positional argument values at
argPos == 0should also be indented under the command key.Apply this diff to fix the indentation:
if (option.option.argPos == 0) { final values = _getOptionValues(option.option); if (values.isNotEmpty) { - _generateCompletelyForOptionValues(out, values); + for (final value in values) { + out.writeln(' - $value'); + } } }
239-250: Option value mappings are properly structured in YAML.The method correctly generates option value mappings with proper keys for argument completions.
🧹 Nitpick comments (2)
packages/cli_tools/lib/src/better_command_runner/completion_command.dart (2)
75-75: Consider improving the null cast pattern.While functionally correct, casting
null as Tis non-idiomatic. Consider either:
- Constraining the generic type
Tto be nullable- Using a more explicit return pattern
Option 1 (if T is meant to be nullable):
-class CompletionCommand<T> extends BetterCommand<CompletionOption, T> { +class CompletionCommand<T extends Object?> extends BetterCommand<CompletionOption, T> {Then simply:
- return null as T; + return null as T; // Safe when T extends Object?Option 2 (document the constraint):
/// T must be a nullable type for this command class CompletionCommand<T> extends BetterCommand<CompletionOption, T> {
252-270: Consider enhancing placeholder specificity for better UX.The generic
<file>and<directory>placeholders could be more descriptive to guide users better.Consider using more context-aware placeholders:
case FileOption(): - return ['<file>']; + // Consider using the option's helpText or a more specific placeholder + return ['<file>']; // Could be '<path/to/file>' or based on expected file type case DirOption(): - return ['<directory>']; + return ['<directory>']; // Could be '<path/to/directory>'You might also want to leverage file extensions or patterns if the FileOption has constraints:
case FileOption(): // If the option has specific extensions or patterns, use them // e.g., if it expects YAML files: ['*.yaml', '*.yml'] return ['<file>'];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/cli_tools/lib/src/better_command_runner/completion_command.dart(1 hunks)
🔇 Additional comments (4)
packages/cli_tools/lib/src/better_command_runner/completion_command.dart (4)
46-77: Good job on implementing async file handling!The implementation correctly awaits flush and close operations when writing to a file, preventing data loss and resource leaks.
163-167: YAML list indentation is correctly fixed.The subcommand list items are now properly indented under the command mapping key.
183-195: Option entries are correctly indented under the command.The option list items (both long and short forms) are properly indented, maintaining valid YAML structure.
272-279: Value list items are correctly indented.The helper now properly indents all value list items under their parent mapping keys.
This is an experimental feature to generate and install command line
completion in bash for commands using
BetterCommandRunner.Prerequisites
This requires the tool
completely, which also requires ruby to be installed.https://github.com/bashly-framework/completely
or with homebrew:
Activate
Construct
BetterCommandRunnerwith the flagexperimentalCompletionCommandset to
true.Install completion
When the
completelytool is installed, run e.g:This will write the completions script to
~/.local/share/bash-completion/completions/,where bash picks it up automatically on start.
In order to update the completions in the current bash shell, run:
exec bashFor end users, the generated bash script can be distributed as a file for them
to install directly.
Summary by CodeRabbit
New Features
Documentation