From 752bfcbbb952b7980d0ee821abe3213d08ce8d17 Mon Sep 17 00:00:00 2001 From: zhengzhijie Date: Thu, 11 Jun 2026 16:45:28 +0800 Subject: [PATCH] feat(sheets): make --target-position and --range mutually exclusive on +pivot-create Both flags map to the same wire field (properties.range), so passing non-default values for both is ambiguous. Mirror the --target-sheet-id / --target-sheet-name mutex pattern: --target-position takes priority over --range, and supplying both with non-default values is rejected up front with a typed FlagErrorf. --target-position=A1 is the documented default and is treated as "not set". Add a symmetric validateCreateInput hook on objectCRUDSpec (alongside the existing validateUpdateInput), wire it into objectCreateInput, and inject the pivot-specific check on pivotSpec. --- shortcuts/sheets/lark_sheet_object_crud.go | 32 ++++++++-- .../sheets/lark_sheet_object_crud_test.go | 62 ++++++++++++++++--- 2 files changed, 83 insertions(+), 11 deletions(-) diff --git a/shortcuts/sheets/lark_sheet_object_crud.go b/shortcuts/sheets/lark_sheet_object_crud.go index 221b9d03d..c87e7cc79 100644 --- a/shortcuts/sheets/lark_sheet_object_crud.go +++ b/shortcuts/sheets/lark_sheet_object_crud.go @@ -49,6 +49,12 @@ type objectCRUDSpec struct { // right nesting level. enhanceCreateInput func(rt flagView, input map[string]interface{}) enhanceUpdateInput func(rt flagView, input map[string]interface{}) + // validateCreateInput, when set, runs after enhanceCreateInput to + // enforce *cross-flag, create-only* constraints JSON Schema can't + // express (e.g. pivot rejects --target-position vs --range when + // both carry non-default values — they map to the same wire field + // and conflicting values are ambiguous). Mirrors validateUpdateInput. + validateCreateInput func(rt flagView, input map[string]interface{}) error // validateUpdateInput, when set, runs after enhanceUpdateInput to // enforce *cross-field, update-only* constraints JSON Schema can't // express (e.g. sparkline requires properties.sparklines[i] to @@ -190,6 +196,11 @@ func objectCreateInput(runtime flagView, token, sheetID, sheetName string, spec if spec.enhanceCreateInput != nil { spec.enhanceCreateInput(runtime, input) } + if spec.validateCreateInput != nil { + if err := spec.validateCreateInput(runtime, input); err != nil { + return nil, err + } + } if err := validateInputAgainstSchema(runtime, input); err != nil { return nil, err } @@ -381,9 +392,6 @@ var pivotSpec = objectCRUDSpec{ }, createWarn: pivotPlacementWarn, enhanceCreateInput: func(rt flagView, input map[string]interface{}) { - if v := strings.TrimSpace(rt.Str("target-position")); v != "" && v != "A1" { - input["target_position"] = v - } props, _ := input["properties"].(map[string]interface{}) if props == nil { return @@ -391,10 +399,26 @@ var pivotSpec = objectCRUDSpec{ if v := strings.TrimSpace(rt.Str("source")); v != "" { props["source"] = v } - if v := strings.TrimSpace(rt.Str("range")); v != "" { + // --target-position 与 --range 都映射到 properties.range； + // --target-position 优先，未给（或为默认值 A1）时回落到 --range。 + // 互斥校验在 validateCreateInput 里做。 + if v := strings.TrimSpace(rt.Str("target-position")); v != "" && v != "A1" { + props["range"] = v + } else if v := strings.TrimSpace(rt.Str("range")); v != "" { props["range"] = v } }, + // --target-position 与 --range 落到同一 wire 字段（properties.range）， + // 同时给非默认值时无法判断意图——按 --target-sheet-id / --target-sheet-name + // 的处理方式，CLI 端直接拒绝（优于静默丢弃其一）。 + validateCreateInput: func(rt flagView, _ map[string]interface{}) error { + pos := strings.TrimSpace(rt.Str("target-position")) + rng := strings.TrimSpace(rt.Str("range")) + if pos != "" && pos != "A1" && rng != "" { + return common.FlagErrorf("--target-position and --range are mutually exclusive (both map to properties.range; pass only one)") + } + return nil + }, } var PivotCreate = newObjectCreateShortcut(pivotSpec) var PivotUpdate = newObjectUpdateShortcut(pivotSpec) diff --git a/shortcuts/sheets/lark_sheet_object_crud_test.go b/shortcuts/sheets/lark_sheet_object_crud_test.go index 728198298..9a651e4d0 100644 --- a/shortcuts/sheets/lark_sheet_object_crud_test.go +++ b/shortcuts/sheets/lark_sheet_object_crud_test.go @@ -137,25 +137,24 @@ func TestObjectCRUDShortcuts_DryRun(t *testing.T) { // covered separately in the +pivot-create empty-selector / mutex // tests below. { - name: "+pivot-create with placement / source / range flags", + name: "+pivot-create with placement / source / target-position flags", sc: PivotCreate, args: []string{ "--url", testURL, "--target-sheet-id", testSheetID, "--properties", `{"rows":[{"field":"A"}]}`, "--source", "Sheet1!A1:F1000", - "--range", "F1", "--target-position", "B5", }, toolName: "manage_pivot_table_object", wantInput: map[string]interface{}{ - "excel_id": testToken, - "sheet_id": testSheetID, - "operation": "create", - "target_position": "B5", + "excel_id": testToken, + "sheet_id": testSheetID, + "operation": "create", "properties": map[string]interface{}{ "rows": []interface{}{map[string]interface{}{"field": "A"}}, "source": "Sheet1!A1:F1000", - "range": "F1", + // --target-position 映射到 properties.range。 + "range": "B5", }, }, }, @@ -507,6 +506,55 @@ func TestPivotCreate_SheetSelectorSemantics(t *testing.T) { }) } +// TestPivotCreate_TargetPositionRangeMutex regresses the "--target-position +// and --range cannot both be set" guardrail on +pivot-create. They map to +// the same wire field (properties.range), so two non-default values are +// ambiguous; the CLI rejects up front (mirrors the --target-sheet-id / +// --target-sheet-name mutex). --target-position=A1 is the documented default +// and is treated as "not set" — pairing it with --range still works. +func TestPivotCreate_TargetPositionRangeMutex(t *testing.T) { + t.Parallel() + + t.Run("both non-default values rejected", func(t *testing.T) { + t.Parallel() + _, stderr, err := runShortcutCapturingErr(t, PivotCreate, []string{ + "--url", testURL, + "--target-sheet-id", testSheetID, + "--properties", `{"rows":[{"field":"A"}]}`, + "--source", "Sheet1!A1:F1000", + "--target-position", "B5", + "--range", "F1", + }) + if err == nil { + t.Fatalf("expected CLI to reject --target-position with --range; stderr=%s", stderr) + } + combined := stderr + err.Error() + if !strings.Contains(combined, "mutually exclusive") { + t.Errorf("expected error to say 'mutually exclusive'; got=%s|%v", stderr, err) + } + if !strings.Contains(combined, "--target-position") || !strings.Contains(combined, "--range") { + t.Errorf("expected error to quote both --target-position and --range; got=%s|%v", stderr, err) + } + }) + + t.Run("default A1 with --range is accepted (range wins)", func(t *testing.T) { + t.Parallel() + body := parseDryRunBody(t, PivotCreate, []string{ + "--url", testURL, + "--target-sheet-id", testSheetID, + "--properties", `{"rows":[{"field":"A"}]}`, + "--source", "Sheet1!A1:F1000", + "--target-position", "A1", + "--range", "F1", + }) + input := decodeToolInput(t, body, "manage_pivot_table_object") + props, _ := input["properties"].(map[string]interface{}) + if got, _ := props["range"].(string); got != "F1" { + t.Errorf("properties.range = %q, want %q", got, "F1") + } + }) +} + // TestPivotCreate_SchemaValidates exercises the schema-driven // validator wired into objectCreateInput. The pivot create schema // doesn't constrain rows/columns/values to be present (the backend