Add shell completion command to cli-processor#551
Conversation
This PRD proposes adding a shell-completion command to the cli-processor module that generates Bash and Zsh completion scripts by introspecting registered CLI commands. Key features: - Dynamic command introspection via CLIProcessor API - IArgumentTypeResolver interface for extensible argument completion - Built-in resolvers for FILE, FORMAT, URL, EXPRESSION types - CompletionScriptGenerator for Bash and Zsh output Addresses metaschema-framework/oscal-cli#85
📝 WalkthroughWalkthroughAdds a type-aware shell-completion subsystem (Bash/Zsh): new completion type interface and registry, a script generator, a shell-completion command, tests and docs; extends ExtraArgument with optional type, annotates many options/arguments with types, exposes CLIProcessor.getTopLevelCommands() publicly, and updates .gitignore for Git worktrees. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant CLI as CLI/CLIProcessor
participant Cmd as ShellCompletionCommand
participant Gen as CompletionScriptGenerator
participant Reg as CompletionTypeRegistry
participant FS as FileSystem/Stdout
User->>CLI: run "shell-completion <bash|zsh> [--to FILE]"
CLI->>Cmd: dispatch command
Cmd->>CLI: getTopLevelCommands()
Cmd->>Gen: new(programName, commands)
Gen->>Reg: lookup types for options/extra-args
Reg-->>Gen: ICompletionType snippets (file, uri, enum, etc.)
Gen->>Gen: compose Bash/Zsh completion script
Gen-->>Cmd: return generated script
Cmd->>FS: write (--to FILE) or stdout
FS-->>User: script content / confirmation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2025-12-13T21:16:12.281ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
PRDs/20251215-shell-completion/implementation-plan.md (1)
205-210: Consider test fixtures for CompletionScriptGenerator tests.The test cases describe verification points (syntax validation, command inclusion) but don't mention how mock commands and options will be created. Plan to create test fixtures with realistic command hierarchies to validate complex scenarios (nested subcommands, options with format arguments, etc.).
Example: Consider creating a test builder to construct realistic ICommand hierarchies for testing.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.gitignore(1 hunks)PRDs/20251215-shell-completion/PRD.md(1 hunks)PRDs/20251215-shell-completion/implementation-plan.md(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Website
- GitHub Check: Code
🔇 Additional comments (4)
.gitignore (1)
37-39: Good addition for local development hygiene.Adding
.worktrees/to ignore local Git worktrees is a sensible practice to prevent committing development artifacts.PRDs/20251215-shell-completion/PRD.md (1)
189-227: Generated examples are appropriately simplified for documentation.The Bash and Zsh examples effectively illustrate the core completion generation approach. Note that actual generated scripts will be more complex with nested subcommands and deeper hierarchies, but the examples capture the essential structure and pattern.
Also applies to: 231-279
PRDs/20251215-shell-completion/implementation-plan.md (2)
83-92: Explicitly document resolver registry initialization approach.The implementation approach states "initialize resolver registry" but doesn't clarify whether
ArgumentTypeResolversshould be injected via constructor parameter or created internally. Since this differs from the PRD's three-parameter signature, make this design choice explicit in the plan.Recommended: Add a note clarifying:
- Will
ArgumentTypeResolversstatic methods be used internally?- Or should the resolver map be a constructor parameter?
- How does this interact with future custom resolver plugins?
172-184: Escalate Task 2.2 investigation—resolve CLIProcessor access before phase 2 implementation.Task 2.2 defers the critical decision on how
ShellCompletionCommandaccesses theCLIProcessor, yet the execution flow in Task 2.1 (line 159) assumesCallingContextprovides this access. Investigate Option A/B/C early (before Task 2.1) to unblock command implementation.Recommend: Move Task 2.2 earlier in the implementation order (before Task 2.1) to validate the access pattern.
Replace argName-based argument type resolution with a type-based approach using Apache Commons CLI's Option.type() method: - Use File.class for file path completion - Use URI.class for URL completion - Introduce ICompletionType interface for custom completion types - Create Format enum implementing ICompletionType for format options This provides type safety, explicit declaration of completion behavior, and extensibility for custom completion types. Also adds Phase 5 task for migrating existing options to use .type().
- Add CompletionTypeRegistry for decoupled type-to-completion mapping - Pre-register built-in types (File, URI, URL) - Add registerEnum() helper for enum types - Add forEnum() factory method for creating enum completion - Update CompletionScriptGenerator to use registry lookup - Update ExtraArgument extension for completion type support - Expand end-user documentation with detailed installation steps for both Bash and Zsh shell completion setup - Fix task numbering in implementation plan
- Add ShellCompletionCommand with bash and zsh script generation - Add CompletionTypeRegistry for type-to-completion mapping - Add ICompletionType interface for completion behavior - Add CompletionScriptGenerator for script output - Extend ExtraArgument with type hints for positional args - Register command via SPI for automatic CLI discovery - Add fallback functions for systems without bash-completion - Add type hints to metaschema-cli options for completion - Register Format and SchemaFormat enums for completion - Add comprehensive unit tests (40 tests passing)
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/completion/CompletionTypeRegistryTest.java (1)
62-83: Test modifies shared static registry state.This test registers
StringBuilder.classin the globalCompletionTypeRegistry. While this won't cause issues with the current test suite, it could affect other tests if run in parallel or if test ordering changes. Consider documenting this side effect or using@AfterEachcleanup if the registry gains anunregistermethod in the future.cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/completion/CompletionTypeRegistry.java (1)
52-54: Consider documenting or logging when existing registrations are overwritten.The
register()method silently replaces any existing registration for a type. While this is flexible, it could lead to unexpected behavior if a type is accidentally registered twice with different completions. Consider adding a debug-level log or documenting this behavior.cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/command/ShellCompletionCommand.java (1)
60-103: Consider reusing theShellenum fromCompletionScriptGenerator.There's a duplicate
Shellenum defined inCompletionScriptGenerator(seeCompletionScriptGenerator.javalines 31-36). Having two identical enums violates DRY and could lead to maintenance issues if they diverge.Consider either:
- Reusing
CompletionScriptGenerator.Shellhere- Moving the enum to a shared location
- /** - * Supported shell types. - */ - public enum Shell { - /** Bash shell. */ - BASH("bash"), - /** Zsh shell. */ - ZSH("zsh"); - - @NonNull - private final String name; - - Shell(@NonNull String name) { - this.name = name; - } - - /** - * Get the shell name. - * - * @return the name - */ - @NonNull - public String getName() { - return name; - } - - /** - * Parse a shell type from a string. - * - * @param value - * the string value to parse - * @return the shell type - * @throws IllegalArgumentException - * if the value is not a recognized shell type - */ - @NonNull - public static Shell fromString(@NonNull String value) { - String normalized = value.toLowerCase(Locale.ROOT); - for (Shell shell : values()) { - if (shell.name.equals(normalized)) { - return shell; - } - } - throw new IllegalArgumentException( - "Unknown shell: " + value + ". Supported shells: bash, zsh"); - } - }Then use
CompletionScriptGenerator.Shellthroughout this class. If a shared location is preferred, consider creating acompletionpackage-levelShelltype.cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/completion/CompletionScriptGenerator.java (1)
284-288: The trailing backslash removal logic works but is fragile.The logic correctly removes the trailing
" \\"from the last_argumentsline, but it relies on the specific structure of the preceding code. If future modifications change how lines are appended, this could break silently.Consider a more explicit approach that tracks whether content was added, though the current implementation is functional.
PRDs/20251215-shell-completion/implementation-plan.md (2)
465-482: Add blank line before closing code fence.The static analysis flags line 481 for a fenced code block language issue. This appears to be due to the nested code blocks in the markdown example. Adding a blank line before the closing fence may resolve the linting warning.
your-cli shell-completion zsh > ~/.zfunc/_your-cli # Add to .zshrc: fpath=(~/.zfunc $fpath); autoload -Uz compinit; compinit
522-532: Add blank lines around the table for proper markdown formatting.Per static analysis, tables should be surrounded by blank lines for consistent rendering across markdown parsers.
**Type assignments:** + | Option | Type | |--------|------| | METASCHEMA_REQUIRED_OPTION | `File.class` | | METASCHEMA_OPTIONAL_OPTION | `File.class` | | TO_OPTION | `Format.class` | | AS_FORMAT_OPTION | `Format.class` | | AS_SCHEMA_FORMAT_OPTION | Custom schema format enum | | CONSTRAINTS_OPTION | `URI.class` | | SARIF_OUTPUT_FILE_OPTION | `File.class` | +
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
PRDs/20251215-shell-completion/PRD.md(1 hunks)PRDs/20251215-shell-completion/implementation-plan.md(1 hunks)cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessor.java(1 hunks)cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/command/ExtraArgument.java(3 hunks)cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/command/ShellCompletionCommand.java(1 hunks)cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/command/impl/DefaultExtraArgument.java(4 hunks)cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/completion/CompletionScriptGenerator.java(1 hunks)cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/completion/CompletionTypeRegistry.java(1 hunks)cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/completion/ICompletionType.java(1 hunks)cli-processor/src/main/resources/META-INF/services/gov.nist.secauto.metaschema.cli.processor.command.ICommand(1 hunks)cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/command/ShellCompletionCommandTest.java(1 hunks)cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/completion/CompletionScriptGeneratorTest.java(1 hunks)cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/completion/CompletionTypeRegistryTest.java(1 hunks)metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/CLI.java(2 hunks)metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/AbstractConvertSubcommand.java(2 hunks)metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/AbstractValidateContentCommand.java(2 hunks)metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/GenerateSchemaCommand.java(2 hunks)metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/MetaschemaCommands.java(5 hunks)metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/metapath/EvaluateMetapathCommand.java(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-11-15T20:00:59.203Z
Learnt from: david-waltermire
Repo: metaschema-framework/metaschema-java PR: 245
File: cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/command/impl/DefaultExtraArgument.java:15-15
Timestamp: 2024-11-15T20:00:59.203Z
Learning: The `DefaultExtraArgument` class in `cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/command/impl/DefaultExtraArgument.java` is currently a stub for future exploration.
Applied to files:
metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/AbstractValidateContentCommand.javametaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/GenerateSchemaCommand.javacli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/command/ExtraArgument.javametaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/AbstractConvertSubcommand.javacli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/command/impl/DefaultExtraArgument.java
🧬 Code graph analysis (5)
metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/AbstractValidateContentCommand.java (1)
core/src/main/java/gov/nist/secauto/metaschema/core/util/ObjectUtils.java (1)
ObjectUtils(18-135)
cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/command/ShellCompletionCommandTest.java (1)
cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/command/ShellCompletionCommand.java (1)
ShellCompletionCommand(37-196)
cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/completion/CompletionTypeRegistryTest.java (1)
cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/completion/CompletionTypeRegistry.java (1)
CompletionTypeRegistry(30-154)
cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/command/ShellCompletionCommand.java (3)
core/src/main/java/gov/nist/secauto/metaschema/core/util/ObjectUtils.java (1)
ObjectUtils(18-135)cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessor.java (2)
SuppressWarnings(58-803)SuppressWarnings(332-802)metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/MetaschemaCommands.java (1)
SuppressWarnings(56-626)
metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/CLI.java (1)
cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/completion/CompletionTypeRegistry.java (1)
CompletionTypeRegistry(30-154)
🪛 markdownlint-cli2 (0.18.1)
PRDs/20251215-shell-completion/implementation-plan.md
481-481: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
523-523: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
PRDs/20251215-shell-completion/PRD.md
481-481: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
523-523: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
🔇 Additional comments (41)
metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/AbstractValidateContentCommand.java (4)
39-39: LGTM!The
Fileimport is correctly added to support the type annotation at line 78.
62-63: LGTM!The
URI.classtype hint correctly identifies the expected argument type for shell completion support, aligning with the "file-or-URI-to-validate" semantic.
66-72: LGTM!The
URI.classtype annotation is semantically consistent with theargName("URL")and correctly enables completion for constraint definition URIs.
74-81: LGTM!The
File.classtype annotation is semantically consistent with theargName("FILE")and correctly enables file-path completion for the SARIF output destination.cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/completion/ICompletionType.java (1)
1-34: LGTM! Clean interface design.The interface is well-structured with clear contracts for shell completion. The non-null return types and comprehensive Javadoc make the API easy to understand and use.
metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/metapath/EvaluateMetapathCommand.java (1)
77-83: LGTM! Appropriate type constraint for URI-based argument.The addition of
.type(URI.class)aligns with the type-aware completion design and is appropriate for the FILE_OR_URL argument.metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/CLI.java (1)
71-73: LGTM! Proper initialization of shell completion types.The enum registrations are correctly placed at the start of
runClibefore processor construction, ensuring completion types are available when the shell-completion command is invoked.cli-processor/src/main/resources/META-INF/services/gov.nist.secauto.metaschema.cli.processor.command.ICommand (1)
1-1: LGTM! Proper SPI registration for command discovery.The SPI registration correctly enables automatic discovery of the ShellCompletionCommand.
cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/CLIProcessor.java (1)
271-273: LGTM! Appropriate visibility change for command introspection.Changing
getTopLevelCommands()fromprotectedtopublicenables the shell-completion subsystem to introspect registered commands. The method remainsfinal, preserving its contract.cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/command/ExtraArgument.java (2)
34-53: LGTM! Well-designed type-aware factory extension.The new
newInstanceoverload with thetypeparameter extends the API cleanly. The@Nullabletype parameter appropriately allows for freeform arguments when no completion type is needed.
79-92: LGTM! Backward-compatible type accessor.The
getType()default method provides a clean extension point while maintaining backward compatibility by returningnullfor implementations that don't support type-aware completion.metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/AbstractConvertSubcommand.java (1)
51-53: LGTM! Appropriate type hints for source and destination arguments.The type hints correctly distinguish between URI (source, which can be URL or file) and File (destination, which is always a file path). This enables proper shell completion behavior for each argument.
metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/GenerateSchemaCommand.java (1)
54-56: Type hints correctly applied for shell completion support.The type hints (
URI.classfor the module file/URL argument andFile.classfor the destination) are semantically appropriate and will enable meaningful shell completions.cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/command/impl/DefaultExtraArgument.java (2)
35-54: Clean constructor chaining pattern with proper documentation.The implementation correctly uses constructor delegation, maintains immutability with
finalfields, and properly documents the optionaltypeparameter for shell completion lookups.
65-70: LGTM - getter correctly implements the interface contract.The
@Overrideannotation indicates this implements the interface method, and@Nullablecorrectly reflects that the type may not be specified.metaschema-cli/src/main/java/gov/nist/secauto/metaschema/cli/commands/MetaschemaCommands.java (3)
82-82: Type hint correctly applied for URI-based option.The
.type(URI.class)metadata will enable the completion system to provide appropriate completions for this option.
122-122: Verify that Format enum is registered for completion.The
.type(Format.class)is correctly added. Ensure thatCompletionTypeRegistry.registerEnum(Format.class)is called during initialization so that shell completions include the enum values.#!/bin/bash # Search for Format.class registration in the codebase rg -n "registerEnum.*Format\.class" --type=java
159-159: No action required. SchemaFormat enum is correctly registered for completion viaCompletionTypeRegistry.registerEnum(SchemaFormat.class)during CLI initialization.cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/completion/CompletionTypeRegistryTest.java (2)
85-91: Same concern: enum registration persists across tests.Similar to the custom type test,
registerEnum(TestFormat.class)modifies global state. This is acceptable for now since the tests are self-contained.
32-60: Good coverage of edge cases and pre-registered types.The tests properly verify null handling, unregistered type behavior, and all pre-registered built-in types.
cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/command/ShellCompletionCommandTest.java (1)
21-77: Comprehensive test coverage for ShellCompletionCommand.The tests thoroughly verify command metadata, shell type parsing with case insensitivity, and error handling for invalid inputs. Good use of
assertThrowsfor exception validation.cli-processor/src/test/java/gov/nist/secauto/metaschema/cli/processor/completion/CompletionScriptGeneratorTest.java (3)
40-44: @BeforeAll correctly used for one-time enum registration.This ensures the
TestFormatenum is registered before any tests run, which is appropriate since the registry is a global singleton.
232-275: Well-designed mock commands for testing.The mock implementations provide appropriate test data with typed options and extra arguments. Returning
nullfromnewExecutoris acceptable since these mocks are only used for script generation, not command execution.
140-168: Good validation of Bash fallback mechanisms.Testing for
_init_completionand_filedirfallbacks ensures the generated scripts work on systems without bash-completion installed.cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/completion/CompletionTypeRegistry.java (3)
30-42: Well-designed static utility class with thread-safe registry.The use of
ConcurrentHashMapensures thread safety, and the private constructor correctly enforces the utility class pattern. Pre-registering common types (File, URI, URL) provides sensible defaults.
82-89: Enum value transformation to lowercase aligns with CLI conventions.Converting enum names to lowercase matches typical shell command conventions where options are case-insensitive.
136-152: EnumCompletionType generates correct shell-specific syntax.The Bash syntax (
compgen -W "...") and Zsh syntax ((...)) are both correct for their respective completion systems.cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/command/ShellCompletionCommand.java (5)
1-30: LGTM!License, package declaration, and imports are appropriate for the command's functionality.
105-129: LGTM!Command interface methods follow the established patterns correctly.
141-174: LGTM!The execution flow is clean and handles the expected scenarios correctly: argument validation, shell type parsing, script generation, and output handling.
189-195: LGTM!The stdout handling is correct - using explicit UTF-8 encoding and flushing without closing the stream.
176-187: No action required. The methodresolveAgainstCWDis properly defined in the parent classAbstractTerminalCommandwith an overloaded version accepting aPathparameter, which matches the call at line 178.Likely an incorrect or invalid review comment.
cli-processor/src/main/java/gov/nist/secauto/metaschema/cli/processor/completion/CompletionScriptGenerator.java (6)
1-56: LGTM!The class setup, constructor, and accessors are well-structured.
78-138: LGTM!The Bash completion script generation is well-structured with appropriate fallbacks for systems without bash-completion installed.
140-186: LGTM!The Zsh completion script generation follows proper Zsh conventions with correct single-quote escaping for descriptions.
188-255: LGTM!The recursive subcommand handling and option completion logic are well-implemented.
337-346: LGTM!The type handling correctly accounts for
Option.getType()returning anObjectthat may not always be aClass<?>.
358-372: LGTM!The fallback to file completion for extra arguments without a registered type is a sensible default.
PRDs/20251215-shell-completion/implementation-plan.md (3)
1-47: LGTM!The implementation plan is well-structured with clear task breakdowns and accurate code examples.
332-401: LGTM!The command implementation plan and testing strategy are comprehensive.
553-577: LGTM!The acceptance criteria are comprehensive and well-aligned with the implementation.
- Add shell completion section to README.txt with bash/zsh setup instructions - Document --to option file handling behavior in PRD (paths, overwrite, errors)
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
metaschema-cli/src/main/distro/README.txt (1)
28-52: Add version and prerequisite notes to shell completion instructions.The documentation is clear and covers both Bash and Zsh effectively, but is missing a few important details:
- Bash process substitution (line 36) requires Bash 4.0+. Consider adding a note for users on older systems to use the
--toapproach instead.- Directory creation (line 45): The Zsh example assumes
~/.zsh/completions/exists. Consider addingmkdir -p ~/.zsh/completionsbefore the command for clarity.- Bash-completion package: Per the PR description, the tool supports fallback when the bash-completion package is unavailable. Consider documenting that some distributions require
sudo apt-get install bash-completion(or equivalent) for enhanced completion, but basic completion will still work.- Minor inconsistency: Bash uses
--to FILEwhile Zsh uses> FILE. While both are valid, it might help to clarify that both approaches are available for each shell (e.g., Zsh also supports--to).Suggested revision (for lines 34–49):
For Bash: # Generate and source completion script source <(metaschema-cli shell-completion bash) + # Note: Requires Bash 4.0+. For older versions, use --to approach: + # metaschema-cli shell-completion bash --to ~/.metaschema-cli-completion.bash # Or save to a file for persistent use metaschema-cli shell-completion bash --to ~/.metaschema-cli-completion.bash echo 'source ~/.metaschema-cli-completion.bash' >> ~/.bashrc For Zsh: + # Ensure completions directory exists + mkdir -p ~/.zsh/completions # Generate and save to completions directory metaschema-cli shell-completion zsh > ~/.zsh/completions/_metaschema-cli + # Or use --to flag + # metaschema-cli shell-completion zsh --to ~/.zsh/completions/_metaschema-cli
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
PRDs/20251215-shell-completion/PRD.md(1 hunks)metaschema-cli/src/main/distro/README.txt(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
PRDs/20251215-shell-completion/PRD.md
44-44: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
506-506: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
521-521: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
533-533: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
539-539: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
545-545: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
557-557: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
611-611: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
630-630: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
641-641: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Code
- GitHub Check: Website
🔇 Additional comments (1)
PRDs/20251215-shell-completion/PRD.md (1)
1-50: Good comprehensive PRD addressing prior feedback.The document successfully addresses the prior review comment on --to option file handling (lines 497-502) with explicit details on path resolution, directory creation behavior, overwrite semantics, and error codes. The constructor signature alignment has also been resolved. Remaining issues are markdown formatting violations, not content concerns.
aj-stein
left a comment
There was a problem hiding this comment.
Looks good so far, I will need to install locally and evaluate the shell completions on my computer before giving a more thorough review though. ⌛
aj-stein
left a comment
There was a problem hiding this comment.
Definitely look forward to using this one since I am lazy tab-completer type on Linux. Thanks for implementing this so quickly.
66640f8
into
metaschema-framework:develop
Summary
Adds a
shell-completioncommand to the CLI that generates tab-completion scripts for Bash and Zsh shells. This improves the user experience by providing intelligent command and option completion.Key Features
metaschema-cli shell-completion bash|zsh [--to FILE]CompletionTypeRegistryOption.type()and extendedExtraArgumentfor completion hintsUsage
Completion Types Supported
File.class_filedir_filesURI.class/URL.class_filedir_urlscompgen -W "values..."(value1 value2 ...)Files Changed
cli-processor module:
ShellCompletionCommand.java- Main command implementationCompletionScriptGenerator.java- Bash/Zsh script generationCompletionTypeRegistry.java- Type-to-completion mappingICompletionType.java- Completion behavior interfaceExtraArgument.java- Extended with type hintsmetaschema-cli module:
.type()to options for completion supportFormatandSchemaFormatenumsTest Plan
--tofile output optionRelated Issues
Related
PRD:
PRDs/20251215-shell-completion/Summary by CodeRabbit
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.