feat(doc): add --width/--height flags to docs +media-insert (#832)

* feat(doc): add width/height params to buildBatchUpdateData

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.

* feat(doc): add --width/--height flags with validation to docs +media-insert

* feat(doc): add aspect-ratio auto-calculation helpers

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.

* feat(doc): wire --width/--height into Execute with aspect-ratio calculation

* feat(doc): add best-effort dimension computation to DryRun

* docs: add --width/--height to docs +media-insert SKILL.md

* fix: add SafeInputPath validation to detectImageDimensionsFromPath

* fix: guard computeMissingDimension against division by zero and add rounding

* fix: add dimension upper bound, fix err variable reuse in Execute

* refactor: use early-return guard for zero native dimensions per review

* fix: add pixels unit to dimension validation error messages

* fix: surface dimension detection failures in dry-run to match Execute behavior

* fix: move dimension detection before upload to fail fast

* fix: restore withRollbackWarning on dimension detection errors in Execute

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.
This commit is contained in:
songyoung77
2026-05-15 18:28:56 +08:00
committed by GitHub
parent 4a45e00139
commit 7400226e34
3 changed files with 393 additions and 11 deletions

View File

@@ -7,6 +7,11 @@ import (
"bytes"
"context"
"fmt"
"image"
_ "image/gif"
_ "image/jpeg"
_ "image/png"
"io"
"path/filepath"
"strings"
@@ -55,6 +60,8 @@ var DocMediaInsert = common.Shortcut{
{Name: "selection-with-ellipsis", Desc: "plain text (or 'start...end' to disambiguate) matching the target block's content. Media is inserted at the top-level ancestor of the matched block — i.e., when the selection is inside a callout, table cell, or nested list, media lands outside that container, not inside it. Pass 'start...end' (a unique prefix and suffix separated by '...') when the plain text appears in more than one block"},
{Name: "before", Type: "bool", Desc: "insert before the matched block instead of after (requires --selection-with-ellipsis)"},
{Name: "file-view", Desc: "file block rendering: card (default) | preview | inline; only applies when --type=file. preview renders audio/video as an inline player"},
{Name: "width", Type: "int", Desc: "image display width in pixels (only for --type=image); if --height is omitted it is auto-computed from the source image aspect ratio"},
{Name: "height", Type: "int", Desc: "image display height in pixels (only for --type=image); if --width is omitted it is auto-computed from the source image aspect ratio"},
},
Validate: func(ctx context.Context, runtime *common.RuntimeContext) error {
filePath := runtime.Str("file")
@@ -93,6 +100,24 @@ var DocMediaInsert = common.Shortcut{
return output.ErrValidation("--file-view only applies when --type=file")
}
}
widthChanged := runtime.Changed("width")
heightChanged := runtime.Changed("height")
if (widthChanged || heightChanged) && runtime.Str("type") != "image" {
return output.ErrValidation("--width/--height only apply when --type=image")
}
if widthChanged && runtime.Int("width") <= 0 {
return output.ErrValidation("--width must be a positive integer")
}
if heightChanged && runtime.Int("height") <= 0 {
return output.ErrValidation("--height must be a positive integer")
}
const maxDimension = 10000
if widthChanged && runtime.Int("width") > maxDimension {
return output.ErrValidation("--width must not exceed %d pixels", maxDimension)
}
if heightChanged && runtime.Int("height") > maxDimension {
return output.ErrValidation("--height must not exceed %d pixels", maxDimension)
}
return nil
},
DryRun: func(ctx context.Context, runtime *common.RuntimeContext) *common.DryRunAPI {
@@ -120,7 +145,25 @@ var DocMediaInsert = common.Shortcut{
} else {
createBlockData["index"] = "<children_len>"
}
batchUpdateData := buildBatchUpdateData("<new_block_id>", mediaType, "<file_token>", runtime.Str("align"), caption)
// Best-effort dimension computation for dry-run.
dryWidth := runtime.Int("width")
dryHeight := runtime.Int("height")
widthChanged := runtime.Changed("width")
heightChanged := runtime.Changed("height")
if (widthChanged || heightChanged) && !(widthChanged && heightChanged) {
if filePath == "<clipboard image>" {
fmt.Fprintf(runtime.IO().ErrOut, "Note: cannot detect clipboard image dimensions in dry-run; provide both --width and --height for accurate preview\n")
} else if nativeW, nativeH, err := detectImageDimensionsFromPath(runtime.FileIO(), filePath); err == nil {
dims := computeMissingDimension(dryWidth, dryHeight, nativeW, nativeH)
dryWidth = dims.width
dryHeight = dims.height
} else {
fmt.Fprintf(runtime.IO().ErrOut, "Note: unable to detect image dimensions from %s; provide both --width and --height to avoid failure at execution time\n", filePath)
}
}
batchUpdateData := buildBatchUpdateData("<new_block_id>", mediaType, "<file_token>", runtime.Str("align"), caption, dryWidth, dryHeight)
d := common.NewDryRunAPI()
totalSteps := 4
@@ -188,6 +231,9 @@ var DocMediaInsert = common.Shortcut{
if runtime.Bool("from-clipboard") {
d.Set("upload_size_note", "clipboard size unknown; single-part vs multipart decision deferred to runtime")
}
if runtime.Bool("from-clipboard") && (widthChanged || heightChanged) && !(widthChanged && heightChanged) {
d.Set("dimension_note", "clipboard dimensions unknown; aspect-ratio calculation deferred to runtime")
}
return d
},
Execute: func(ctx context.Context, runtime *common.RuntimeContext) error {
@@ -314,6 +360,42 @@ var DocMediaInsert = common.Shortcut{
// interface stays a true nil for the --file path. Passing a typed-nil
// *bytes.Reader here would make the downstream `if cfg.Content != nil`
// check incorrectly take the clipboard branch and crash on Read.
// Resolve display dimensions before upload to fail fast on unreadable images.
var finalWidth, finalHeight int
if mediaType == "image" {
userWidth := runtime.Int("width")
userHeight := runtime.Int("height")
widthChanged := runtime.Changed("width")
heightChanged := runtime.Changed("height")
if widthChanged && heightChanged {
finalWidth = userWidth
finalHeight = userHeight
} else if widthChanged || heightChanged {
var nativeW, nativeH int
var dimErr error
if clipboardContent != nil {
nativeW, nativeH, dimErr = detectImageDimensions(bytes.NewReader(clipboardContent))
} else {
f, openErr := runtime.FileIO().Open(filePath)
if openErr != nil {
return withRollbackWarning(output.ErrValidation(
"unable to detect image dimensions from %s for aspect-ratio calculation; provide both --width and --height", fileName))
}
nativeW, nativeH, dimErr = detectImageDimensions(f)
f.Close()
}
if dimErr != nil {
return withRollbackWarning(output.ErrValidation(
"unable to detect image dimensions from %s for aspect-ratio calculation; provide both --width and --height", fileName))
}
dims := computeMissingDimension(userWidth, userHeight, nativeW, nativeH)
finalWidth = dims.width
finalHeight = dims.height
fmt.Fprintf(runtime.IO().ErrOut, "Image dimensions: %dx%d (native: %dx%d)\n", finalWidth, finalHeight, nativeW, nativeH)
}
}
uploadCfg := UploadDocMediaFileConfig{
FilePath: filePath,
FileName: fileName,
@@ -337,16 +419,23 @@ var DocMediaInsert = common.Shortcut{
if _, err := runtime.CallAPI("PATCH",
fmt.Sprintf("/open-apis/docx/v1/documents/%s/blocks/batch_update", validate.EncodePathSegment(documentID)),
nil, buildBatchUpdateData(replaceBlockID, mediaType, fileToken, alignStr, caption)); err != nil {
nil, buildBatchUpdateData(replaceBlockID, mediaType, fileToken, alignStr, caption, finalWidth, finalHeight)); err != nil {
return withRollbackWarning(err)
}
runtime.Out(map[string]interface{}{
outData := map[string]interface{}{
"document_id": documentID,
"block_id": blockId,
"file_token": fileToken,
"type": mediaType,
}, nil)
}
if finalWidth > 0 {
outData["width"] = finalWidth
}
if finalHeight > 0 {
outData["height"] = finalHeight
}
runtime.Out(outData, nil)
return nil
},
}
@@ -453,7 +542,51 @@ func resolveDocxDocumentID(runtime *common.RuntimeContext, input string) (string
}
}
func buildBatchUpdateData(blockID, mediaType, fileToken, alignStr, caption string) map[string]interface{} {
type imageDimensions struct {
width int
height int
}
func computeMissingDimension(userWidth, userHeight, nativeWidth, nativeHeight int) imageDimensions {
if nativeWidth <= 0 || nativeHeight <= 0 {
return imageDimensions{width: userWidth, height: userHeight}
}
if userWidth > 0 && userHeight == 0 {
return imageDimensions{
width: userWidth,
height: (userWidth*nativeHeight + nativeWidth/2) / nativeWidth,
}
}
if userHeight > 0 && userWidth == 0 {
return imageDimensions{
width: (userHeight*nativeWidth + nativeHeight/2) / nativeHeight,
height: userHeight,
}
}
return imageDimensions{width: userWidth, height: userHeight}
}
func detectImageDimensions(r io.Reader) (width, height int, err error) {
cfg, _, err := image.DecodeConfig(r)
if err != nil {
return 0, 0, err
}
return cfg.Width, cfg.Height, nil
}
func detectImageDimensionsFromPath(fio fileio.FileIO, filePath string) (int, int, error) {
if _, err := validate.SafeInputPath(filePath); err != nil {
return 0, 0, err
}
f, err := fio.Open(filePath)
if err != nil {
return 0, 0, err
}
defer f.Close()
return detectImageDimensions(f)
}
func buildBatchUpdateData(blockID, mediaType, fileToken, alignStr, caption string, width, height int) map[string]interface{} {
request := map[string]interface{}{
"block_id": blockID,
}
@@ -465,6 +598,12 @@ func buildBatchUpdateData(blockID, mediaType, fileToken, alignStr, caption strin
replaceImage := map[string]interface{}{
"token": fileToken,
}
if width > 0 {
replaceImage["width"] = width
}
if height > 0 {
replaceImage["height"] = height
}
if alignVal, ok := alignMap[alignStr]; ok {
replaceImage["align"] = alignVal
}

View File

@@ -6,6 +6,7 @@ package doc
import (
"context"
"encoding/json"
"fmt"
"reflect"
"strings"
"testing"
@@ -176,7 +177,7 @@ func TestBuildDeleteBlockDataUsesHalfOpenInterval(t *testing.T) {
func TestBuildBatchUpdateDataForImage(t *testing.T) {
t.Parallel()
got := buildBatchUpdateData("blk_1", "image", "file_tok", "center", "caption text")
got := buildBatchUpdateData("blk_1", "image", "file_tok", "center", "caption text", 0, 0)
want := map[string]interface{}{
"requests": []interface{}{
map[string]interface{}{
@@ -199,7 +200,7 @@ func TestBuildBatchUpdateDataForImage(t *testing.T) {
func TestBuildBatchUpdateDataForFile(t *testing.T) {
t.Parallel()
got := buildBatchUpdateData("blk_2", "file", "file_tok", "", "")
got := buildBatchUpdateData("blk_2", "file", "file_tok", "", "", 0, 0)
want := map[string]interface{}{
"requests": []interface{}{
map[string]interface{}{
@@ -215,6 +216,48 @@ func TestBuildBatchUpdateDataForFile(t *testing.T) {
}
}
func TestBuildBatchUpdateDataForImageWithWidthHeight(t *testing.T) {
t.Parallel()
got := buildBatchUpdateData("blk_1", "image", "file_tok", "center", "caption text", 800, 447)
want := map[string]interface{}{
"requests": []interface{}{
map[string]interface{}{
"block_id": "blk_1",
"replace_image": map[string]interface{}{
"token": "file_tok",
"width": 800,
"height": 447,
"align": 2,
"caption": map[string]interface{}{"content": "caption text"},
},
},
},
}
if !reflect.DeepEqual(got, want) {
t.Fatalf("buildBatchUpdateData(image, 800, 447) = %#v, want %#v", got, want)
}
}
func TestBuildBatchUpdateDataForFileIgnoresWidthHeight(t *testing.T) {
t.Parallel()
got := buildBatchUpdateData("blk_2", "file", "file_tok", "", "", 800, 600)
want := map[string]interface{}{
"requests": []interface{}{
map[string]interface{}{
"block_id": "blk_2",
"replace_file": map[string]interface{}{
"token": "file_tok",
},
},
},
}
if !reflect.DeepEqual(got, want) {
t.Fatalf("buildBatchUpdateData(file, 800, 600) = %#v, want %#v", got, want)
}
}
func TestExtractAppendTargetUsesRootChildrenCount(t *testing.T) {
t.Parallel()
@@ -669,10 +712,202 @@ func newMediaInsertValidateRuntime(t *testing.T, doc, mediaType, fileView string
return common.TestNewRuntimeContext(cmd, nil)
}
// Validate is the real user-facing contract for --file-view: unknown
// values must be rejected, and passing the flag alongside --type!=file
// must also be rejected. buildCreateBlockData tests alone cannot catch
// regressions here, so lock the guard logic down explicitly.
func newMediaInsertValidateRuntimeWithSize(t *testing.T, doc, mediaType string, width, height int, setWidth, setHeight bool) *common.RuntimeContext {
t.Helper()
cmd := &cobra.Command{Use: "docs +media-insert"}
cmd.Flags().String("file", "", "")
cmd.Flags().Bool("from-clipboard", false, "")
cmd.Flags().String("doc", "", "")
cmd.Flags().String("type", "", "")
cmd.Flags().String("file-view", "", "")
cmd.Flags().Int("width", 0, "")
cmd.Flags().Int("height", 0, "")
cmd.Flags().String("selection-with-ellipsis", "", "")
cmd.Flags().Bool("before", false, "")
if err := cmd.Flags().Set("file", "dummy.bin"); err != nil {
t.Fatalf("set --file: %v", err)
}
if err := cmd.Flags().Set("doc", doc); err != nil {
t.Fatalf("set --doc: %v", err)
}
if err := cmd.Flags().Set("type", mediaType); err != nil {
t.Fatalf("set --type: %v", err)
}
if setWidth {
if err := cmd.Flags().Set("width", fmt.Sprintf("%d", width)); err != nil {
t.Fatalf("set --width: %v", err)
}
}
if setHeight {
if err := cmd.Flags().Set("height", fmt.Sprintf("%d", height)); err != nil {
t.Fatalf("set --height: %v", err)
}
}
return common.TestNewRuntimeContext(cmd, nil)
}
func TestDocMediaInsertValidateWidthHeightOnlyForImage(t *testing.T) {
t.Parallel()
tests := []struct {
name string
mediaType string
width int
height int
setWidth bool
setHeight bool
wantErr string
}{
{
name: "width with file type is rejected",
mediaType: "file",
width: 800,
setWidth: true,
wantErr: "--width/--height only apply when --type=image",
},
{
name: "height with file type is rejected",
mediaType: "file",
height: 600,
setHeight: true,
wantErr: "--width/--height only apply when --type=image",
},
{
name: "explicit zero width is rejected",
mediaType: "image",
width: 0,
setWidth: true,
wantErr: "--width must be a positive integer",
},
{
name: "negative width is rejected",
mediaType: "image",
width: -1,
setWidth: true,
wantErr: "--width must be a positive integer",
},
{
name: "negative height is rejected",
mediaType: "image",
height: -5,
setHeight: true,
wantErr: "--height must be a positive integer",
},
{
name: "valid width with image type is accepted",
mediaType: "image",
width: 800,
setWidth: true,
},
{
name: "valid width and height with image type is accepted",
mediaType: "image",
width: 800,
height: 600,
setWidth: true,
setHeight: true,
},
}
for _, ttTemp := range tests {
tt := ttTemp
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
rt := newMediaInsertValidateRuntimeWithSize(t, "doxcnValidateSize", tt.mediaType, tt.width, tt.height, tt.setWidth, tt.setHeight)
err := DocMediaInsert.Validate(context.Background(), rt)
if tt.wantErr == "" {
if err != nil {
t.Fatalf("Validate() unexpected error: %v", err)
}
return
}
if err == nil {
t.Fatalf("Validate() error = nil, want error containing %q", tt.wantErr)
}
if !strings.Contains(err.Error(), tt.wantErr) {
t.Fatalf("Validate() error = %q, want substring %q", err.Error(), tt.wantErr)
}
})
}
}
func TestDocMediaInsertValidateNoWidthHeightIsValid(t *testing.T) {
t.Parallel()
rt := newMediaInsertValidateRuntimeWithSize(t, "doxcnNoSize", "image", 0, 0, false, false)
err := DocMediaInsert.Validate(context.Background(), rt)
if err != nil {
t.Fatalf("Validate() unexpected error when neither --width nor --height passed: %v", err)
}
}
func TestAutoAspectRatioFromWidth(t *testing.T) {
t.Parallel()
// Native image: 1200x800 (3:2 ratio)
// User provides width=600 → expected height = 600 * 800 / 1200 = 400
got := computeMissingDimension(600, 0, 1200, 800)
wantWidth, wantHeight := 600, 400
if got.width != wantWidth || got.height != wantHeight {
t.Fatalf("computeMissingDimension(600, 0, 1200, 800) = (%d, %d), want (%d, %d)", got.width, got.height, wantWidth, wantHeight)
}
}
func TestAutoAspectRatioFromHeight(t *testing.T) {
t.Parallel()
// Native image: 1200x800 (3:2 ratio)
// User provides height=400 → expected width = 400 * 1200 / 800 = 600
got := computeMissingDimension(0, 400, 1200, 800)
wantWidth, wantHeight := 600, 400
if got.width != wantWidth || got.height != wantHeight {
t.Fatalf("computeMissingDimension(0, 400, 1200, 800) = (%d, %d), want (%d, %d)", got.width, got.height, wantWidth, wantHeight)
}
}
func TestComputeMissingDimensionBothProvided(t *testing.T) {
t.Parallel()
got := computeMissingDimension(800, 600, 1200, 900)
if got.width != 800 || got.height != 600 {
t.Fatalf("computeMissingDimension(800, 600, 1200, 900) = (%d, %d), want (800, 600)", got.width, got.height)
}
}
func TestComputeMissingDimensionNeitherProvided(t *testing.T) {
t.Parallel()
got := computeMissingDimension(0, 0, 1200, 900)
if got.width != 0 || got.height != 0 {
t.Fatalf("computeMissingDimension(0, 0, 1200, 900) = (%d, %d), want (0, 0)", got.width, got.height)
}
}
func TestComputeMissingDimensionZeroNativeWidth(t *testing.T) {
t.Parallel()
got := computeMissingDimension(600, 0, 0, 800)
if got.width != 600 || got.height != 0 {
t.Fatalf("computeMissingDimension(600, 0, 0, 800) = (%d, %d), want (600, 0)", got.width, got.height)
}
}
func TestComputeMissingDimensionZeroNativeHeight(t *testing.T) {
t.Parallel()
got := computeMissingDimension(0, 400, 1200, 0)
if got.width != 0 || got.height != 400 {
t.Fatalf("computeMissingDimension(0, 400, 1200, 0) = (%d, %d), want (0, 400)", got.width, got.height)
}
}
func TestComputeMissingDimensionRounding(t *testing.T) {
t.Parallel()
got := computeMissingDimension(999, 0, 1000, 333)
want := (999*333 + 500) / 1000
if got.height != want {
t.Fatalf("computeMissingDimension(999, 0, 1000, 333).height = %d, want %d (rounded)", got.height, want)
}
}
func TestDocMediaInsertValidateFileView(t *testing.T) {
t.Parallel()

View File

@@ -67,6 +67,12 @@ lark-cli docs +media-insert --doc doxcnXXX --file ./spec.pdf --type file
# 图片对齐与描述caption
lark-cli docs +media-insert --doc doxcnXXX --from-clipboard --align center --caption "架构图"
# Insert image with explicit display width (height auto-computed from aspect ratio)
lark-cli docs +media-insert --doc doxcnXXX --file ./banner.png --width 800 --align center
# Insert image with explicit width and height
lark-cli docs +media-insert --doc doxcnXXX --from-clipboard --width 800 --height 447 --caption "architecture diagram"
```
## 参数
@@ -79,6 +85,8 @@ lark-cli docs +media-insert --doc doxcnXXX --from-clipboard --align center --cap
| `--type <type>` | 否 | `image`(默认)或 `file``--from-clipboard` 目前只产出 image。 |
| `--align <align>` | 否 | 仅图片:`left` / `center`(默认)/ `right` |
| `--caption <text>` | 否 | 仅图片:图片描述 |
| `--width <px>` | 否 | Image display width in pixels (only for `--type=image`). If `--height` is omitted, it is auto-computed from the source image aspect ratio. Supported auto-detection formats: PNG, JPEG, GIF; other formats (WebP, BMP, etc.) require both `--width` and `--height`. |
| `--height <px>` | 否 | Image display height in pixels (only for `--type=image`). If `--width` is omitted, it is auto-computed from the source image aspect ratio. Supported auto-detection formats: PNG, JPEG, GIF; other formats (WebP, BMP, etc.) require both `--width` and `--height`. |
> [!IMPORTANT]
> 如果上一步是 [`lark-doc-create`](lark-doc-create.md),并且它在知识库/知识空间场景下返回的是 `/wiki/...` 形式的 `doc_url`,后续调用 `docs +media-insert` 时应优先传 `doc_id`,不要直接传这个 `doc_url`。