Skip to content

fix(cli): make completion command discoverable#307

Open
zfaustk wants to merge 5 commits intolarksuite:mainfrom
zfaustk:main
Open

fix(cli): make completion command discoverable#307
zfaustk wants to merge 5 commits intolarksuite:mainfrom
zfaustk:main

Conversation

@zfaustk
Copy link
Copy Markdown

@zfaustk zfaustk commented Apr 7, 2026

Summary

Fixes #264.

lark-cli completion already works, but it is hidden from normal command discovery and is not documented in either README. This change makes the command visible and adds short shell completion setup examples in both English and Chinese docs.

Changes

  • remove Hidden: true from cmd/completion/completion.go
  • add cmd/completion/completion_test.go to verify the command stays visible and auth-free
  • document shell completion usage in README.md
  • document shell completion usage in README.zh.md

Why this change

  • issue #264 reports that the command is usable today but hidden from users
  • CHANGELOG.md already describes completion support as a shipped feature
  • keeping the command hidden makes --help discovery weaker than the actual product surface

Validation

  • git diff --check
  • focused source review of cmd/completion/completion.go, cmd/root.go, README.md, and README.zh.md
  • Go tests not run in this environment because go is not installed in the container

Summary by CodeRabbit

  • New Features

    • Shell completion command is now visible and directly accessible in the CLI help output
  • Documentation

    • Added comprehensive setup guides for shell completion with platform-specific installation instructions for Bash, Zsh, and Fish shells
    • Clarified that completion functionality operates independently without requiring user authentication

@github-actions github-actions bot added the size/L Large or sensitive change across domains or core paths label Apr 7, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 7, 2026

📝 Walkthrough

Walkthrough

The lark-cli completion subcommand is made visible by removing the hidden flag from its command definition. Documentation for generating shell completion scripts (Bash, Zsh, Fish) is added to both English and Chinese README files. A test is added to verify the command is visible and auth-free.

Changes

Cohort / File(s) Summary
Documentation
README.md, README.zh.md
Added sections documenting lark-cli completion as a top-level command without authentication requirements. Includes shell-specific installation instructions for Bash, Zsh, and Fish shell completions.
Command Visibility
cmd/completion/completion.go
Removed Hidden: true flag to make the completion command visible in CLI help output.
Test Coverage
cmd/completion/completion_test.go
Added new test TestNewCmdCompletion_IsVisibleAndAuthFree to verify the completion command is not hidden and authentication checks are disabled.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A hidden command now sees the light,
No secret magic, no auth required—right!
Shell completions bloom in Bash and Fish,
Documentation granted—a rabbit's wish! 🌟

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix(cli): make completion command discoverable' directly and specifically describes the main change: making the hidden completion command visible.
Description check ✅ Passed The description includes all required template sections: Summary (with issue reference), Changes (bulleted list of modifications), and Validation details explaining testing approach.
Linked Issues check ✅ Passed The PR fully addresses issue #264 by making the completion command visible through code changes and documenting usage in both README files as requested.
Out of Scope Changes check ✅ Passed All changes directly support the linked issue objective: visibility fix via code change, test to verify visibility, and documentation in both English and Chinese READMEs.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

Copy link
Copy Markdown

@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: 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 `@README.md`:
- Around line 101-104: The Zsh completion snippet is missing the fpath and
compinit configuration; update the README near the Zsh section (after the mkdir
and the generation step that writes to ~/.zfunc/_lark-cli) to instruct users to
add their function directory to fpath and initialize completions by adding
fpath=(~/.zfunc $fpath) and running autoload -Uz compinit && compinit in their
.zshrc so the generated _lark-cli file is discovered and activated.

In `@README.zh.md`:
- Around line 101-104: The Zsh section only creates ~/.zfunc and the _lark-cli
file but omits telling users to add that directory to their zsh fpath and
initialize completions; update the README.zh.md Zsh instructions to tell users
to add ~/.zfunc to their fpath in their .zshrc and to run the zsh completion
initialization (compinit) so the _lark-cli completion is loaded.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: f07e8763-b24d-4ed6-9155-70271d1c27dc

📥 Commits

Reviewing files that changed from the base of the PR and between d6fada0 and c9fe226.

📒 Files selected for processing (4)
  • README.md
  • README.zh.md
  • cmd/completion/completion.go
  • cmd/completion/completion_test.go
💤 Files with no reviewable changes (1)
  • cmd/completion/completion.go

