feat: guide lark-doc v2 usage#710
Conversation
|
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 API-version-aware help rendering for the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 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. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (9.09%) is below the target coverage (60.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #710 +/- ##
==========================================
+ Coverage 64.06% 64.13% +0.06%
==========================================
Files 503 504 +1
Lines 44091 44313 +222
==========================================
+ Hits 28249 28422 +173
- Misses 13382 13417 +35
- Partials 2460 2474 +14 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@5451f8adb191b24417fa95b924a730ff06ef5d21🧩 Skill updatenpx skills add larksuite/cli#sun/online -y -g |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@shortcuts/doc/shortcuts.go`:
- Around line 48-50: The early return when checking
cmd.Flags().Lookup("api-version") stops the rest of the help wiring and Tips
installation; instead, change the flow so the lookup only skips
creating/registering the "api-version" flag but does NOT return early. Locate
the block that checks cmd.Flags().Lookup("api-version") and remove the return;
wrap only the flag-creation logic in an if/else (or guard) so that if the flag
exists you skip adding it, but still proceed to run the custom help function
wiring and the Tips output installation for the command (ensure the code that
installs help/Tips runs unconditionally after the flag existence check).
In `@shortcuts/register_test.go`:
- Around line 85-87: The tests are creating a raw &cmdutil.Factory{} for
RegisterShortcuts(program, &cmdutil.Factory{}); replace that with the standard
test helper by calling cmdutil.TestFactory(t, config) and pass the returned
factory into RegisterShortcuts (e.g., factory := cmdutil.TestFactory(t, config);
RegisterShortcuts(program, factory)); also ensure you isolate config state per
test by creating a temp directory and setting LARKSUITE_CLI_CONFIG_DIR to it
(cleanup after test) so each test uses its own config directory.
In `@skills/lark-doc/references/lark-doc-fetch.md`:
- Around line 52-55: The docs claim a 4-layer fallback for `keyword`/`--keyword`
(substring → normalization → tokenization → RE2) but the CLI contract currently
only forwards the raw `keyword` as "substring + regex"; update all occurrences
(including the block describing `keyword` and the example at the later mention)
to reflect the actual contract: remove the 4-layer fallback wording and the
implied `\|` multi-OR semantics unless the CLI implements them, and clearly
state that `--keyword` accepts a raw value that is treated as substring match or
an RE2 regex; adjust the example (`"部署\|发布\|上线"`) and any mention of automatic
dedup/fallback to match real CLI behavior.
In `@skills/lark-doc/references/lark-doc-xml.md`:
- Line 88: The docs list "purple" as a base color but the codebase lacks
support; either remove "purple" from the base color list in
skills/lark-doc/references/lark-doc-xml.md (change the sentence listing base
colors to: red, orange, yellow, green, blue, gray) or implement full purple
support by adding "purple" to your color enum/type (e.g., Color /
allowedColors), updating any color validation logic (e.g., isValidColor or
validateColor), adding mapping/stylesheet entries where base colors are defined,
and including example usages in lark-doc-xml.md (and tests) that demonstrate
purple rendering so docs and code stay consistent.
🪄 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: 2c2adb77-0bb5-4cdf-97da-b10be0f0a0bb
📒 Files selected for processing (9)
shortcuts/doc/docs_fetch_v2.goshortcuts/doc/shortcuts.goshortcuts/register.goshortcuts/register_test.goskills/lark-doc/SKILL.mdskills/lark-doc/references/lark-doc-create.mdskills/lark-doc/references/lark-doc-fetch.mdskills/lark-doc/references/lark-doc-xml.mdskills/lark-doc/references/style/lark-doc-create-workflow.md
37a224f to
87a8c9f
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
shortcuts/doc/shortcuts.go (1)
48-50:⚠️ Potential issue | 🟠 MajorRemove the early return so help wiring always installs.
Line 48–Line 50 currently returns when
--api-versionalready exists, which skipsSetHelpFuncand the “Tips:” guidance entirely for preconfigured/reuseddocscommands.Proposed fix
cmd.Long = strings.TrimSpace(docsServiceHelpDefault) - if cmd.Flags().Lookup("api-version") != nil { - return + if cmd.Flags().Lookup("api-version") == nil { + cmd.Flags().String("api-version", "", "show docs help for API version (v1|v2)") + cmdutil.RegisterFlagCompletion(cmd, "api-version", func(_ *cobra.Command, _ []string, _ string) ([]string, cobra.ShellCompDirective) { + return []string{"v1", "v2"}, cobra.ShellCompDirectiveNoFileComp + }) } - cmd.Flags().String("api-version", "", "show docs help for API version (v1|v2)") - cmdutil.RegisterFlagCompletion(cmd, "api-version", func(_ *cobra.Command, _ []string, _ string) ([]string, cobra.ShellCompDirective) { - return []string{"v1", "v2"}, cobra.ShellCompDirectiveNoFileComp - }) defaultHelp := cmd.HelpFunc()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/doc/shortcuts.go` around lines 48 - 50, The early return when cmd.Flags().Lookup("api-version") != nil prevents SetHelpFunc and the "Tips:" help wiring from being installed for preconfigured/reused docs commands; remove that return and instead only skip creating the flag when it already exists. Concretely, replace the current if (Lookup != nil) return with logic that checks cmd.Flags().Lookup("api-version") and only creates/registers the flag when Lookup returns nil, but always call cmd.SetHelpFunc(...) (and any help/tips wiring) so help text is installed even if the flag was preconfigured.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@shortcuts/doc/shortcuts.go`:
- Around line 48-50: The early return when cmd.Flags().Lookup("api-version") !=
nil prevents SetHelpFunc and the "Tips:" help wiring from being installed for
preconfigured/reused docs commands; remove that return and instead only skip
creating the flag when it already exists. Concretely, replace the current if
(Lookup != nil) return with logic that checks cmd.Flags().Lookup("api-version")
and only creates/registers the flag when Lookup returns nil, but always call
cmd.SetHelpFunc(...) (and any help/tips wiring) so help text is installed even
if the flag was preconfigured.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6d7045bc-6aaf-4e4b-9d97-80844b9bb251
📒 Files selected for processing (9)
shortcuts/doc/docs_fetch_v2.goshortcuts/doc/shortcuts.goshortcuts/register.goshortcuts/register_test.goskills/lark-doc/SKILL.mdskills/lark-doc/references/lark-doc-create.mdskills/lark-doc/references/lark-doc-fetch.mdskills/lark-doc/references/lark-doc-xml.mdskills/lark-doc/references/style/lark-doc-create-workflow.md
✅ Files skipped from review due to trivial changes (3)
- skills/lark-doc/references/lark-doc-xml.md
- shortcuts/doc/docs_fetch_v2.go
- skills/lark-doc/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (3)
- shortcuts/register.go
- shortcuts/register_test.go
- skills/lark-doc/references/lark-doc-fetch.md
87a8c9f to
b50af07
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (2)
shortcuts/register_test.go (1)
84-87:⚠️ Potential issue | 🟠 MajorUse standard test factory + config isolation in these new tests
Line 86 and Line 122 use a raw
&cmdutil.Factory{}. Please switch these tests tocmdutil.TestFactory(t, config)and isolate config state per test withLARKSUITE_CLI_CONFIG_DIR=t.TempDir().As per coding guidelines: "
**/*_test.go: Usecmdutil.TestFactory(t, config)for test factories" and "Isolate config state in tests by settingLARKSUITE_CLI_CONFIG_DIRto a temporary directory".Also applies to: 120-123
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/register_test.go` around lines 84 - 87, Replace the raw test factory usage in TestRegisterShortcutsDocsHelpAddsVersionSelectorAndLegacyTips (and the similar test at lines ~120-123) by creating a test factory via cmdutil.TestFactory(t, config) instead of &cmdutil.Factory{}, and isolate config state by setting the environment variable LARKSUITE_CLI_CONFIG_DIR to t.TempDir() at the start of each test; update calls that pass the factory into RegisterShortcuts to use the returned TestFactory value so tests use the standard test factory and an isolated config directory.shortcuts/doc/shortcuts.go (1)
48-50:⚠️ Potential issue | 🟠 MajorDo not return early when
--api-versionalready existsLine 48–Line 50 exits before wiring the custom help function, so docs tips/versioned help are skipped whenever the flag is pre-registered. Keep the existence check only around flag registration, not around help wiring.
Suggested fix
cmd.Long = strings.TrimSpace(docsServiceHelpDefault) - if cmd.Flags().Lookup("api-version") != nil { - return + if cmd.Flags().Lookup("api-version") == nil { + cmd.Flags().String("api-version", "", "show docs help for API version (v1|v2)") + cmdutil.RegisterFlagCompletion(cmd, "api-version", func(_ *cobra.Command, _ []string, _ string) ([]string, cobra.ShellCompDirective) { + return []string{"v1", "v2"}, cobra.ShellCompDirectiveNoFileComp + }) } - cmd.Flags().String("api-version", "", "show docs help for API version (v1|v2)") - cmdutil.RegisterFlagCompletion(cmd, "api-version", func(_ *cobra.Command, _ []string, _ string) ([]string, cobra.ShellCompDirective) { - return []string{"v1", "v2"}, cobra.ShellCompDirectiveNoFileComp - })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/doc/shortcuts.go` around lines 48 - 50, The early return triggered by if cmd.Flags().Lookup("api-version") != nil { return } prevents wiring the custom help function and should be removed; instead, only skip registering the flag when it already exists. Locate the block using cmd.Flags().Lookup("api-version") and change the logic so that you still execute the help wiring code (the custom help/versioned help setup) regardless of the flag's presence, but guard only the flag registration call (e.g., RegisterFlag/Add flag code) behind the existence check so existing pre-registered flags are not re-registered.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@shortcuts/doc/shortcuts.go`:
- Around line 48-50: The early return triggered by if
cmd.Flags().Lookup("api-version") != nil { return } prevents wiring the custom
help function and should be removed; instead, only skip registering the flag
when it already exists. Locate the block using cmd.Flags().Lookup("api-version")
and change the logic so that you still execute the help wiring code (the custom
help/versioned help setup) regardless of the flag's presence, but guard only the
flag registration call (e.g., RegisterFlag/Add flag code) behind the existence
check so existing pre-registered flags are not re-registered.
In `@shortcuts/register_test.go`:
- Around line 84-87: Replace the raw test factory usage in
TestRegisterShortcutsDocsHelpAddsVersionSelectorAndLegacyTips (and the similar
test at lines ~120-123) by creating a test factory via cmdutil.TestFactory(t,
config) instead of &cmdutil.Factory{}, and isolate config state by setting the
environment variable LARKSUITE_CLI_CONFIG_DIR to t.TempDir() at the start of
each test; update calls that pass the factory into RegisterShortcuts to use the
returned TestFactory value so tests use the standard test factory and an
isolated config directory.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 03bc43fe-18b8-4ec0-93f0-ddf466c8e353
📒 Files selected for processing (9)
shortcuts/doc/docs_fetch_v2.goshortcuts/doc/shortcuts.goshortcuts/register.goshortcuts/register_test.goskills/lark-doc/SKILL.mdskills/lark-doc/references/lark-doc-create.mdskills/lark-doc/references/lark-doc-fetch.mdskills/lark-doc/references/lark-doc-xml.mdskills/lark-doc/references/style/lark-doc-create-workflow.md
✅ Files skipped from review due to trivial changes (5)
- shortcuts/doc/docs_fetch_v2.go
- skills/lark-doc/references/lark-doc-xml.md
- skills/lark-doc/SKILL.md
- skills/lark-doc/references/lark-doc-create.md
- skills/lark-doc/references/lark-doc-fetch.md
b50af07 to
ed6c8a2
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
shortcuts/register_test.go (1)
19-24: Drop the redundant config-dir env override in the test factory helper.
cmdutil.TestFactoryalready supplies in-memory config, so settingLARKSUITE_CLI_CONFIG_DIRhere is unnecessary process-global mutation for these unit tests.Based on learnings: "Shortcut tests that build the CLI via cmdutil.TestFactory(t, config) should not set this env var because TestFactory provides an in-memory config closure and does not access the filesystem."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/register_test.go` around lines 19 - 24, The helper newRegisterTestFactory sets the environment variable LARKSUITE_CLI_CONFIG_DIR unnecessarily even though cmdutil.TestFactory(t, &core.CliConfig{}) already provides an in-memory config; remove the t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) call from newRegisterTestFactory so the tests rely solely on cmdutil.TestFactory's in-memory config behavior (locate the call in the newRegisterTestFactory function and delete that single t.Setenv line).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@shortcuts/register_test.go`:
- Around line 158-195: The test sets the "api-version" flag on the child
fetchCmd but that flag is defined on the parent docs command; update the test
(TestRegisterShortcutsDocsFetchHelpKeepsShortcutHelp) to set the flag on the
parent docs command before rendering fetchCmd.Help()—obtain the parent either by
finding the docs command (e.g., program.Find([]string{"docs"}) or using
fetchCmd.Parent()) and call docsCmd.Flags().Set("api-version","v2") so the
parent is in v2 mode while asserting fetchCmd's help output.
---
Nitpick comments:
In `@shortcuts/register_test.go`:
- Around line 19-24: The helper newRegisterTestFactory sets the environment
variable LARKSUITE_CLI_CONFIG_DIR unnecessarily even though
cmdutil.TestFactory(t, &core.CliConfig{}) already provides an in-memory config;
remove the t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) call from
newRegisterTestFactory so the tests rely solely on cmdutil.TestFactory's
in-memory config behavior (locate the call in the newRegisterTestFactory
function and delete that single t.Setenv line).
🪄 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: eb81b659-491c-423e-a821-f0d9f24ee2ec
📒 Files selected for processing (9)
shortcuts/doc/docs_fetch_v2.goshortcuts/doc/shortcuts.goshortcuts/register.goshortcuts/register_test.goskills/lark-doc/SKILL.mdskills/lark-doc/references/lark-doc-create.mdskills/lark-doc/references/lark-doc-fetch.mdskills/lark-doc/references/lark-doc-xml.mdskills/lark-doc/references/style/lark-doc-create-workflow.md
✅ Files skipped from review due to trivial changes (2)
- shortcuts/doc/docs_fetch_v2.go
- skills/lark-doc/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (4)
- shortcuts/register.go
- skills/lark-doc/references/lark-doc-create.md
- skills/lark-doc/references/lark-doc-xml.md
- skills/lark-doc/references/lark-doc-fetch.md
## Summary
Add explicit guidance on the parent `docs` command so agents pick the right
lark-doc API version. Without this, agents that have an older lark-doc skill
installed can mistakenly mix v2 flags into a v1 flow.
## Changes
- Add `--api-version` help flag and a Tips section to `docs` so `lark docs --help`
(and `--api-version v2`) explain when v2 should be used.
- Refresh the lark-doc skill references and `docs_fetch_v2` keyword flag
description for clarity.
- Add `shortcuts/register_test.go` covering the new docs help wiring.
## Test Plan
- [x] Unit tests pass (`go test ./shortcuts/...`)
- [x] Manual local verification confirms the `lark docs --help` and
`lark docs --help --api-version v2` commands work as expected
## Related Issues
- None
Change-Id: Id3b3196e6a069bb52f95a6fc679b8258313faf3d
ed6c8a2 to
5451f8a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@shortcuts/doc/shortcuts.go`:
- Around line 75-80: The "Tips" block is being printed to stdout via out :=
cmd.OutOrStdout() but should be written to stderr as hint content; change the
write target to use cmd.ErrOrStderr() (e.g., err := cmd.ErrOrStderr()) and
replace fmt.Fprintln/out/ fmt.Fprintf calls that print the "Tips:" header and
loop over docsVersionSelectionTips to use that stderr writer instead so hints go
to stderr while keeping any JSON/program output on stdout.
🪄 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: 740a2055-faad-4913-aeca-d79f5975dfc6
📒 Files selected for processing (13)
shortcuts/doc/docs_create.goshortcuts/doc/docs_fetch.goshortcuts/doc/docs_fetch_v2.goshortcuts/doc/docs_update.goshortcuts/doc/shortcuts.goshortcuts/doc/versioned_help.goshortcuts/register.goshortcuts/register_test.goskills/lark-doc/SKILL.mdskills/lark-doc/references/lark-doc-create.mdskills/lark-doc/references/lark-doc-fetch.mdskills/lark-doc/references/lark-doc-xml.mdskills/lark-doc/references/style/lark-doc-create-workflow.md
💤 Files with no reviewable changes (1)
- shortcuts/doc/versioned_help.go
✅ Files skipped from review due to trivial changes (5)
- shortcuts/doc/docs_update.go
- shortcuts/doc/docs_create.go
- skills/lark-doc/references/lark-doc-xml.md
- shortcuts/doc/docs_fetch_v2.go
- shortcuts/doc/docs_fetch.go
🚧 Files skipped from review as they are similar to previous changes (4)
- skills/lark-doc/references/lark-doc-create.md
- skills/lark-doc/references/lark-doc-fetch.md
- shortcuts/register.go
- shortcuts/register_test.go
Change-Id: Id3b3196e6a069bb52f95a6fc679b8258313faf3d
feat: guide lark-doc v2 usage
Summary
Add explicit guidance on the parent
docscommand so agents pick the rightlark-doc API version. Without this, agents that have an older lark-doc skill
installed can mistakenly mix v2 flags into a v1 flow.
Changes
--api-versionhelp flag and a Tips section todocssolark docs --help(and
--api-version v2) explain when v2 should be used.docs_fetch_v2keyword flagdescription for clarity.
shortcuts/register_test.gocovering the new docs help wiring.Test Plan
go test ./shortcuts/...)lark docs --helpandlark docs --help --api-version v2commands work as expectedSummary by CodeRabbit
New Features
Documentation
Tests