feat(sheets): add dimension shortcuts for row/column operations#413
Merged
fangshuyu-768 merged 1 commit intomainfrom Apr 11, 2026
Merged
feat(sheets): add dimension shortcuts for row/column operations#413fangshuyu-768 merged 1 commit intomainfrom
fangshuyu-768 merged 1 commit intomainfrom
Conversation
github-actions
bot
added
domain/ccm
|
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:
📝 WalkthroughWalkthroughIntroduces five new Google Sheets shortcuts for managing spreadsheet dimensions: Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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 SummaryAdds five sheet dimension shortcuts ( Confidence Score: 5/5Safe to merge; all previous P1 concerns are resolved and only a minor docs gap remains. Both previously-flagged P1 issues (missing destination-index validation in move, missing fixed-size lower-bound check in update) are addressed. All five shortcuts follow the established codebase pattern, are well-tested with 48 cases, and the API endpoints/HTTP methods match the Lark spec. The one remaining finding is a P2 doc gap in the permissions table. skills/lark-sheets/SKILL.md — permissions table should be updated to include the 5 new shortcuts. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User invokes lark-cli sheets dimension-cmd] --> B[Validate]
B --> C{Token source?}
C -->|url flag| D[extractSpreadsheetToken]
C -->|spreadsheet-token flag| E[Use directly]
D --> F{Token empty?}
E --> F
F -->|yes| G[FlagError: missing token]
F -->|no| H{Operation-specific guards}
H -->|add-dimension| I[length in 1..5000]
H -->|insert-dimension| J[start >= 0 and end > start]
H -->|update-dimension| K[start >= 1 and end >= start and visible or fixed-size set]
H -->|move-dimension| L[start >= 0 and end >= start and destination >= 0]
H -->|delete-dimension| M[start >= 1 and end >= start]
I --> N[Execute or DryRun]
J --> N
K --> N
L --> N
M --> N
N -->|DryRun| O[Print API call preview]
N -->|Execute| P[CallAPI]
P --> Q[v2 POST dimension_range - add]
P --> R[v2 POST insert_dimension_range - insert]
P --> S[v2 PUT dimension_range - update]
P --> T[v3 POST move_dimension - move]
P --> U[v2 DELETE dimension_range - delete]
Reviews (5): Last reviewed commit: "feat(sheets): add dimension shortcuts fo..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@fb62faf4d273becb6cec31b1fd74aa7f3adfdc45🧩 Skill updatenpx skills add larksuite/cli#feat/sheet_range -y -g |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
shortcuts/sheets/sheet_update_dimension.go (1)
45-47: Add validation for--fixed-sizeto reject negative values.The suggestion to add input validation for
--fixed-sizeis valid. However, the proposed threshold needs correction: the Lark/Feishu Sheets API explicitly allowsfixedSize = 0(used to hide rows/columns), so the check should reject only negative values, not zero.Update the validation to:
if runtime.Cmd.Flags().Changed("fixed-size") && runtime.Int("fixed-size") < 0 { return common.FlagErrorf("--fixed-size must be >= 0") }This applies to lines 45–47, 59–61, and 85–87.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/sheets/sheet_update_dimension.go` around lines 45 - 47, Add input validation so that when the flag "--fixed-size" is present you reject negative values (but allow zero); specifically, replace the existing check that blocks when both "--visible" and "--fixed-size" are unset with an additional guard: if runtime.Cmd.Flags().Changed("fixed-size") && runtime.Int("fixed-size") < 0 { return common.FlagErrorf("--fixed-size must be >= 0") }. Apply this exact validation where similar flag checks occur (the blocks using runtime.Cmd.Flags().Changed("visible") / runtime.Cmd.Flags().Changed("fixed-size") and runtime.Int("fixed-size")) in the three places mentioned so negative fixed-size values return an error while zero is permitted.shortcuts/sheets/sheet_add_dimension.go (1)
28-33: Extract shared token/body builders to avoid drift between Validate, DryRun, and Execute.The token resolution and request payload are duplicated across three blocks. Centralizing this will reduce future mismatch risk between dry-run and real execution paths.
♻️ Suggested refactor
+func resolveSpreadsheetToken(runtime *common.RuntimeContext) string { + token := runtime.Str("spreadsheet-token") + if runtime.Str("url") != "" { + token = extractSpreadsheetToken(runtime.Str("url")) + } + return token +} + +func buildAddDimensionBody(runtime *common.RuntimeContext) map[string]interface{} { + return map[string]interface{}{ + "dimension": map[string]interface{}{ + "sheetId": runtime.Str("sheet-id"), + "majorDimension": runtime.Str("dimension"), + "length": runtime.Int("length"), + }, + } +} + Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { - token := runtime.Str("spreadsheet-token") - if runtime.Str("url") != "" { - token = extractSpreadsheetToken(runtime.Str("url")) - } + token := resolveSpreadsheetToken(runtime) ... DryRun: func(ctx context.Context, runtime *common.RuntimeContext) *common.DryRunAPI { - token := runtime.Str("spreadsheet-token") - if runtime.Str("url") != "" { - token = extractSpreadsheetToken(runtime.Str("url")) - } + token := resolveSpreadsheetToken(runtime) return common.NewDryRunAPI(). POST("/open-apis/sheets/v2/spreadsheets/:token/dimension_range"). - Body(map[string]interface{}{ ... }). + Body(buildAddDimensionBody(runtime)). Set("token", token) }, Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { - token := runtime.Str("spreadsheet-token") - if runtime.Str("url") != "" { - token = extractSpreadsheetToken(runtime.Str("url")) - } + token := resolveSpreadsheetToken(runtime) data, err := runtime.CallAPI("POST", fmt.Sprintf("/open-apis/sheets/v2/spreadsheets/%s/dimension_range", validate.EncodePathSegment(token)), nil, - map[string]interface{}{ ... }, + buildAddDimensionBody(runtime), )Also applies to: 42-57, 58-74
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/sheets/sheet_add_dimension.go` around lines 28 - 33, The Validate, DryRun, and Execute blocks duplicate token resolution and request-body construction; extract this logic into a shared helper (e.g., createSheetRequest or resolveSpreadsheetTokenAndBody) and call it from Validate, DryRun, and Execute to ensure consistent behavior. Specifically, centralize the token resolution that currently uses runtime.Str("spreadsheet-token") and extractSpreadsheetToken(runtime.Str("url")), and build the request payload used by the Execute/DryRun paths inside that helper; then replace the duplicated code in Validate, DryRun, and Execute with calls to the new helper to return the token and request body.
🤖 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_move_dimension.go`:
- Around line 30-44: The Validate function in sheet_move_dimension.go currently
checks start-index and end-index but misses a lower-bound check for
destination-index; add a non-negative validation for
runtime.Int("destination-index") (e.g., if runtime.Int("destination-index") < 0
return common.FlagErrorf("--destination-index must be >= 0")) inside the same
Validate function so negative destination indices fail fast before the API call.
---
Nitpick comments:
In `@shortcuts/sheets/sheet_add_dimension.go`:
- Around line 28-33: The Validate, DryRun, and Execute blocks duplicate token
resolution and request-body construction; extract this logic into a shared
helper (e.g., createSheetRequest or resolveSpreadsheetTokenAndBody) and call it
from Validate, DryRun, and Execute to ensure consistent behavior. Specifically,
centralize the token resolution that currently uses
runtime.Str("spreadsheet-token") and
extractSpreadsheetToken(runtime.Str("url")), and build the request payload used
by the Execute/DryRun paths inside that helper; then replace the duplicated code
in Validate, DryRun, and Execute with calls to the new helper to return the
token and request body.
In `@shortcuts/sheets/sheet_update_dimension.go`:
- Around line 45-47: Add input validation so that when the flag "--fixed-size"
is present you reject negative values (but allow zero); specifically, replace
the existing check that blocks when both "--visible" and "--fixed-size" are
unset with an additional guard: if runtime.Cmd.Flags().Changed("fixed-size") &&
runtime.Int("fixed-size") < 0 { return common.FlagErrorf("--fixed-size must be
>= 0") }. Apply this exact validation where similar flag checks occur (the
blocks using runtime.Cmd.Flags().Changed("visible") /
runtime.Cmd.Flags().Changed("fixed-size") and runtime.Int("fixed-size")) in the
three places mentioned so negative fixed-size values return an error while zero
is permitted.
🪄 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: e9dd42d1-cec6-464f-9c5a-27d67fdda900
📒 Files selected for processing (13)
shortcuts/sheets/sheet_add_dimension.goshortcuts/sheets/sheet_delete_dimension.goshortcuts/sheets/sheet_dimension_test.goshortcuts/sheets/sheet_insert_dimension.goshortcuts/sheets/sheet_move_dimension.goshortcuts/sheets/sheet_update_dimension.goshortcuts/sheets/shortcuts.goskills/lark-sheets/SKILL.mdskills/lark-sheets/references/lark-sheets-add-dimension.mdskills/lark-sheets/references/lark-sheets-delete-dimension.mdskills/lark-sheets/references/lark-sheets-insert-dimension.mdskills/lark-sheets/references/lark-sheets-move-dimension.mdskills/lark-sheets/references/lark-sheets-update-dimension.md
caojie0621
force-pushed
the
feat/sheet_range
branch
from
efe5d33 to
13fc344
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
shortcuts/sheets/sheet_add_dimension.go (1)
42-80: Consider extracting token resolution to reduce duplication.The token extraction logic (
runtime.Str("spreadsheet-token")/extractSpreadsheetToken(runtime.Str("url"))) is repeated identically inValidate,DryRun, andExecute. This pattern appears across all dimension shortcuts. While functional, extracting a helper method would improve maintainability.That said, this matches the existing pattern in sibling shortcuts (e.g.,
sheet_update_dimension.go,sheet_insert_dimension.go), so consistency is preserved.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/sheets/sheet_add_dimension.go` around lines 42 - 80, Extract the repeated token resolution into a helper (e.g., add a function resolveSpreadsheetToken(runtime *common.RuntimeContext) string) and replace the duplicated logic in DryRun and Execute (and Validate if present) with a single call to that helper; ensure the helper returns runtime.Str("spreadsheet-token") by default and uses extractSpreadsheetToken(runtime.Str("url")) when url is non-empty so the POST path building (fmt.Sprintf/validate.EncodePathSegment and common.NewDryRunAPI().Set("token", ...)) and body construction remain unchanged.
🤖 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_add_dimension.go`:
- Around line 42-80: Extract the repeated token resolution into a helper (e.g.,
add a function resolveSpreadsheetToken(runtime *common.RuntimeContext) string)
and replace the duplicated logic in DryRun and Execute (and Validate if present)
with a single call to that helper; ensure the helper returns
runtime.Str("spreadsheet-token") by default and uses
extractSpreadsheetToken(runtime.Str("url")) when url is non-empty so the POST
path building (fmt.Sprintf/validate.EncodePathSegment and
common.NewDryRunAPI().Set("token", ...)) and body construction remain unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 302a3fcd-9495-449d-b663-06bfb05c8b10
📒 Files selected for processing (13)
shortcuts/sheets/sheet_add_dimension.goshortcuts/sheets/sheet_delete_dimension.goshortcuts/sheets/sheet_dimension_test.goshortcuts/sheets/sheet_insert_dimension.goshortcuts/sheets/sheet_move_dimension.goshortcuts/sheets/sheet_update_dimension.goshortcuts/sheets/shortcuts.goskills/lark-sheets/SKILL.mdskills/lark-sheets/references/lark-sheets-add-dimension.mdskills/lark-sheets/references/lark-sheets-delete-dimension.mdskills/lark-sheets/references/lark-sheets-insert-dimension.mdskills/lark-sheets/references/lark-sheets-move-dimension.mdskills/lark-sheets/references/lark-sheets-update-dimension.md
✅ Files skipped from review due to trivial changes (5)
- skills/lark-sheets/references/lark-sheets-delete-dimension.md
- skills/lark-sheets/references/lark-sheets-update-dimension.md
- skills/lark-sheets/references/lark-sheets-add-dimension.md
- skills/lark-sheets/references/lark-sheets-move-dimension.md
- skills/lark-sheets/references/lark-sheets-insert-dimension.md
🚧 Files skipped from review as they are similar to previous changes (5)
- shortcuts/sheets/shortcuts.go
- skills/lark-sheets/SKILL.md
- shortcuts/sheets/sheet_update_dimension.go
- shortcuts/sheets/sheet_insert_dimension.go
- shortcuts/sheets/sheet_delete_dimension.go
caojie0621
force-pushed
the
feat/sheet_range
branch
from
13fc344 to
96ab01a
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
shortcuts/sheets/sheet_delete_dimension.go (1)
29-33: Refactor token resolution to a shared helper.
--url/--spreadsheet-tokenresolution is repeated inValidate,DryRun, andExecute. This is easy to drift over time (precedence, trimming, future validation rules). Consider centralizing it in one helper and reusing it in all three hooks.♻️ Suggested 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: 45-49, 62-66
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/sheets/sheet_delete_dimension.go` around lines 29 - 33, The token resolution logic duplicated in Validate, DryRun, and Execute should be extracted into a single helper (e.g., resolveSpreadsheetToken or getSpreadsheetToken) that accepts the RuntimeContext and returns the final token after applying the precedence (url over spreadsheet-token), trimming and any validation; replace the inline lines in Validate, DryRun, and Execute with a call to that helper and update calls that currently use extractSpreadsheetToken/runtime.Str("spreadsheet-token") to use the new helper so behavior remains consistent across all three hooks.shortcuts/sheets/sheet_dimension_test.go (1)
109-127: Prefer structured JSON assertions over substring matching in DryRun tests.These checks are valid today, but
strings.Containson serialized JSON is brittle and verbose. A small helper that unmarshals intomap[string]any(or typed structs) would make failures clearer and reduce repetition.Also applies to: 245-264, 470-489, 637-655, 807-825
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/sheets/sheet_dimension_test.go` around lines 109 - 127, The DryRun test TestSheetAddDimensionDryRun uses brittle substring checks; replace them with structured JSON assertions by unmarshaling the marshalDryRun output into a map[string]any (or a small typed struct) and asserting the presence and values of keys like "dimension_range" -> map with "sheetId" == "sheet1" and "majorDimension" == "ROWS", and "length" == 8; add a reusable helper (e.g., unmarshalDryRun or assertDryRunField) and update TestSheetAddDimensionDryRun and the other listed tests (around lines 245-264, 470-489, 637-655, 807-825) to call that helper and perform type-safe checks instead of strings.Contains.cmd/tmptoken/main.go (1)
29-29: Parameterize hardcoded spreadsheet/range for reuse.The endpoint embeds fixed spreadsheet/range values, which makes this utility brittle and easy to misuse across environments. Consider flags/env inputs even for a temporary tool.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/tmptoken/main.go` at line 29, Replace the hardcoded Feishu sheets endpoint string used when creating the request (the POST to "https://open.feishu.cn/open-apis/sheets/v2/spreadsheets/.../values_image") with a constructed URL built from inputs: add flags or env vars for spreadsheet ID and range/path (e.g., spreadsheetID and valuesPath or a full baseEndpoint), read them in main (with sensible defaults/fallbacks), validate they are present, and build the final URL via fmt.Sprintf or url.Join before calling http.NewRequest using the existing body and req variables; ensure you update any error messages or logs to reference these new parameters (so the request creation in main uses the parameterized URL rather than the hardcoded string).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/tmptoken/main.go`:
- Around line 25-26: Guard access to os.Args[1], validate the input path with
validate.SafeInputPath, replace os.ReadFile with vfs.ReadFile and handle its
error instead of ignoring it: check argument count in main, call
validate.SafeInputPath on the provided path, use vfs.ReadFile to read into
imgData and return/log the error if any, then base64 encode imgData into b64;
update any references in main.go around the imgData and b64 variables
accordingly.
- Line 27: Replace manual JSON construction for variable body with proper JSON
marshaling: build a payload struct (e.g., type payload struct { Range string
`json:"range"`; Image string `json:"image"`; Name string `json:"name"` })
populated with "5351aa!D1:D1", the b64 variable, and os.Args[1], then call
json.Marshal to produce body and handle the error. Also validate input (check
len(os.Args) and use os.Stat on os.Args[1]) before file I/O, and stop ignoring
returned errors around the file read/base64 steps (handle the errors instead of
assigning to `_`) so any ReadFile/base64 or json.Marshal failures are checked
and returned/logged.
- Around line 33-41: The code uses http.DefaultClient with no timeout and
ignores non-2xx responses and io.ReadAll errors, and reads untrusted file paths
via os.ReadFile; replace http.DefaultClient.Do(req) with a custom
http.Client{Timeout: <reasonable duration>} and check resp.StatusCode (ensure
200-299) before reading the body, handle and propagate errors from io.ReadAll,
and fail with a clear message on non-2xx; also replace os.ReadFile usage with
vfs.ReadFile and validate the CLI path using validate.SafeInputPath before
reading to prevent unsafe input. Ensure you update the code paths around the
request/response handling (the call site using http.DefaultClient.Do,
resp.Body.Close, io.ReadAll) and the file-read site that currently calls
os.ReadFile so they follow these checks and error handling patterns.
---
Nitpick comments:
In `@cmd/tmptoken/main.go`:
- Line 29: Replace the hardcoded Feishu sheets endpoint string used when
creating the request (the POST to
"https://open.feishu.cn/open-apis/sheets/v2/spreadsheets/.../values_image") with
a constructed URL built from inputs: add flags or env vars for spreadsheet ID
and range/path (e.g., spreadsheetID and valuesPath or a full baseEndpoint), read
them in main (with sensible defaults/fallbacks), validate they are present, and
build the final URL via fmt.Sprintf or url.Join before calling http.NewRequest
using the existing body and req variables; ensure you update any error messages
or logs to reference these new parameters (so the request creation in main uses
the parameterized URL rather than the hardcoded string).
In `@shortcuts/sheets/sheet_delete_dimension.go`:
- Around line 29-33: The token resolution logic duplicated in Validate, DryRun,
and Execute should be extracted into a single helper (e.g.,
resolveSpreadsheetToken or getSpreadsheetToken) that accepts the RuntimeContext
and returns the final token after applying the precedence (url over
spreadsheet-token), trimming and any validation; replace the inline lines in
Validate, DryRun, and Execute with a call to that helper and update calls that
currently use extractSpreadsheetToken/runtime.Str("spreadsheet-token") to use
the new helper so behavior remains consistent across all three hooks.
In `@shortcuts/sheets/sheet_dimension_test.go`:
- Around line 109-127: The DryRun test TestSheetAddDimensionDryRun uses brittle
substring checks; replace them with structured JSON assertions by unmarshaling
the marshalDryRun output into a map[string]any (or a small typed struct) and
asserting the presence and values of keys like "dimension_range" -> map with
"sheetId" == "sheet1" and "majorDimension" == "ROWS", and "length" == 8; add a
reusable helper (e.g., unmarshalDryRun or assertDryRunField) and update
TestSheetAddDimensionDryRun and the other listed tests (around lines 245-264,
470-489, 637-655, 807-825) to call that helper and perform type-safe checks
instead of strings.Contains.
🪄 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: a10a95e4-790e-4eec-9832-62b1193f431e
📒 Files selected for processing (14)
cmd/tmptoken/main.goshortcuts/sheets/sheet_add_dimension.goshortcuts/sheets/sheet_delete_dimension.goshortcuts/sheets/sheet_dimension_test.goshortcuts/sheets/sheet_insert_dimension.goshortcuts/sheets/sheet_move_dimension.goshortcuts/sheets/sheet_update_dimension.goshortcuts/sheets/shortcuts.goskills/lark-sheets/SKILL.mdskills/lark-sheets/references/lark-sheets-add-dimension.mdskills/lark-sheets/references/lark-sheets-delete-dimension.mdskills/lark-sheets/references/lark-sheets-insert-dimension.mdskills/lark-sheets/references/lark-sheets-move-dimension.mdskills/lark-sheets/references/lark-sheets-update-dimension.md
✅ Files skipped from review due to trivial changes (5)
- skills/lark-sheets/references/lark-sheets-update-dimension.md
- skills/lark-sheets/references/lark-sheets-move-dimension.md
- skills/lark-sheets/references/lark-sheets-add-dimension.md
- skills/lark-sheets/references/lark-sheets-insert-dimension.md
- skills/lark-sheets/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (5)
- shortcuts/sheets/shortcuts.go
- skills/lark-sheets/references/lark-sheets-delete-dimension.md
- shortcuts/sheets/sheet_add_dimension.go
- shortcuts/sheets/sheet_insert_dimension.go
- shortcuts/sheets/sheet_update_dimension.go
caojie0621
force-pushed
the
feat/sheet_range
branch
from
96ab01a to
efb5892
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
license-eye has checked 885 files.
| Valid | Invalid | Ignored | Fixed |
|---|---|---|---|
| 583 | 1 | 301 | 0 |
Click to see the invalid file list
- cmd/tmptoken/main.go
Use this command to fix any missing license headers
```bash
docker run -it --rm -v $(pwd):/github/workspace apache/skywalking-eyes header fix
</details>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
cmd/tmptoken/main.go (1)
29-29:⚠️ Potential issue | 🟡 MinorHandle the
http.NewRequesterror.The error return from
http.NewRequestis ignored. While unlikely to fail with valid inputs, discarding errors masks issues and violates defensive coding practices.Proposed fix
- req, _ := http.NewRequest("POST", "https://open.feishu.cn/open-apis/sheets/v2/spreadsheets/HHfDsIOuZh9WSntt3licHRqynFc/values_image", strings.NewReader(body)) + req, err := http.NewRequest("POST", "https://open.feishu.cn/open-apis/sheets/v2/spreadsheets/HHfDsIOuZh9WSntt3licHRqynFc/values_image", strings.NewReader(body)) + if err != nil { + fmt.Fprintln(os.Stderr, "request build error:", err) + os.Exit(1) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/tmptoken/main.go` at line 29, The call to http.NewRequest when creating req ignores its error; update the code around the http.NewRequest("POST", "...values_image", strings.NewReader(body)) call to capture the returned error (e.g., req, err := http.NewRequest(...)), check err, and handle it appropriately (log the error and exit or return it from main) so that failures constructing the request are not silently ignored; ensure subsequent code uses the validated req variable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmd/tmptoken/main.go`:
- Line 17: ResolveConfig's error is being discarded; change the call to receive
the error (e.g., cfg, err := cmdutil.ResolveConfig("")) and if err != nil in
main (or the init routine) log the error and exit (use log.Fatalf or
fmt.Fprintf(os.Stderr, ...) + os.Exit(1)) so config resolution failures are
reported immediately before using cfg.
---
Duplicate comments:
In `@cmd/tmptoken/main.go`:
- Line 29: The call to http.NewRequest when creating req ignores its error;
update the code around the http.NewRequest("POST", "...values_image",
strings.NewReader(body)) call to capture the returned error (e.g., req, err :=
http.NewRequest(...)), check err, and handle it appropriately (log the error and
exit or return it from main) so that failures constructing the request are not
silently ignored; ensure subsequent code uses the validated req variable.
🪄 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: 77207c77-a32d-4f84-a420-3c2362b47fa6
📒 Files selected for processing (14)
cmd/tmptoken/main.goshortcuts/sheets/sheet_add_dimension.goshortcuts/sheets/sheet_delete_dimension.goshortcuts/sheets/sheet_dimension_test.goshortcuts/sheets/sheet_insert_dimension.goshortcuts/sheets/sheet_move_dimension.goshortcuts/sheets/sheet_update_dimension.goshortcuts/sheets/shortcuts.goskills/lark-sheets/SKILL.mdskills/lark-sheets/references/lark-sheets-add-dimension.mdskills/lark-sheets/references/lark-sheets-delete-dimension.mdskills/lark-sheets/references/lark-sheets-insert-dimension.mdskills/lark-sheets/references/lark-sheets-move-dimension.mdskills/lark-sheets/references/lark-sheets-update-dimension.md
✅ Files skipped from review due to trivial changes (7)
- shortcuts/sheets/shortcuts.go
- skills/lark-sheets/references/lark-sheets-update-dimension.md
- skills/lark-sheets/references/lark-sheets-delete-dimension.md
- skills/lark-sheets/SKILL.md
- skills/lark-sheets/references/lark-sheets-move-dimension.md
- skills/lark-sheets/references/lark-sheets-insert-dimension.md
- shortcuts/sheets/sheet_dimension_test.go
🚧 Files skipped from review as they are similar to previous changes (5)
- skills/lark-sheets/references/lark-sheets-add-dimension.md
- shortcuts/sheets/sheet_delete_dimension.go
- shortcuts/sheets/sheet_insert_dimension.go
- shortcuts/sheets/sheet_update_dimension.go
- shortcuts/sheets/sheet_move_dimension.go
Add 5 new sheet shortcuts for row/column management: - +add-dimension: append rows/columns at the end - +insert-dimension: insert rows/columns at a position - +update-dimension: update visibility and size - +move-dimension: move rows/columns to a new position - +delete-dimension: delete rows/columns Includes unit tests (89-100% coverage) and skill reference docs.
caojie0621
force-pushed
the
feat/sheet_range
branch
from
efb5892 to
fb62faf
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
shortcuts/sheets/sheet_dimension_test.go (1)
20-52: Wire the helper from the shortcut's real flag definitions.
newDimTestRuntimeonly registers whichever flags each test remembered to pass, so the validation and dry-run tests can still stay green after a production flag rename, type change, or removal. Building the Cobra command fromcommon.Shortcut.Flagswould keep these tests aligned with the real CLI contract and remove a lot of manual flag bookkeeping.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/sheets/sheet_dimension_test.go` around lines 20 - 52, newDimTestRuntime currently registers only the flags passed by each test, which can drift from the real CLI; update newDimTestRuntime to build its cobra.Command from the actual shortcut flag definitions (use common.Shortcut.Flags or the appropriate Shortcut instance that holds Flags) instead of iterating strFlags/intFlags/boolFlags, registering flags according to each Flag's type/name/default/usage on the cobra.Command, then parse and Set the passed test values into that command and return the common.RuntimeContext (keep the existing error handling/parsing/setting logic but source registered flags from common.Shortcut.Flags so tests reflect production flag definitions).
🤖 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_add_dimension.go`:
- Around line 28-39: The Validate currently lets a non-empty --url silently
override --spreadsheet-token; change the logic so the explicit token wins: if
runtime.Str("spreadsheet-token") is non-empty use it; if it's empty and --url is
provided, extract the token with extractSpreadsheetToken and use that; if both
are provided and the extracted token from --url differs from the explicit token
return a FlagErrorf indicating the mismatch. Implement this logic as a small
helper (e.g. getSpreadsheetToken(runtime *common.RuntimeContext) (string,
error)) and call it from Validate, DryRun, and Execute (referencing Validate,
DryRun, Execute and extractSpreadsheetToken) so all three paths share the same
behavior and add tests covering explicit-token only, url-only, and both-mismatch
cases.
---
Nitpick comments:
In `@shortcuts/sheets/sheet_dimension_test.go`:
- Around line 20-52: newDimTestRuntime currently registers only the flags passed
by each test, which can drift from the real CLI; update newDimTestRuntime to
build its cobra.Command from the actual shortcut flag definitions (use
common.Shortcut.Flags or the appropriate Shortcut instance that holds Flags)
instead of iterating strFlags/intFlags/boolFlags, registering flags according to
each Flag's type/name/default/usage on the cobra.Command, then parse and Set the
passed test values into that command and return the common.RuntimeContext (keep
the existing error handling/parsing/setting logic but source registered flags
from common.Shortcut.Flags so tests reflect production flag definitions).
🪄 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: 093cffea-90a5-4ac4-9f01-30b3798d4baa
📒 Files selected for processing (13)
shortcuts/sheets/sheet_add_dimension.goshortcuts/sheets/sheet_delete_dimension.goshortcuts/sheets/sheet_dimension_test.goshortcuts/sheets/sheet_insert_dimension.goshortcuts/sheets/sheet_move_dimension.goshortcuts/sheets/sheet_update_dimension.goshortcuts/sheets/shortcuts.goskills/lark-sheets/SKILL.mdskills/lark-sheets/references/lark-sheets-add-dimension.mdskills/lark-sheets/references/lark-sheets-delete-dimension.mdskills/lark-sheets/references/lark-sheets-insert-dimension.mdskills/lark-sheets/references/lark-sheets-move-dimension.mdskills/lark-sheets/references/lark-sheets-update-dimension.md
✅ Files skipped from review due to trivial changes (7)
- shortcuts/sheets/shortcuts.go
- skills/lark-sheets/references/lark-sheets-add-dimension.md
- skills/lark-sheets/references/lark-sheets-delete-dimension.md
- skills/lark-sheets/references/lark-sheets-update-dimension.md
- skills/lark-sheets/references/lark-sheets-move-dimension.md
- skills/lark-sheets/SKILL.md
- skills/lark-sheets/references/lark-sheets-insert-dimension.md
🚧 Files skipped from review as they are similar to previous changes (2)
- shortcuts/sheets/sheet_update_dimension.go
- shortcuts/sheets/sheet_move_dimension.go
fangshuyu-768
approved these changes
Apr 11, 2026
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
2 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.
Summary
Add 5 new
sheetsshortcuts for row/column dimension management, covering add, insert, update, move, and delete operations.Changes
+add-dimension: append rows/columns at the end of a sheet (POST/v2/.../dimension_range)+insert-dimension: insert rows/columns at a specified position with optional style inheritance (POST/v2/.../insert_dimension_range)+update-dimension: update row/column visibility and size (PUT/v2/.../dimension_range)+move-dimension: move rows/columns to a new position (POST/v3/.../move_dimension)+delete-dimension: delete rows/columns (DELETE/v2/.../dimension_range)Test Plan
go test ./shortcuts/sheets/)lark-cli sheets +xxxcommands work as expectedRelated Issues
Summary by CodeRabbit
Release Notes
New Features
Documentation
Tests