Comment on lines +101 to +104
# Zsh
mkdir -p ~/.zfunc
lark-cli completion zsh > ~/.zfunc/_lark-cli
```
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Zsh completion setup is incomplete (same issue as Chinese README).

Missing fpath configuration. Users need to add the function path to .zshrc:

fpath=(~/.zfunc $fpath)
autoload -Uz compinit && compinit
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@README.md` around lines 101 - 104, The Zsh completion snippet is missing the
fpath and compinit configuration; update the README near the Zsh section (after
the mkdir and the generation step that writes to ~/.zfunc/_lark-cli) to instruct
users to add their function directory to fpath and initialize completions by
adding fpath=(~/.zfunc $fpath) and running autoload -Uz compinit && compinit in
their .zshrc so the generated _lark-cli file is discovered and activated.

Comment on lines +101 to +104
# Zsh
mkdir -p ~/.zfunc
lark-cli completion zsh > ~/.zfunc/_lark-cli
```
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Zsh completion setup is incomplete.

The Zsh instructions create the file but don't mention adding ~/.zfunc to fpath. Users would need to add this to their .zshrc:

fpath=(~/.zfunc $fpath)
autoload -Uz compinit && compinit

Consider adding this note or linking to the Zsh completion documentation.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@README.zh.md` around lines 101 - 104, The Zsh section only creates ~/.zfunc
and the _lark-cli file but omits telling users to add that directory to their
zsh fpath and initialize completions; update the README.zh.md Zsh instructions
to tell users to add ~/.zfunc to their fpath in their .zshrc and to run the zsh
completion initialization (compinit) so the _lark-cli completion is loaded.

@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Apr 7, 2026

Greptile Summary

Removes Hidden: true from the completion cobra command so it appears in --help output, adds a regression test to guard against re-hiding, and documents shell completion setup in both READMEs. One P2 doc gap: the zsh examples in both README files omit the fpath and compinit wiring that zsh requires to actually load the generated completion file.

Confidence Score: 5/5

Safe to merge; the core change is a one-field removal and the regression test is well-written

All findings are P2 documentation style issues. The logic change (removing Hidden: true) is minimal and correct, and the new test properly guards both the visibility and auth-skip properties. No correctness or reliability issues in the code.

README.md and README.zh.md — the zsh completion examples should include fpath and compinit instructions to avoid silent failures for zsh users

Important Files Changed

Filename Overview
cmd/completion/completion.go Removes Hidden: true so the command is discoverable; DisableAuthCheck and all four shell cases are correct
cmd/completion/completion_test.go New regression test correctly asserts cmd.Hidden is false and auth is disabled via TestFactory
README.md Adds shell completion docs; zsh block missing fpath and compinit setup instructions needed for completions to activate
README.zh.md Chinese equivalent of README.md; same zsh fpath/compinit omission

Sequence Diagram

sequenceDiagram
    participant U as User Shell
    participant B as lark-cli binary
    participant C as completion cmd

    U->>B: lark-cli completion bash|zsh|fish|powershell
    B->>C: cobra routes to NewCmdCompletion
    Note over C: Auth check skipped (DisableAuthCheck)
    Note over C: Update notice suppressed (isCompletionCommand)
    C->>B: root.GenXxxCompletion(out)
    B->>U: completion script written to stdout
    U->>U: user redirects to shell completion dir
    Note over U: zsh: ~/.zfunc/_lark-cli also needs<br/>fpath + compinit in .zshrc
Loading

Reviews (1): Last reviewed commit: "Create completion_test.go" | Re-trigger Greptile

Comment on lines +101 to +104
# Zsh
mkdir -p ~/.zfunc
lark-cli completion zsh > ~/.zfunc/_lark-cli
```
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Zsh setup missing fpath and compinit steps

The example writes a completion file to ~/.zfunc/_lark-cli but does not tell users to register that directory with zsh or initialize the completion system. Without adding fpath=(~/.zfunc $fpath) and autoload -Uz compinit && compinit to ~/.zshrc, the file exists on disk but zsh never loads it — completions silently fail. The same gap is present in README.zh.md (lines 101–103).

Suggested change
# Zsh
mkdir -p ~/.zfunc
lark-cli completion zsh > ~/.zfunc/_lark-cli
```
# Zsh
mkdir -p ~/.zfunc
lark-cli completion zsh > ~/.zfunc/_lark-cli
# Add to ~/.zshrc (if not already present):
# fpath=(~/.zfunc $fpath)
# autoload -Uz compinit && compinit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/L Large or sensitive change across domains or core paths

Projects

None yet

Development

Successfully merging this pull request may close these issues.

lark-cli completion 子命令被隐藏

1 participant