feat(sheets): add cell operation shortcuts for merge, replace, and style#412
feat(sheets): add cell operation shortcuts for merge, replace, and style#412fangshuyu-768 merged 1 commit intomainfrom
Conversation
📝 WalkthroughWalkthroughAdds five new Sheets shortcuts— Changes
Sequence Diagram(s)sequenceDiagram
participant User as CLI User
participant CLI as CLI Handler
participant Validator as Validator
participant Executor as Executor
participant API as Sheets API
participant Output as Output Handler
User->>CLI: Invoke sheets +<operation>\n(--url/--spreadsheet-token, flags)
CLI->>Validator: Validate flags
Validator->>Validator: Derive spreadsheet token\n(from --spreadsheet-token or --url)
Validator->>Validator: Validate operation-specific inputs
Validator-->>CLI: Return validation result
alt DryRun
CLI->>Executor: Build DryRun request
Executor->>Executor: Normalize ranges / parse JSON
Executor-->>CLI: Return request preview
CLI-->>User: Display request preview
else Execute
CLI->>Executor: Execute operation
Executor->>Executor: Prepare payload / path-encode token
Executor->>API: POST/PUT /open-apis/sheets/...
API-->>Executor: Return response
Executor->>Output: Write result to runtime.Out
Output-->>User: Print JSON output
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR adds 5 new Confidence Score: 5/5Safe to merge; all remaining findings are P2 suggestions that do not block functionality. No P0 or P1 issues found. All five shortcuts follow established codebase patterns (mirroring the existing shortcuts/sheets/sheet_batch_set_style.go — minor UX improvement possible for per-item structure validation Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant CLI
participant Validate
participant DryRun
participant LarkAPI
User->>CLI: lark-cli sheets +merge-cells / +unmerge-cells / +set-style / +batch-set-style / +replace [flags]
CLI->>Validate: Check token, range, JSON shape
Validate-->>CLI: error | nil
alt --dry-run
CLI->>DryRun: Build DryRunAPI (method, URL, body)
DryRun-->>User: Print preview (no network call)
else execute
CLI->>LarkAPI: POST/PUT /open-apis/sheets/v2 or v3/...
Note over CLI,LarkAPI: merge_cells · unmerge_cells · style · styles_batch_update · sheets/{id}/replace
LarkAPI-->>CLI: JSON response
CLI-->>User: Print JSON output
end
Reviews (4): Last reviewed commit: "feat(sheets): add cell operation shortcu..." | Re-trigger Greptile |
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@4daf883739f6bf8331dbd850a2e9a7624be301ae🧩 Skill updatenpx skills add larksuite/cli#feat/sheet_cell -y -g |
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 `@shortcuts/sheets/sheet_replace.go`:
- Around line 41-43: validateSheetRangeInput currently lets absolute ranges like
"sheet2!A1:B2" bypass validation, so calling SheetReplace with --sheet-id sheet1
and --range sheet2!A1:B2 produces a contradictory request; update
validateSheetRangeInput to parse the range for an explicit sheet name and, if
present, ensure it matches the provided sheet-id (rejecting with an error on
mismatch), or change the function signature used by the three call sites (the
validateSheetRangeInput calls in SheetReplace flow) to accept both sheet-id and
range and perform this check; add a regression test that calls the SheetReplace
entrypoint with conflicting --sheet-id and absolute --range and asserts it
returns an error.
In `@shortcuts/sheets/sheet_set_style.go`:
- Around line 37-40: The current unmarshalling into a plain interface{} allows
arrays, strings, and null for runtime.Str("style"); change the validation so
that after json.Unmarshal you assert the result is an object
(map[string]interface{} or map[string]any) and return a FlagErrorf if it's not;
specifically update the block around variable style, the json.Unmarshal call,
and the place that assigns to appendStyle.style to only accept an object-shaped
map and reject other JSON types with an error like "--style must be a JSON
object".
🪄 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: 74ad2f72-1fe0-4362-84ca-7fe86f9a0343
📒 Files selected for processing (13)
shortcuts/sheets/sheet_batch_set_style.goshortcuts/sheets/sheet_cell_ops_test.goshortcuts/sheets/sheet_merge_cells.goshortcuts/sheets/sheet_replace.goshortcuts/sheets/sheet_set_style.goshortcuts/sheets/sheet_unmerge_cells.goshortcuts/sheets/shortcuts.goskills/lark-sheets/SKILL.mdskills/lark-sheets/references/lark-sheets-batch-set-style.mdskills/lark-sheets/references/lark-sheets-merge-cells.mdskills/lark-sheets/references/lark-sheets-replace.mdskills/lark-sheets/references/lark-sheets-set-style.mdskills/lark-sheets/references/lark-sheets-unmerge-cells.md
8961893 to
e9456db
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
shortcuts/sheets/sheet_unmerge_cells.go (1)
28-31: Consider centralizing spreadsheet token resolution to avoid drift.The same
--url/--spreadsheet-tokenresolution logic is duplicated inValidate,DryRun, andExecute. A small helper (e.g.,resolveSpreadsheetToken(runtime)) would reduce maintenance risk.Refactor sketch
+func resolveSpreadsheetToken(runtime *common.RuntimeContext) string { + token := runtime.Str("spreadsheet-token") + if runtime.Str("url") != "" { + token = extractSpreadsheetToken(runtime.Str("url")) + } + return token +} ... - token := runtime.Str("spreadsheet-token") - if runtime.Str("url") != "" { - token = extractSpreadsheetToken(runtime.Str("url")) - } + token := resolveSpreadsheetToken(runtime)Also applies to: 41-44, 54-57
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/sheets/sheet_unmerge_cells.go` around lines 28 - 31, The spreadsheet token resolution logic (using runtime.Str("spreadsheet-token") and overriding with extractSpreadsheetToken(runtime.Str("url"))) is duplicated across Validate, DryRun, and Execute; create a small helper function (e.g., resolveSpreadsheetToken(runtime) or resolveSpreadsheetTokenFromRuntime) that accepts the runtime object, encapsulates the token selection logic and extractSpreadsheetToken call, and replace the duplicated blocks in Validate, DryRun, and Execute with calls to this helper to centralize behavior and avoid drift.shortcuts/sheets/sheet_set_style.go (1)
75-77: KeepExecutestyle-type checks consistent withValidate.
Validateenforces object-only JSON, butExecutecurrently re-parses without re-checking the object type. Consider using one shared parser/validator to keep behavior consistent and prevent future drift.Based on learnings: “In the `larksuite/cli` shortcut runner, `Validate` is always executed before `DryRun` … guards are intentionally kept in `Validate` and not duplicated in `DryRun`.”Refactor sketch
+func parseStyleObject(raw string) (map[string]interface{}, error) { + var style map[string]interface{} + if err := json.Unmarshal([]byte(raw), &style); err != nil { + return nil, common.FlagErrorf("--style must be valid JSON: %v", err) + } + if style == nil { + return nil, common.FlagErrorf("--style must be a JSON object") + } + return style, nil +} ... - var style interface{} - if err := json.Unmarshal([]byte(runtime.Str("style")), &style); err != nil { - return common.FlagErrorf("--style must be valid JSON: %v", err) - } + style, err := parseStyleObject(runtime.Str("style")) + if err != nil { + return err + }Also applies to: 83-86
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/sheets/sheet_set_style.go` around lines 75 - 77, Execute currently unmarshals runtime.Str("style") with json.Unmarshal into style without re-checking that the JSON is an object like Validate enforces; update Execute (in sheet_set_style.go, the Execute method that calls json.Unmarshal([]byte(runtime.Str("style")), &style)) to reuse the same parsing/validation logic used by Validate (or call a shared helper) so the parsed value is verified to be a JSON object (map[string]interface{}) before proceeding and return a FlagError if it is not; ensure the helper accepts the raw string (runtime.Str("style")), returns the parsed object, and is used by both Validate and Execute to keep behavior consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@shortcuts/sheets/sheet_set_style.go`:
- Around line 75-77: Execute currently unmarshals runtime.Str("style") with
json.Unmarshal into style without re-checking that the JSON is an object like
Validate enforces; update Execute (in sheet_set_style.go, the Execute method
that calls json.Unmarshal([]byte(runtime.Str("style")), &style)) to reuse the
same parsing/validation logic used by Validate (or call a shared helper) so the
parsed value is verified to be a JSON object (map[string]interface{}) before
proceeding and return a FlagError if it is not; ensure the helper accepts the
raw string (runtime.Str("style")), returns the parsed object, and is used by
both Validate and Execute to keep behavior consistent.
In `@shortcuts/sheets/sheet_unmerge_cells.go`:
- Around line 28-31: The spreadsheet token resolution logic (using
runtime.Str("spreadsheet-token") and overriding with
extractSpreadsheetToken(runtime.Str("url"))) is duplicated across Validate,
DryRun, and Execute; create a small helper function (e.g.,
resolveSpreadsheetToken(runtime) or resolveSpreadsheetTokenFromRuntime) that
accepts the runtime object, encapsulates the token selection logic and
extractSpreadsheetToken call, and replace the duplicated blocks in Validate,
DryRun, and Execute with calls to this helper to centralize behavior and avoid
drift.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bc1b952d-7b47-41b3-baec-313caf16957e
📒 Files selected for processing (13)
shortcuts/sheets/sheet_batch_set_style.goshortcuts/sheets/sheet_cell_ops_test.goshortcuts/sheets/sheet_merge_cells.goshortcuts/sheets/sheet_replace.goshortcuts/sheets/sheet_set_style.goshortcuts/sheets/sheet_unmerge_cells.goshortcuts/sheets/shortcuts.goskills/lark-sheets/SKILL.mdskills/lark-sheets/references/lark-sheets-batch-set-style.mdskills/lark-sheets/references/lark-sheets-merge-cells.mdskills/lark-sheets/references/lark-sheets-replace.mdskills/lark-sheets/references/lark-sheets-set-style.mdskills/lark-sheets/references/lark-sheets-unmerge-cells.md
✅ Files skipped from review due to trivial changes (7)
- shortcuts/sheets/shortcuts.go
- skills/lark-sheets/SKILL.md
- skills/lark-sheets/references/lark-sheets-replace.md
- skills/lark-sheets/references/lark-sheets-merge-cells.md
- skills/lark-sheets/references/lark-sheets-batch-set-style.md
- skills/lark-sheets/references/lark-sheets-unmerge-cells.md
- skills/lark-sheets/references/lark-sheets-set-style.md
🚧 Files skipped from review as they are similar to previous changes (4)
- shortcuts/sheets/sheet_batch_set_style.go
- shortcuts/sheets/sheet_merge_cells.go
- shortcuts/sheets/sheet_cell_ops_test.go
- shortcuts/sheets/sheet_replace.go
e9456db to
f2ddb21
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/sheets/sheet_batch_set_style.go`:
- Around line 28-31: The code currently overwrites an explicit
--spreadsheet-token with a token parsed from --url; update the token selection
in Validate, DryRun, and Execute so that runtime.Str("spreadsheet-token") is
used as the primary value and only if it is empty do you parse and assign from
runtime.Str("url") (i.e., change the conditional to check token == "" before
calling extractSpreadsheetToken for Validate, DryRun, and Execute).
🪄 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: 30d73169-2e50-4e3b-a27b-c21f8759e93c
📒 Files selected for processing (13)
shortcuts/sheets/sheet_batch_set_style.goshortcuts/sheets/sheet_cell_ops_test.goshortcuts/sheets/sheet_merge_cells.goshortcuts/sheets/sheet_replace.goshortcuts/sheets/sheet_set_style.goshortcuts/sheets/sheet_unmerge_cells.goshortcuts/sheets/shortcuts.goskills/lark-sheets/SKILL.mdskills/lark-sheets/references/lark-sheets-batch-set-style.mdskills/lark-sheets/references/lark-sheets-merge-cells.mdskills/lark-sheets/references/lark-sheets-replace.mdskills/lark-sheets/references/lark-sheets-set-style.mdskills/lark-sheets/references/lark-sheets-unmerge-cells.md
✅ Files skipped from review due to trivial changes (7)
- shortcuts/sheets/shortcuts.go
- skills/lark-sheets/references/lark-sheets-replace.md
- skills/lark-sheets/references/lark-sheets-unmerge-cells.md
- skills/lark-sheets/references/lark-sheets-merge-cells.md
- skills/lark-sheets/references/lark-sheets-batch-set-style.md
- shortcuts/sheets/sheet_cell_ops_test.go
- skills/lark-sheets/references/lark-sheets-set-style.md
🚧 Files skipped from review as they are similar to previous changes (5)
- skills/lark-sheets/SKILL.md
- shortcuts/sheets/sheet_merge_cells.go
- shortcuts/sheets/sheet_unmerge_cells.go
- shortcuts/sheets/sheet_set_style.go
- shortcuts/sheets/sheet_replace.go
Add 5 new sheet shortcuts for cell operations: - +merge-cells: merge cells with MERGE_ALL/MERGE_ROWS/MERGE_COLUMNS - +unmerge-cells: split merged cells - +replace: find and replace cell values - +set-style: set cell style (font, color, alignment, border) - +batch-set-style: batch set styles for multiple ranges Includes unit tests (81-89% coverage) and skill reference docs.
f2ddb21 to
4daf883
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
shortcuts/sheets/sheet_unmerge_cells.go (1)
28-31: Factor token resolution into one helper to prevent drift.The same
url/spreadsheet-tokenprecedence logic is repeated three times. A shared helper keeps behavior consistent acrossValidate,DryRun, andExecute.Also applies to: 41-44, 54-57
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/sheets/sheet_unmerge_cells.go` around lines 28 - 31, Create a shared helper (e.g., resolveSpreadsheetToken) that encapsulates the precedence logic currently duplicated in Validate, DryRun, and Execute: read runtime.Str("spreadsheet-token") then override it if runtime.Str("url") != "" by calling extractSpreadsheetToken(runtime.Str("url")); replace the three repeated code blocks in the methods (the token := ... and conditional override) with a single call to this helper so behavior stays consistent across Validate, DryRun, and Execute.
🤖 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/sheets/sheet_replace.go`:
- Around line 34-37: The code currently unconditionally overwrites token :=
runtime.Str("spreadsheet-token") with
extractSpreadsheetToken(runtime.Str("url")), which loses an explicit
--spreadsheet-token when URL extraction fails; change each overwrite (the token
assignment using extractSpreadsheetToken in the blocks around the token
variable) to first call extracted := extractSpreadsheetToken(runtime.Str("url"))
and only set token = extracted when extracted is non-empty/valid (e.g., != ""),
leaving the original token intact otherwise; apply this same pattern to the
other two occurrences where runtime.Str("url") is used to replace the token.
In `@shortcuts/sheets/sheet_unmerge_cells.go`:
- Line 35: The current call to validateSheetRangeInput(runtime.Str("sheet-id"),
runtime.Str("range")) only checks shape but not consistency between a provided
--sheet-id flag and a sheet-prefixed range like "<sheetId>!A1:B2"; update the
logic (either inside validateSheetRangeInput or just before calling it in
sheet_unmerge_cells.go) to detect when both a separate sheet-id flag and a
sheet-prefixed range are present and, if their IDs differ, return an error
refusing to proceed so the command cannot silently ignore --sheet-id and write
to the wrong sheet; reference validateSheetRangeInput and the runtime inputs
"sheet-id" and "range" when implementing the conflict check.
---
Nitpick comments:
In `@shortcuts/sheets/sheet_unmerge_cells.go`:
- Around line 28-31: Create a shared helper (e.g., resolveSpreadsheetToken) that
encapsulates the precedence logic currently duplicated in Validate, DryRun, and
Execute: read runtime.Str("spreadsheet-token") then override it if
runtime.Str("url") != "" by calling extractSpreadsheetToken(runtime.Str("url"));
replace the three repeated code blocks in the methods (the token := ... and
conditional override) with a single call to this helper so behavior stays
consistent across Validate, DryRun, and Execute.
🪄 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: 5964dc13-afb5-42cf-83e4-7482b6f30ed3
📒 Files selected for processing (13)
shortcuts/sheets/sheet_batch_set_style.goshortcuts/sheets/sheet_cell_ops_test.goshortcuts/sheets/sheet_merge_cells.goshortcuts/sheets/sheet_replace.goshortcuts/sheets/sheet_set_style.goshortcuts/sheets/sheet_unmerge_cells.goshortcuts/sheets/shortcuts.goskills/lark-sheets/SKILL.mdskills/lark-sheets/references/lark-sheets-batch-set-style.mdskills/lark-sheets/references/lark-sheets-merge-cells.mdskills/lark-sheets/references/lark-sheets-replace.mdskills/lark-sheets/references/lark-sheets-set-style.mdskills/lark-sheets/references/lark-sheets-unmerge-cells.md
✅ Files skipped from review due to trivial changes (7)
- skills/lark-sheets/references/lark-sheets-unmerge-cells.md
- skills/lark-sheets/SKILL.md
- skills/lark-sheets/references/lark-sheets-batch-set-style.md
- skills/lark-sheets/references/lark-sheets-merge-cells.md
- skills/lark-sheets/references/lark-sheets-set-style.md
- shortcuts/sheets/sheet_cell_ops_test.go
- skills/lark-sheets/references/lark-sheets-replace.md
🚧 Files skipped from review as they are similar to previous changes (4)
- shortcuts/sheets/shortcuts.go
- shortcuts/sheets/sheet_set_style.go
- shortcuts/sheets/sheet_batch_set_style.go
- shortcuts/sheets/sheet_merge_cells.go
Summary
Add 5 new
sheetsshortcuts for cell operations: merge/unmerge cells, find-and-replace, set style, and batch set style.Changes
+merge-cells: merge cells with MERGE_ALL/MERGE_ROWS/MERGE_COLUMNS+unmerge-cells: split merged cells+replace: find and replace cell values with regex/case-sensitive options+set-style: set cell style (font, color, alignment, border)+batch-set-style: batch set styles for multiple rangesTest Plan
go test ./shortcuts/sheets/)lark-cli sheets +xxxcommands work as expectedRelated Issues
Summary by CodeRabbit
New Features
Documentation
Tests