feat(doc): add --width/--height flags to docs +media-insert#832
Conversation
Extend buildBatchUpdateData signature with width and height int params. When mediaType is "image" and either dimension is positive, the value is included in the replace_image payload. Existing call sites pass 0, 0.
Add computeMissingDimension (pure ratio math) and detectImageDimensions (header-only image.DecodeConfig) with PNG/JPEG/GIF blank-import decoders, plus imageDimensions struct; drive with two new TDD tests.
|
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 ChangesImage sizing feature
Sequence DiagramsequenceDiagram
participant CLI as lark-cli (docs +media-insert)
participant Input as Source (file or clipboard)
participant Decoder as Image decoder
participant API as Docs API (blocks.batch_update)
CLI->>Input: read file path or clipboard bytes
CLI->>Decoder: request native dimensions (when needed)
Decoder-->>CLI: native width/height (or error)
CLI->>CLI: compute missing dimension (aspect-ratio)
CLI->>API: buildBatchUpdateData with replace_image (+width/+height)
API-->>CLI: response (block_id, status)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 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 unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
github-actions
Bot
added
domain/ccm
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@shortcuts/doc/doc_media_insert.go`:
- Around line 565-571: detectImageDimensionsFromPath opens an untrusted filePath
directly; call validate.SafeInputPath(filePath) at the start of
detectImageDimensionsFromPath (before fio.Open) and return any validation error
to prevent reading unsafe paths. Keep the existing signature and behavior
otherwise (open via fio.Open, defer Close, call detectImageDimensions), and
ensure the validation error is propagated so callers (e.g., DryRun/Execute)
receive the failure.
🪄 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: c198a35c-11cf-45cc-a55e-7bbcbcbfef84
📒 Files selected for processing (3)
shortcuts/doc/doc_media_insert.goshortcuts/doc/doc_media_insert_test.goskills/lark-doc/references/lark-doc-media-insert.md
|
|
fangshuyu-768
left a comment
fangshuyu-768
left a comment
There was a problem hiding this comment.
Thanks for the PR! Overall the implementation is clean and well-structured. I have a few concerns below — the division-by-zero risk (#1) should be addressed before merge, the rest are suggestions.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
shortcuts/doc/doc_media_insert_test.go (1)
715-748: ⚡ Quick winIsolate config state in the new validate runtime helper
At Line 715, this helper is used by parallel tests but does not isolate
LARKSUITE_CLI_CONFIG_DIR, which can leak ambient config state across tests.Suggested patch
func newMediaInsertValidateRuntimeWithSize(t *testing.T, doc, mediaType string, width, height int, setWidth, setHeight bool) *common.RuntimeContext { t.Helper() + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) cmd := &cobra.Command{Use: "docs +media-insert"}As per coding guidelines,
**/*_test.go: Uset.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir())to isolate config state in tests.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@shortcuts/doc/doc_media_insert_test.go` around lines 715 - 748, The helper newMediaInsertValidateRuntimeWithSize doesn't isolate LARKSUITE_CLI_CONFIG_DIR causing config leakage across parallel tests; add t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) at the top of newMediaInsertValidateRuntimeWithSize (before creating the cobra.Command and flags) so each test gets its own temporary config dir and environment isolation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@shortcuts/doc/doc_media_insert_test.go`:
- Around line 715-748: The helper newMediaInsertValidateRuntimeWithSize doesn't
isolate LARKSUITE_CLI_CONFIG_DIR causing config leakage across parallel tests;
add t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) at the top of
newMediaInsertValidateRuntimeWithSize (before creating the cobra.Command and
flags) so each test gets its own temporary config dir and environment isolation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6f6d89d0-9c7f-40b8-81d4-96dcfc8940c3
📒 Files selected for processing (2)
shortcuts/doc/doc_media_insert.goshortcuts/doc/doc_media_insert_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- shortcuts/doc/doc_media_insert.go
…cute Dimension detection runs after the placeholder block is created (Step 2), so failures must clean up the block to avoid leaving an empty placeholder in the document.
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (41.48%) 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 #832 +/- ##
==========================================
+ Coverage 65.40% 65.90% +0.50%
==========================================
Files 508 523 +15
Lines 46795 49679 +2884
==========================================
+ Hits 30605 32742 +2137
- Misses 13548 14136 +588
- Partials 2642 2801 +159 ☔ 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@f9bc6067d921279ef0754e1c1843f5476d423437🧩 Skill updatenpx skills add songyoung77/cli#feat/doc-media-insert-width-height -y -g |
3 participants
Summary
Add
--widthand--heightflags todocs +media-insertso users and AI agents can control the display size of inserted images. When only one dimension is specified, the other is auto-calculated from the image's native aspect ratio.Changes
--widthand--heightint flags with validation inshortcuts/doc/doc_media_insert.gocomputeMissingDimension()anddetectImageDimensions()aspect-ratio helpers inshortcuts/doc/doc_media_insert.goshortcuts/doc/doc_media_insert.goshortcuts/doc/doc_media_insert_test.goskills/lark-doc/references/lark-doc-media-insert.mdTest Plan
make unit-testpassedmake validatepassedspace:document:deletescope for cleanup; skillave 6/6)lark-cli docs +media-insert --doc <token> --file img.png --width 600 --as bot— confirmed aspect-ratio auto-calc and output fieldsRelated Issues
Closes #489
Summary by CodeRabbit
New Features
Bug Fixes / Validation
Documentation
Tests