feat(cmd): add Example fields with validation utilities#61
Merged
feloy merged 7 commits intokortex-hub:mainfrom Mar 12, 2026
Merged
feat(cmd): add Example fields with validation utilities#61feloy merged 7 commits intokortex-hub:mainfrom
feloy merged 7 commits intokortex-hub:mainfrom
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Example help text to several Cobra commands, implements parsing and validation utilities for example blocks, adds an alias-adaptation helper, and introduces unit tests to validate examples and alias transformations against the CLI command tree. Changes
Sequence Diagram(s)sequenceDiagram
participant Examples as Example Block
participant Parser as Example Parser
participant Validator as Example Validator
participant Cobra as Cobra Command Tree
Examples->>Parser: provide multi-line example text
Parser->>Parser: parse lines into ExampleCommand structs
Parser->>Validator: deliver parsed ExampleCommand(s)
Validator->>Cobra: lookup commands/subcommands and flags
Cobra-->>Validator: return command/flag metadata
Validator-->>Parser: return validation results (ok / errors)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📝 Coding Plan for PR 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
pkg/cmd/list.go (1)
21-35: Avoid importingpkg/cmd/testutilinto runtime command construction.
NewListCmd()now depends on a package namedtestutiljust to rewrite help text. I'd moveAdaptExampleForAliasintopkg/cmdor a neutral helper package and keep the parser/validator helpers intestutil.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/cmd/list.go` around lines 21 - 35, NewListCmd imports testutil solely for AdaptExampleForAlias which couples runtime command construction to test helpers; move AdaptExampleForAlias into a neutral runtime helper in pkg/cmd (or a new pkg/cmd/internal/helpers) and update NewListCmd to call that helper instead of pkg/cmd/testutil. Modify the code that defines AdaptExampleForAlias (currently in pkg/cmd/testutil) so it is exported from the new location, update references in NewListCmd to use the new package/function name, and remove the testutil import from pkg/cmd/list.go so runtime code no longer depends on test-only utilities.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/cmd/testutil/example_validator_test.go`:
- Around line 23-190: Add regression cases to TestParseExampleCommands to assert
ParseExampleCommands returns an error for malformed examples: include tests
named like "extra positional to leaf command" with example `kortex-cli workspace
list extra`, "string flag missing value" with example `kortex-cli init --name`,
and "unmatched quotes" with an example containing a missing closing quote (e.g.,
`kortex-cli init --name "proj`). Place these new entries in the tests slice
inside TestParseExampleCommands and set wantErr: true so the parser
(ParseExampleCommands) is validated to reject extra positional args for leaf
commands, flags that require values but have none, and unbalanced quotation
marks.
In `@pkg/cmd/testutil/example_validator.go`:
- Around line 66-68: splitCommandLine currently accepts lines with unterminated
quotes and must be changed to fail instead; update splitCommandLine to return an
error when it reaches EOF while inQuote (e.g., change signature to return
([]string, error) or otherwise propagate an error) and propagate that error up
in parseCommandLine so parseCommandLine returns an error for inputs like
`kortex-cli init "/tmp/project`. Adjust callers (parseCommandLine and any other
uses) to handle the new error return and ensure ExampleCommand parsing fails on
unterminated quoted arguments.
- Around line 26-31: The ExampleCommand flag handling currently stores every
flag value as a string, losing whether a value was actually provided; update
ExampleCommand to store per-flag metadata (e.g., change Flags from
map[string]string to a map[string]FlagMeta or add a separate map like
FlagPresent map[string]bool and FlagValues map[string]string) so you can
distinguish "no value supplied" vs "empty value supplied"; then update the
validator code that inspects flags (references: ExampleCommand, Flags, the
validation function that checks flag values, and use pflag.Flag.NoOptDefVal) to
treat non-boolean StringVar/StringVarP flags as invalid when the flag was
present but no value was supplied (FlagPresent true && FlagValues[key] == "" &&
pflag.Flag.NoOptDefVal == "") and allow empty values only when
pflag.Flag.NoOptDefVal indicates the flag accepts an explicit empty value;
ensure boolean flags continue to be valid when no value is provided.
- Around line 176-190: In ValidateExampleCommand, after finding the resolved
command (cmd) and remainingArgs, invoke the command's Args validator by calling
cmd.Args(cmd, remainingArgs) and return any error it produces (e.g., handle
cobra.NoArgs, ExactArgs, etc.); replace the current heuristic that only checks
cmd.HasSubCommands() for rejecting unknown args with this Args-based validation
(you may still keep the existing unknown-subcommand error for clearer messages,
but do not accept examples simply because HasSubCommands() is false—use cmd.Args
to decide validity).
---
Nitpick comments:
In `@pkg/cmd/list.go`:
- Around line 21-35: NewListCmd imports testutil solely for AdaptExampleForAlias
which couples runtime command construction to test helpers; move
AdaptExampleForAlias into a neutral runtime helper in pkg/cmd (or a new
pkg/cmd/internal/helpers) and update NewListCmd to call that helper instead of
pkg/cmd/testutil. Modify the code that defines AdaptExampleForAlias (currently
in pkg/cmd/testutil) so it is exported from the new location, update references
in NewListCmd to use the new package/function name, and remove the testutil
import from pkg/cmd/list.go so runtime code no longer depends on test-only
utilities.
🪄 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: d20da726-7278-4e6e-96a9-8c0f6d553b07
📒 Files selected for processing (11)
AGENTS.mdpkg/cmd/init.gopkg/cmd/init_test.gopkg/cmd/list.gopkg/cmd/remove.gopkg/cmd/testutil/example_validator.gopkg/cmd/testutil/example_validator_test.gopkg/cmd/workspace_list.gopkg/cmd/workspace_list_test.gopkg/cmd/workspace_remove.gopkg/cmd/workspace_remove_test.go
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
pkg/cmd/testutil/example_validator_test.go (1)
26-160:⚠️ Potential issue | 🟡 MinorCover malformed parse inputs, not just the wrong-binary path.
TestParseExampleCommandsstill only asserts one failing case. It should also lock down tokenizer failures like a missing flag value and unbalanced quotes; otherwise those branches inParseExampleCommandscan regress unnoticed.Suggested table entries
{ name: "invalid command - wrong binary", example: `other-cli init`, wantErr: true, }, + { + name: "string flag missing value", + example: `kortex-cli init --name`, + wantErr: true, + }, + { + name: "unmatched quotes", + example: `kortex-cli init --name "my-project`, + wantErr: true, + }, { name: "command with multiple flags", example: `kortex-cli init --name my-project --verbose`, wantCount: 1,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/cmd/testutil/example_validator_test.go` around lines 26 - 160, The test suite TestParseExampleCommands only asserts the wrong-binary error path; add additional negative test cases to exercise ParseExampleCommands tokenizer failures (e.g., a command with a long flag missing its value like "kortex-cli cmd --output", an unbalanced quoted argument like `kortex-cli cmd "unterminated`, and a short-flag missing value like `kortex-cli cmd -o`) so those branches return wantErr=true; locate the tests slice in TestParseExampleCommands and add entries that set example to the malformed strings and wantErr: true (using the existing ExampleCommand/ParseExampleCommands flow) to ensure tokenizer and parse errors are covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/cmd/testutil/example_validator_test.go`:
- Around line 432-518: Add a table test case in the tests slice that verifies
only the command prefix is rewritten and not occurrences inside argument values:
create a test entry (use the same fields name, example, originalCmd="workspace
list", aliasCmd="list", want) where example contains a line with the command
prefix "kortex-cli workspace list" and another line with a flag whose value
contains the literal "workspace list" (e.g. kortex-cli workspace list --label
"workspace list"); set want so the command prefix is replaced to "kortex-cli
list" but the quoted flag value remains "workspace list". Ensure the new case is
included alongside the other tests in the tests slice.
---
Duplicate comments:
In `@pkg/cmd/testutil/example_validator_test.go`:
- Around line 26-160: The test suite TestParseExampleCommands only asserts the
wrong-binary error path; add additional negative test cases to exercise
ParseExampleCommands tokenizer failures (e.g., a command with a long flag
missing its value like "kortex-cli cmd --output", an unbalanced quoted argument
like `kortex-cli cmd "unterminated`, and a short-flag missing value like
`kortex-cli cmd -o`) so those branches return wantErr=true; locate the tests
slice in TestParseExampleCommands and add entries that set example to the
malformed strings and wantErr: true (using the existing
ExampleCommand/ParseExampleCommands flow) to ensure tokenizer and parse errors
are covered.
🪄 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: a472f180-846c-47fb-a5cf-f656d4f500e0
📒 Files selected for processing (1)
pkg/cmd/testutil/example_validator_test.go
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
pkg/cmd/testutil/example_validator.go (2)
95-108:⚠️ Potential issue | 🟠 MajorKeep “value supplied” separate from the parsed value.
hasValueis computed and discarded, so--name,--name=, and--name ""all collapse to"". The check on Line 258 then rejects explicitly-empty values as if the flag were missing a value. Please store that metadata per flag and validate against it instead offlagValue == "".Suggested direction
type ExampleCommand struct { Raw string Binary string Args []string FlagPresent map[string]bool + FlagHasValue map[string]bool FlagValues map[string]string Flags map[string]string } @@ cmd := ExampleCommand{ Raw: line, Binary: parts[0], Args: []string{}, FlagPresent: make(map[string]bool), + FlagHasValue: make(map[string]bool), FlagValues: make(map[string]string), Flags: make(map[string]string), } @@ flagName, flagValue, hasValue := parseLongFlag(part, parts, &i) cmd.FlagPresent[flagName] = true + cmd.FlagHasValue[flagName] = hasValue cmd.FlagValues[flagName] = flagValue @@ flagName, flagValue, hasValue := parseShortFlag(part, parts, &i) cmd.FlagPresent[flagName] = true + cmd.FlagHasValue[flagName] = hasValue cmd.FlagValues[flagName] = flagValue @@ - flagValue := exampleCmd.FlagValues[flagName] - if flagValue == "" && flag.NoOptDefVal == "" && flag.Value.Type() != "bool" { + if !exampleCmd.FlagHasValue[flagName] && flag.NoOptDefVal == "" && flag.Value.Type() != "bool" { cmdPath := cmd.CommandPath()For spf13/pflag / Cobra, how are `--flag`, `--flag=`, and `--flag ""` distinguished for string flags, and what does `pflag.Flag.NoOptDefVal` mean for flags that may omit a value?Also applies to: 251-265
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/cmd/testutil/example_validator.go` around lines 95 - 108, The parsing currently discards the hasValue boolean from parseLongFlag/parseShortFlag causing `--name`, `--name=`, and `--name ""` to be indistinguishable; add per-flag metadata (e.g., a new map on the cmd struct like FlagHasValue map[string]bool) and set FlagHasValue[flagName] = hasValue inside both parseLongFlag and parseShortFlag handling (alongside existing FlagPresent/FlagValues/Flags assignments), then update the validation logic that currently checks flagValue == "" to instead consult FlagHasValue (and FlagPresent) so you treat explicitly-supplied empty values differently from omitted values.
68-70:⚠️ Potential issue | 🟡 MinorReject unterminated quoted arguments.
splitCommandLine()still succeeds when EOF is reached withinQuote == true, so malformed examples likekortex-cli init --name "projare accepted and then validated as if they were real commands.Suggested fix
-func parseCommandLine(line string) (ExampleCommand, error) { - // Split by whitespace, respecting quotes (simple parsing) - parts := splitCommandLine(line) +func parseCommandLine(line string) (ExampleCommand, error) { + parts, err := splitCommandLine(line) + if err != nil { + return ExampleCommand{}, err + } if len(parts) == 0 { return ExampleCommand{}, fmt.Errorf("empty command line") } @@ -func splitCommandLine(line string) []string { +func splitCommandLine(line string) ([]string, error) { var parts []string var current strings.Builder inQuote := false quoteChar := rune(0) @@ if current.Len() > 0 { parts = append(parts, current.String()) } - return parts + if inQuote { + return nil, fmt.Errorf("unterminated quoted string") + } + + return parts, nil }Also applies to: 155-185
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/cmd/testutil/example_validator.go` around lines 68 - 70, splitCommandLine currently accepts input that ends with an open quote (inQuote==true) and returns a successful parts slice; change splitCommandLine so it detects unterminated quotes and returns an error (e.g., "unterminated quoted argument") instead of silently succeeding, and update parseCommandLine to propagate and return that error from splitCommandLine (so parseCommandLine returns the error for inputs like `kortex-cli init --name "proj`). Also audit the other parsing usage block around the second parser logic (the code referenced at the alternative location) and make the same change there so both call sites validate and reject unterminated quoted arguments.pkg/cmd/helpers.go (1)
45-47:⚠️ Potential issue | 🟡 MinorRewrite only the command path.
strings.Replace(..., 1)rewrites the first matching substring anywhere on the line. IforiginalCmdappears in a flag value before the real command tokens, this mutates the argument instead of the command and produces a broken alias example. Please match and replace thekortex-cli <originalCmd>segment explicitly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/cmd/helpers.go` around lines 45 - 47, The current replacement uses strings.Replace(line, originalCmd, aliasCmd, 1) which can modify occurrences of originalCmd in flag values; instead locate the exact "kortex-cli "+originalCmd segment and replace only that token; e.g., after confirming strings.HasPrefix(trimmed, "kortex-cli "), compute the prefix segment ("kortex-cli "+originalCmd) and replace that exact substring with "kortex-cli "+aliasCmd (or use TrimPrefix/strings.HasPrefix on the rest) so only the command path (variables line, trimmed, originalCmd, aliasCmd) is mutated, leaving flag values intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/cmd/testutil/example_validator.go`:
- Around line 25-33: Replace the exported concrete struct ExampleCommand with an
unexported implementation type (e.g., exampleCommand) and expose a small
exported interface (e.g., ExampleCommandResult) that provides accessors Raw(),
Binary(), Args(), FlagPresent(), FlagValues() and any needed behavior; add a
factory function NewExampleCommand(...) that returns the interface, keep Flags
as internal/deprecated on the unexported struct, and add a compile-time
assurance var _ ExampleCommandResult = (*exampleCommand)(nil) to satisfy the
repo's interface-based design requirements.
---
Duplicate comments:
In `@pkg/cmd/helpers.go`:
- Around line 45-47: The current replacement uses strings.Replace(line,
originalCmd, aliasCmd, 1) which can modify occurrences of originalCmd in flag
values; instead locate the exact "kortex-cli "+originalCmd segment and replace
only that token; e.g., after confirming strings.HasPrefix(trimmed, "kortex-cli
"), compute the prefix segment ("kortex-cli "+originalCmd) and replace that
exact substring with "kortex-cli "+aliasCmd (or use TrimPrefix/strings.HasPrefix
on the rest) so only the command path (variables line, trimmed, originalCmd,
aliasCmd) is mutated, leaving flag values intact.
In `@pkg/cmd/testutil/example_validator.go`:
- Around line 95-108: The parsing currently discards the hasValue boolean from
parseLongFlag/parseShortFlag causing `--name`, `--name=`, and `--name ""` to be
indistinguishable; add per-flag metadata (e.g., a new map on the cmd struct like
FlagHasValue map[string]bool) and set FlagHasValue[flagName] = hasValue inside
both parseLongFlag and parseShortFlag handling (alongside existing
FlagPresent/FlagValues/Flags assignments), then update the validation logic that
currently checks flagValue == "" to instead consult FlagHasValue (and
FlagPresent) so you treat explicitly-supplied empty values differently from
omitted values.
- Around line 68-70: splitCommandLine currently accepts input that ends with an
open quote (inQuote==true) and returns a successful parts slice; change
splitCommandLine so it detects unterminated quotes and returns an error (e.g.,
"unterminated quoted argument") instead of silently succeeding, and update
parseCommandLine to propagate and return that error from splitCommandLine (so
parseCommandLine returns the error for inputs like `kortex-cli init --name
"proj`). Also audit the other parsing usage block around the second parser logic
(the code referenced at the alternative location) and make the same change there
so both call sites validate and reject unterminated quoted arguments.
🪄 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: 5c71a821-f079-453f-8050-6aaead03d65c
📒 Files selected for processing (7)
AGENTS.mdpkg/cmd/helpers.gopkg/cmd/helpers_test.gopkg/cmd/list.gopkg/cmd/remove.gopkg/cmd/testutil/example_validator.gopkg/cmd/testutil/example_validator_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/cmd/list.go
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/cmd/testutil/example_validator_test.go`:
- Around line 209-210: The test fixture incorrectly registers a non-existent
"-f" alias for the production workspace list command; remove the
Flags().StringP("format","f","text",...) call (or replace it with the real
"-o"/"--output" alias) so listCmd only defines the actual output flag via
Flags().String("output",...) and then update any table rows or test cases (the
ones around the second occurrence noted) to use "-o" or "--output" instead of
"-f"/"format" so the tests match the real command behavior.
- Around line 216-230: The test CLI tree (rootCmd, workspaceCmd, initCmd in
example_validator_test.go) has drifted from the real CLI: update the fixture to
mirror the production root (or import/instantiate the real root command) so it
includes the init command's single positional argument and the
--workspace-configuration flag defined in pkg/cmd/init.go, and ensure the
top-level list/remove aliases registered in pkg/cmd/root.go are present (also
apply the same fix for the duplicated fixture at lines 387-401); either replace
the fake rootCmd with the real root constructor or reproduce those exact
flags/positional args and aliases on initCmd and rootCmd so examples validate
against the real CLI surface.
🪄 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: 703289b2-e009-45e3-9c42-c10c6c322d9d
📒 Files selected for processing (1)
pkg/cmd/testutil/example_validator_test.go
benoitf
approved these changes
Mar 12, 2026
Adds Example fields to init, workspace list, and workspace remove commands to improve help documentation. Includes a validation utility in pkg/cmd/testutil that parses and validates examples against the actual command structure, ensuring examples stay accurate as the codebase evolves. The validator catches invalid flags, wrong binary names, invalid subcommands, while correctly allowing valid positional arguments. Updated AGENTS.md to require examples and validation tests for all new commands. Closes kortex-hub#53 Co-Authored-By: Claude Code (Claude Sonnet 4.5) <noreply@anthropic.com> Signed-off-by: Philippe Martin <phmartin@redhat.com>
Adds AdaptExampleForAlias utility function that allows alias commands to inherit examples from their original commands while adapting them to show the alias syntax. The function replaces the original command with the alias only in command lines, preserving comments unchanged. Applied to list and remove alias commands. Updated AGENTS.md with documentation and usage examples for creating aliases with examples. Co-Authored-By: Claude Code (Claude Sonnet 4.5) <noreply@anthropic.com> Signed-off-by: Philippe Martin <phmartin@redhat.com>
Signed-off-by: Philippe Martin <phmartin@redhat.com>
Runtime command code (list.go, remove.go) was importing testutil solely for AdaptExampleForAlias, which coupled runtime code to test-only utilities. Moved the function to pkg/cmd/helpers.go as a neutral runtime helper, along with its tests. Co-Authored-By: Claude Code (Claude Sonnet 4.5) <noreply@anthropic.com> Signed-off-by: Philippe Martin <phmartin@redhat.com>
The ExampleCommand structure now tracks flag presence and values separately using FlagPresent and FlagValues maps, allowing the validator to distinguish between flags with no value vs empty values. This enables validation that string flags requiring values (NoOptDefVal == "") actually have values provided, while still allowing boolean flags to be used without values. Co-Authored-By: Claude Code (Claude Sonnet 4.5) <noreply@anthropic.com> Signed-off-by: Philippe Martin <phmartin@redhat.com>
ValidateExampleCommand now invokes cmd.Args(cmd, remainingArgs) to validate remaining arguments, respecting each command's Args validator (cobra.NoArgs, cobra.ExactArgs, etc.) instead of only checking HasSubCommands(). This provides more accurate validation that matches runtime command behavior. Co-Authored-By: Claude Code (Claude Sonnet 4.5) <noreply@anthropic.com> Signed-off-by: Philippe Martin <phmartin@redhat.com>
There was a problem hiding this comment.
♻️ Duplicate comments (2)
pkg/cmd/testutil/example_validator_test.go (1)
196-230:⚠️ Potential issue | 🟠 MajorBuild these validator tests from the real command constructors.
These fixtures have already drifted from production: Lines 209-210 invent
-f/--formatforworkspace list, and theinitstubs omit production contracts likecobra.MaximumNArgs(1)and--workspace-configuration. That means this suite can certify examples the realNewRootCmd()would reject, or miss valid examples the real CLI accepts. PreferNewRootCmd()or a single shared fixture kept in lockstep with the production commands.Also applies to: 387-401
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/cmd/testutil/example_validator_test.go` around lines 196 - 230, The test fixture function createTestCommandTree diverges from production command definitions (it invents flags like -f/--format for workspace list and omits real contracts such as cobra.MaximumNArgs(1) and --workspace-configuration on init), so replace the hand-rolled tree with the actual production constructor (call NewRootCmd() or use the shared fixture used by production) inside the tests; ensure tests reference the real command tree instead of createTestCommandTree (or update createTestCommandTree to delegate to NewRootCmd()), preserving real flags, Args validators and persistent flags for workspace, list, init and root commands so examples validated by tests match the real CLI behavior.pkg/cmd/testutil/example_validator.go (1)
69-71:⚠️ Potential issue | 🟠 MajorDon’t collapse distinct flag syntaxes into the same parsed state.
The parser currently loses too much information:
splitCommandLine()drops empty quoted tokens and never errors on EOF inside a quote, whileparseCommandLine()throws awayhasValue. As a result,--name,--name=, and--name ""collapse into the same state, and malformed input likekortex-cli init --name "projcan still parse as valid. Please preserve empty quoted values, retain an explicithasValuebit, and fail on unterminated quotes.Also applies to: 93-108, 155-185, 257-264
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/cmd/testutil/example_validator.go` around lines 69 - 71, splitCommandLine currently drops empty quoted tokens and ignores unterminated quotes, and parseCommandLine discards whether a flag had an explicit value; update splitCommandLine to preserve empty quoted tokens (return "" for ""), return an error on EOF inside an open quote, and change its signature to propagate that error; update parseCommandLine to track an explicit hasValue boolean on parsed flags/options (distinct from the value string) so --name, --name=, and --name "" are distinguishable, and ensure parsing logic treats "=" (even with empty RHS) as hasValue=true while absence of "=" is hasValue=false; update callers to handle the new error and hasValue flag accordingly (references: splitCommandLine and parseCommandLine).
🧹 Nitpick comments (1)
pkg/cmd/list.go (1)
34-34: Validate the aliasExampleonce.Line 34 publishes a different help string than
NewWorkspaceListCmd(), so the original command’s example test does not prove the alias help stays executable. A smallValidateCommandExamples(NewRootCmd(), NewListCmd().Example)regression test would catch alias-only breakage inAdaptExampleForAlias().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/cmd/list.go` at line 34, The alias help example isn’t being validated: add a regression test that calls ValidateCommandExamples(NewRootCmd(), NewListCmd().Example) (or ValidateCommandExamples(NewRootCmd(), workspaceListCmd.Example) depending on which Example is the alias) to ensure AdaptExampleForAlias() keeps the alias Example executable; locate AdaptExampleForAlias, NewWorkspaceListCmd/NewListCmd and workspaceListCmd.Example in the code and add a unit test that constructs the root command and validates the alias Example once to catch alias-only breakage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@pkg/cmd/testutil/example_validator_test.go`:
- Around line 196-230: The test fixture function createTestCommandTree diverges
from production command definitions (it invents flags like -f/--format for
workspace list and omits real contracts such as cobra.MaximumNArgs(1) and
--workspace-configuration on init), so replace the hand-rolled tree with the
actual production constructor (call NewRootCmd() or use the shared fixture used
by production) inside the tests; ensure tests reference the real command tree
instead of createTestCommandTree (or update createTestCommandTree to delegate to
NewRootCmd()), preserving real flags, Args validators and persistent flags for
workspace, list, init and root commands so examples validated by tests match the
real CLI behavior.
In `@pkg/cmd/testutil/example_validator.go`:
- Around line 69-71: splitCommandLine currently drops empty quoted tokens and
ignores unterminated quotes, and parseCommandLine discards whether a flag had an
explicit value; update splitCommandLine to preserve empty quoted tokens (return
"" for ""), return an error on EOF inside an open quote, and change its
signature to propagate that error; update parseCommandLine to track an explicit
hasValue boolean on parsed flags/options (distinct from the value string) so
--name, --name=, and --name "" are distinguishable, and ensure parsing logic
treats "=" (even with empty RHS) as hasValue=true while absence of "=" is
hasValue=false; update callers to handle the new error and hasValue flag
accordingly (references: splitCommandLine and parseCommandLine).
---
Nitpick comments:
In `@pkg/cmd/list.go`:
- Line 34: The alias help example isn’t being validated: add a regression test
that calls ValidateCommandExamples(NewRootCmd(), NewListCmd().Example) (or
ValidateCommandExamples(NewRootCmd(), workspaceListCmd.Example) depending on
which Example is the alias) to ensure AdaptExampleForAlias() keeps the alias
Example executable; locate AdaptExampleForAlias, NewWorkspaceListCmd/NewListCmd
and workspaceListCmd.Example in the code and add a unit test that constructs the
root command and validates the alias Example once to catch alias-only breakage.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 150e3af1-0688-40a9-a49b-b7be36e156a2
📒 Files selected for processing (13)
AGENTS.mdpkg/cmd/helpers.gopkg/cmd/helpers_test.gopkg/cmd/init.gopkg/cmd/init_test.gopkg/cmd/list.gopkg/cmd/remove.gopkg/cmd/testutil/example_validator.gopkg/cmd/testutil/example_validator_test.gopkg/cmd/workspace_list.gopkg/cmd/workspace_list_test.gopkg/cmd/workspace_remove.gopkg/cmd/workspace_remove_test.go
🚧 Files skipped from review as they are similar to previous changes (7)
- pkg/cmd/init.go
- pkg/cmd/helpers.go
- pkg/cmd/remove.go
- pkg/cmd/workspace_remove.go
- pkg/cmd/workspace_remove_test.go
- pkg/cmd/init_test.go
- pkg/cmd/workspace_list.go
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
3 participants
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.
Adds Example fields to init, workspace list, and workspace remove commands to improve help documentation. Includes a validation utility in pkg/cmd/testutil that parses and validates examples against the actual command structure, ensuring examples stay accurate as the
codebase evolves.
The validator catches invalid flags, wrong binary names, invalid subcommands, while correctly allowing valid positional arguments.
Updated AGENTS.md to require examples and validation tests for all new commands.
Adds AdaptExampleForAlias utility function that allows alias commands to inherit examples from their original commands while adapting them to show the alias syntax. The function replaces the original command with the alias only in command lines, preserving comments unchanged.
Applied to list and remove alias commands. Updated AGENTS.md with documentation and usage examples for creating aliases with examples.
Closes #53