mirror of
https://github.com/larksuite/cli.git
synced 2026-08-03 08:32:46 +08:00
fix(sheets): close review gaps in freeze, styles and error reporting
Review of the aggregate diff turned up nine places where the surface did not do what this PR says it does. Each is small; the theme they share is that a prescription pointed somewhere the caller could not follow. Contradictions with this PR's own retirements: - The six --frozen-* unknown-flag hints prescribed --dimension row --count N. Those flags are hidden from --help, so the hint named a flag missing from the valid-flags list printed beside it, and following it earned a deprecation note steering back. They now prescribe --rows / --cols, as does the "nothing to freeze" error. - A Sheet! range prefix was only stripped when the styles item carried a name. +workbook-create --values items need none, so row_sizes like "Sheet1!2:3" still failed there as a malformed range — the exact bug this PR reports as fixed across all three --styles carriers. Stripping is now unconditional; only the "names a different sheet" report needs a name to compare against. - Two +dim-freeze sub-ops in one batch cancel each other, and only the CLI can see it (a batch cannot read current state, and +styles-put is not batchable). Per-op "equivalent to --rows 1" notes never said so. A collision note now names the colliding ops, the state actually reached, and the single sub-op that holds both axes. dimFreezeAxes/dimFreezeSpelling became the shared mapping so the request body, the deprecation note and this one cannot drift. - +dim-insert with --inherit-style omitted sent no `side`, so "omitting is the same as after" held only if the backend happened to default it to before — and if it defaulted to after, the insert would land on the wrong side of --position, silently breaking the command's stated contract. It is now sent explicitly; TestDimInsertOmittedMatchesAfter pins the two bodies together. Errors that misdescribed themselves: - "resend only operations[N:]" was emitted for every batch_update caller, but only +batch-update's array is caller-written. +styles-put coalesces and +dim-delete --ranges deliberately re-sorts descending, so the index names nothing the caller can locate. Those callers now get a read-back procedure. - A sub-op carrying both an alias and its target (size + width on +cols-resize) was reported as an unknown input key: the alias branch fell through, and the conflict check never fired because keys are walked in sorted order and "size" sorts first. Identical values now drop the alias; differing ones name both spellings. - Folding per-item failures into one error dropped each inner Hint, so the more mistakes a payload had, the less guidance it got — including the +workbook-info pointer this PR had just added. A lone issue inherits the hint; a folded list inlines each. - --max-chars 0 sent no cap, which makes the read tool apply its own ~50000 fallback: asking for no limit produced the smallest one. It now resolves to the same ceiling as leaving the flag alone. Also: --inherit-style before anchors one row/column earlier, so its dry-run showed a position the caller never typed; a note explains it is not an off-by-one. The style vocabulary now walks sorted keys everywhere, since every one of those loops can abort and map order decided which of several bad fields got reported.
This commit is contained in:
@@ -5,7 +5,6 @@ package localfileio
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"unicode"
|
||||
|
||||
@@ -275,7 +275,7 @@ func TestFlattenToolErrorMsg_PartialFailureRecovery(t *testing.T) {
|
||||
|
||||
t.Run("single failure prescribes resend-from-index", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
msg := flattenToolErrorMsg(wrap(`{"message":"batch_update: 4 succeeded, 1 failed","failures":[{"index":4,"tool_name":"set_cell_range","error":"cells is required"}]}`), false)
|
||||
msg := flattenToolErrorMsg(wrap(`{"message":"batch_update: 4 succeeded, 1 failed","failures":[{"index":4,"tool_name":"set_cell_range","error":"cells is required"}]}`), false, true)
|
||||
for _, want := range []string{"operations[4] (set_cell_range)", "no rollback", "resend only operations[4:]"} {
|
||||
if !strings.Contains(msg, want) {
|
||||
t.Fatalf("msg %q missing %q", msg, want)
|
||||
@@ -285,7 +285,7 @@ func TestFlattenToolErrorMsg_PartialFailureRecovery(t *testing.T) {
|
||||
|
||||
t.Run("multiple failures prescribe failed-only resend", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
msg := flattenToolErrorMsg(wrap(`{"message":"batch_update: 3 succeeded, 2 failed","failures":[{"index":1,"tool_name":"set_cell_range","error":"e1"},{"index":3,"tool_name":"resize_range","error":"e2"}]}`), false)
|
||||
msg := flattenToolErrorMsg(wrap(`{"message":"batch_update: 3 succeeded, 2 failed","failures":[{"index":1,"tool_name":"set_cell_range","error":"e1"},{"index":3,"tool_name":"resize_range","error":"e2"}]}`), false, true)
|
||||
if !strings.Contains(msg, "resend only the failed operations") {
|
||||
t.Fatalf("msg %q missing failed-only prescription", msg)
|
||||
}
|
||||
@@ -293,7 +293,7 @@ func TestFlattenToolErrorMsg_PartialFailureRecovery(t *testing.T) {
|
||||
|
||||
t.Run("zero succeeded gets no note", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
msg := flattenToolErrorMsg(wrap(`{"message":"batch_update: 0 succeeded, 1 failed","failures":[{"index":0,"tool_name":"set_cell_range","error":"e"}]}`), false)
|
||||
msg := flattenToolErrorMsg(wrap(`{"message":"batch_update: 0 succeeded, 1 failed","failures":[{"index":0,"tool_name":"set_cell_range","error":"e"}]}`), false, true)
|
||||
if strings.Contains(msg, "no rollback") {
|
||||
t.Fatalf("msg %q must not carry the note when nothing was applied", msg)
|
||||
}
|
||||
|
||||
@@ -446,13 +446,33 @@ func normalizeSubOpInputKeys(sc string, input map[string]interface{}) error {
|
||||
if err := claim(target, k); err != nil {
|
||||
return err
|
||||
}
|
||||
if _, taken := input[target]; !taken {
|
||||
if _, taken := input[strings.ReplaceAll(target, "-", "_")]; !taken {
|
||||
input[target] = input[k]
|
||||
delete(input, k)
|
||||
continue
|
||||
}
|
||||
underscored := strings.ReplaceAll(target, "-", "_")
|
||||
_, hyphenTaken := input[target]
|
||||
_, underscoreTaken := input[underscored]
|
||||
if !hyphenTaken && !underscoreTaken {
|
||||
input[target] = input[k]
|
||||
delete(input, k)
|
||||
continue
|
||||
}
|
||||
// The alias AND its target are both present. This key is recognized,
|
||||
// so it must not fall through to the generic "unknown input key"
|
||||
// below — the claim() conflict message never fires here either,
|
||||
// because keys are walked in sorted order and the alias can sort
|
||||
// before its target ("size" < "width"), so nothing has claimed the
|
||||
// logical key yet. Name both spellings and the survivor.
|
||||
taken := target
|
||||
if underscoreTaken {
|
||||
taken = underscored
|
||||
}
|
||||
if jsonEqual(input[k], input[taken]) {
|
||||
delete(input, k) // same value under two names: drop the alias.
|
||||
// Hand the logical key over to the surviving spelling, or the
|
||||
// claim recorded above would still point at the deleted alias
|
||||
// and make that spelling's own turn read as a conflict.
|
||||
canonical[target] = taken
|
||||
continue
|
||||
}
|
||||
return fmt.Errorf("%s got both %q and %q, which are two names for the same flag, with different values — keep %q", sc, k, taken, taken) //nolint:forbidigo // intermediate error; the batch dispatcher wraps it into a typed operations validation error
|
||||
}
|
||||
if strings.ToLower(hv) == "ranges" && vocab["range"] && !vocab["ranges"] {
|
||||
if _, taken := input["range"]; taken {
|
||||
@@ -678,7 +698,11 @@ func translateBatchOperations(rawOps []interface{}, token string) ([]interface{}
|
||||
}
|
||||
parts := make([]string, 0, len(shown))
|
||||
for i, e := range shown {
|
||||
parts = append(parts, fmt.Sprintf("%d) %s", i+1, e.Error()))
|
||||
// aggregatedIssueText keeps each op's own hint (the "<shortcut> input
|
||||
// keys: …" contract) inline: folding N errors leaves one Hint slot, so
|
||||
// without this the multi-op error would carry LESS guidance than the
|
||||
// single-op one it replaces.
|
||||
parts = append(parts, fmt.Sprintf("%d) %s", i+1, aggregatedIssueText(e)))
|
||||
}
|
||||
msg := fmt.Sprintf("%d of %d operations failed validation: %s", len(opErrs), len(rawOps), strings.Join(parts, "; "))
|
||||
if truncated {
|
||||
|
||||
@@ -91,13 +91,17 @@ var intuitiveFlagHints = map[string]map[string]string{
|
||||
"+dim-insert": {
|
||||
"dimension": "+dim-insert infers rows vs columns from --position: a row number like 3 inserts rows, a column letter like C inserts columns; pair with --count N",
|
||||
},
|
||||
// Must prescribe --rows / --cols, never the retired --dimension/--count
|
||||
// pair (DEPRECATED(phase-2) on dimFreezeLegacyNote): those flags are hidden
|
||||
// from --help, so they do not even appear in the "valid flags" list printed
|
||||
// beside this hint, and using them earns a second note steering back here.
|
||||
"+dim-freeze": {
|
||||
"frozen-rows": "freeze the first N rows with --dimension row --count N",
|
||||
"frozen-cols": "freeze the first N columns with --dimension column --count N",
|
||||
"frozen-columns": "freeze the first N columns with --dimension column --count N",
|
||||
"frozen-row-count": "freeze the first N rows with --dimension row --count N",
|
||||
"frozen-col-count": "freeze the first N columns with --dimension column --count N",
|
||||
"frozen-column-count": "freeze the first N columns with --dimension column --count N",
|
||||
"frozen-rows": "freeze the first N rows with --rows N (add --cols M to hold columns too — one call states the whole freeze state)",
|
||||
"frozen-cols": "freeze the first N columns with --cols N (add --rows M to hold rows too — one call states the whole freeze state)",
|
||||
"frozen-columns": "freeze the first N columns with --cols N (add --rows M to hold rows too — one call states the whole freeze state)",
|
||||
"frozen-row-count": "freeze the first N rows with --rows N (add --cols M to hold columns too — one call states the whole freeze state)",
|
||||
"frozen-col-count": "freeze the first N columns with --cols N (add --rows M to hold rows too — one call states the whole freeze state)",
|
||||
"frozen-column-count": "freeze the first N columns with --cols N (add --rows M to hold rows too — one call states the whole freeze state)",
|
||||
},
|
||||
"+cells-set-style": {
|
||||
"bold": "use --font-weight bold",
|
||||
|
||||
@@ -501,6 +501,9 @@ func TestShortcuts_IntuitiveFlagHints(t *testing.T) {
|
||||
args []string
|
||||
wrong string
|
||||
wantHint []string
|
||||
// rejectHint pins what a prescription must NOT name — used where the
|
||||
// obvious wording would steer into a deprecated flag.
|
||||
rejectHint []string
|
||||
}{
|
||||
{
|
||||
command: "+dim-insert",
|
||||
@@ -512,7 +515,11 @@ func TestShortcuts_IntuitiveFlagHints(t *testing.T) {
|
||||
command: "+dim-freeze",
|
||||
args: []string{"--url", testURL, "--sheet-name", "s", "--frozen-rows", "2"},
|
||||
wrong: "--frozen-rows",
|
||||
wantHint: []string{"--dimension row --count N"},
|
||||
// Must prescribe the CURRENT spelling: --dimension/--count is
|
||||
// retired and hidden from --help, so a hint naming it would point at
|
||||
// a flag missing from the same error's valid-flags list.
|
||||
wantHint: []string{"--rows N"},
|
||||
rejectHint: []string{"--dimension", "--count"},
|
||||
},
|
||||
{
|
||||
command: "+cells-set-style",
|
||||
@@ -541,16 +548,18 @@ func TestShortcuts_IntuitiveFlagHints(t *testing.T) {
|
||||
{
|
||||
command: "+dim-freeze",
|
||||
args: []string{"--url", testURL, "--sheet-name", "s", "--frozen-row-count", "1"},
|
||||
wrong: "--frozen-row-count",
|
||||
wantHint: []string{"--dimension row --count N"},
|
||||
wrong: "--frozen-row-count",
|
||||
wantHint: []string{"--rows N"},
|
||||
rejectHint: []string{"--dimension", "--count"},
|
||||
},
|
||||
{
|
||||
// The parse error reports the flag as typed: the underscore
|
||||
// spelling must hit the same curated entry as the hyphenated one.
|
||||
command: "+dim-freeze",
|
||||
args: []string{"--url", testURL, "--sheet-name", "s", "--frozen_rows", "2"},
|
||||
wrong: "--frozen_rows",
|
||||
wantHint: []string{"--dimension row --count N"},
|
||||
wrong: "--frozen_rows",
|
||||
wantHint: []string{"--rows N"},
|
||||
rejectHint: []string{"--dimension", "--count"},
|
||||
},
|
||||
{
|
||||
command: "+cells-set-style",
|
||||
@@ -600,6 +609,14 @@ func TestShortcuts_IntuitiveFlagHints(t *testing.T) {
|
||||
t.Errorf("hint should contain %q, got %q", want, ve.Hint)
|
||||
}
|
||||
}
|
||||
// The valid-flags list is appended to the same Hint, so only the
|
||||
// prescription itself is checked for banned wording.
|
||||
prescription, _, _ := strings.Cut(ve.Hint, "; valid flags:")
|
||||
for _, banned := range tc.rejectHint {
|
||||
if strings.Contains(prescription, banned) {
|
||||
t.Errorf("prescription must not steer to %q, got %q", banned, prescription)
|
||||
}
|
||||
}
|
||||
// A curated prescription must not ship contradicting edit-distance
|
||||
// candidates (--font-bold used to carry --font-color/--font-line/
|
||||
// --font-size in params while the fix is --font-weight).
|
||||
|
||||
@@ -590,6 +590,51 @@ func requireJSONObject(runtime flagView, name string) (map[string]interface{}, e
|
||||
return m, nil
|
||||
}
|
||||
|
||||
// ─── aggregated sub-error rendering ────────────────────────────────────
|
||||
//
|
||||
// Several flags collect per-item failures and fold them into ONE typed error
|
||||
// (--styles, --writes, --operations). A Problem carries a single Hint slot,
|
||||
// so the naive fold — taking only each inner error's Message — silently drops
|
||||
// the very prescriptions this domain adds (requireSheetSelector's
|
||||
// "+workbook-info" pointer, the batch key contract). These two helpers keep
|
||||
// them: a lone failure hands its Hint to the outer error's Hint field, and a
|
||||
// folded list inlines each hint next to its own message.
|
||||
|
||||
// aggregatedIssueParts splits a collected sub-error into its message and its
|
||||
// hint ("" when it carries none), unwrapping the typed Problem so the message
|
||||
// is the bare text rather than the Error() rendering.
|
||||
func aggregatedIssueParts(err error) (msg, hint string) {
|
||||
if p, ok := errs.ProblemOf(err); ok {
|
||||
return p.Message, p.Hint
|
||||
}
|
||||
return err.Error(), ""
|
||||
}
|
||||
|
||||
// aggregatedIssueText renders one collected sub-error for a folded, multi-issue
|
||||
// message, appending its hint in parentheses so a per-item prescription is not
|
||||
// lost to the single shared Hint slot.
|
||||
func aggregatedIssueText(err error) string {
|
||||
msg, hint := aggregatedIssueParts(err)
|
||||
if hint == "" {
|
||||
return msg
|
||||
}
|
||||
return msg + " (" + hint + ")"
|
||||
}
|
||||
|
||||
// prefixValidationIssue re-labels a collected sub-error with the path it was
|
||||
// found at ("--writes[2]"), keeping its Hint. Formatting the inner error into
|
||||
// a new message with "%v" would drop that hint on the floor — the collectors
|
||||
// only ever read Message and Hint, so the two must stay separate all the way
|
||||
// to the fold.
|
||||
func prefixValidationIssue(path string, err error) error {
|
||||
msg, hint := aggregatedIssueParts(err)
|
||||
out := common.ValidationErrorf("%s: %s", path, msg).WithCause(err)
|
||||
if hint != "" {
|
||||
out = out.WithHint("%s", hint)
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
// requireJSONArray is parseJSONFlag + a type assertion to []interface{}.
|
||||
func requireJSONArray(runtime flagView, name string) ([]interface{}, error) {
|
||||
v, err := parseJSONFlag(runtime, name)
|
||||
|
||||
@@ -148,9 +148,89 @@ func batchWarnings(runtime *common.RuntimeContext) []string {
|
||||
if batchNeedsDimInsertBeforeStyleWarning(runtime) {
|
||||
out = append(out, dimInsertBeforeStyleWarning)
|
||||
}
|
||||
out = append(out, batchCollidingDimFreezeNotes(runtime)...)
|
||||
return append(out, batchLegacyDimFreezeNotes(runtime)...)
|
||||
}
|
||||
|
||||
// batchCollidingDimFreezeNotes reports +dim-freeze sub-ops that target the SAME
|
||||
// sheet more than once. Freeze is full-state replacement, so each of them
|
||||
// discards the previous one and only the last survives — both still report
|
||||
// success, which is exactly why the mistake goes unnoticed. The CLI has already
|
||||
// walked the whole ops array by this point, so it can name the survivor and the
|
||||
// single sub-op that holds everything the caller clearly meant to hold.
|
||||
//
|
||||
// A batch cannot read current state, and +styles-put (the other combined-freeze
|
||||
// carrier) is not batchable, so folding into ONE sub-op is the only fix — hence
|
||||
// a note rather than a suggestion to reorder.
|
||||
func batchCollidingDimFreezeNotes(runtime *common.RuntimeContext) []string {
|
||||
rawOps, err := parseBatchOperationsFlag(runtime)
|
||||
if err != nil {
|
||||
return nil // a malformed --operations is the translator's to report.
|
||||
}
|
||||
type freezeOp struct {
|
||||
index int
|
||||
rows, cols int
|
||||
}
|
||||
// Keyed by the sub-op's sheet selector: freezes on different sheets are
|
||||
// independent. Order of first appearance keeps the notes deterministic.
|
||||
bySheet := map[string][]freezeOp{}
|
||||
var order []string
|
||||
for i, raw := range rawOps {
|
||||
op, ok := raw.(map[string]interface{})
|
||||
if !ok {
|
||||
continue
|
||||
}
|
||||
if sc, _ := op["shortcut"].(string); sc != "+dim-freeze" {
|
||||
continue
|
||||
}
|
||||
input, _ := op["input"].(map[string]interface{})
|
||||
if input == nil {
|
||||
continue
|
||||
}
|
||||
fv := newMapFlagViewForCommand("+dim-freeze", input)
|
||||
rows, cols, ok := dimFreezeAxes(fv)
|
||||
if !ok {
|
||||
continue // an unusable sub-op is the translator's to report.
|
||||
}
|
||||
key := strings.TrimSpace(fv.Str("sheet-id")) + "\x00" + strings.TrimSpace(fv.Str("sheet-name"))
|
||||
if _, seen := bySheet[key]; !seen {
|
||||
order = append(order, key)
|
||||
}
|
||||
bySheet[key] = append(bySheet[key], freezeOp{index: i, rows: rows, cols: cols})
|
||||
}
|
||||
|
||||
var notes []string
|
||||
for _, key := range order {
|
||||
ops := bySheet[key]
|
||||
if len(ops) < 2 {
|
||||
continue
|
||||
}
|
||||
indexes := make([]string, 0, len(ops))
|
||||
// The combined state is what the caller almost certainly meant: keep the
|
||||
// last positive value named for each axis. An axis nobody ever freezes
|
||||
// stays 0, so a deliberate "unfreeze everything" batch still renders as
|
||||
// --rows 0 --cols 0 rather than inventing a freeze.
|
||||
combinedRows, combinedCols := 0, 0
|
||||
for _, op := range ops {
|
||||
indexes = append(indexes, fmt.Sprintf("operations[%d]", op.index))
|
||||
if op.rows > 0 {
|
||||
combinedRows = op.rows
|
||||
}
|
||||
if op.cols > 0 {
|
||||
combinedCols = op.cols
|
||||
}
|
||||
}
|
||||
last := ops[len(ops)-1]
|
||||
notes = append(notes, fmt.Sprintf(
|
||||
"warning: %s are all +dim-freeze on the same sheet — freeze replaces the WHOLE state, so each one discards the previous and only %s survives (ending at %s). They all report success. Replace them with ONE sub-op: %s",
|
||||
strings.Join(indexes, ", "),
|
||||
indexes[len(indexes)-1],
|
||||
dimFreezeSpelling(last.rows, last.cols),
|
||||
dimFreezeSpelling(combinedRows, combinedCols)))
|
||||
}
|
||||
return notes
|
||||
}
|
||||
|
||||
// batchLegacyDimFreezeNotes steers +dim-freeze sub-ops still written in the
|
||||
// deprecated --dimension/--count form (see DEPRECATED(phase-2) on
|
||||
// dimFreezeLegacyNote). The standalone command prints that note from its own
|
||||
|
||||
@@ -729,3 +729,145 @@ func TestSplitSheetPrefixedRange(t *testing.T) {
|
||||
// Compile-time use of json import
|
||||
_ = json.Marshal
|
||||
}
|
||||
|
||||
// TestBatchUpdate_CollidingDimFreezeWarns covers the failure mode the legacy
|
||||
// deprecation note alone could not surface: two +dim-freeze sub-ops on one
|
||||
// sheet. Freeze is full-state replacement, so the second silently discards the
|
||||
// first — and BOTH report success, which is why it goes unnoticed. Per-op
|
||||
// "equivalent to --rows 1" / "equivalent to --cols 2" notes do not say that;
|
||||
// the caller has to infer the interaction. This pins that the CLI states it.
|
||||
func TestBatchUpdate_CollidingDimFreezeWarns(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
t.Run("two per-axis freezes on one sheet", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
warning := dryRunWarning(t, BatchUpdate, []string{
|
||||
"--url", testURL,
|
||||
"--operations", `[
|
||||
{"shortcut":"+dim-freeze","input":{"sheet_name":"S1","rows":1}},
|
||||
{"shortcut":"+dim-freeze","input":{"sheet_name":"S1","cols":2}}
|
||||
]`,
|
||||
"--yes",
|
||||
})
|
||||
for _, want := range []string{
|
||||
"operations[0], operations[1]",
|
||||
"only operations[1] survives",
|
||||
"--cols 2)", // the state actually reached
|
||||
"ONE sub-op: --rows 1 --cols 2", // the fix
|
||||
} {
|
||||
if !strings.Contains(warning, want) {
|
||||
t.Errorf("collision warning should contain %q, got %q", want, warning)
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("legacy spelling collides the same way and keeps its own note", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
warning := dryRunWarning(t, BatchUpdate, []string{
|
||||
"--url", testURL,
|
||||
"--operations", `[
|
||||
{"shortcut":"+dim-freeze","input":{"sheet_name":"S1","dimension":"row","count":1}},
|
||||
{"shortcut":"+dim-freeze","input":{"sheet_name":"S1","dimension":"column","count":2}}
|
||||
]`,
|
||||
"--yes",
|
||||
})
|
||||
if !strings.Contains(warning, "ONE sub-op: --rows 1 --cols 2") {
|
||||
t.Errorf("legacy spelling should collide too, got %q", warning)
|
||||
}
|
||||
if !strings.Contains(warning, "superseded by --rows/--cols") {
|
||||
t.Errorf("per-op deprecation note should still ride along, got %q", warning)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("different sheets do not collide", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
warning := dryRunWarning(t, BatchUpdate, []string{
|
||||
"--url", testURL,
|
||||
"--operations", `[
|
||||
{"shortcut":"+dim-freeze","input":{"sheet_name":"S1","rows":1}},
|
||||
{"shortcut":"+dim-freeze","input":{"sheet_name":"S2","cols":2}}
|
||||
]`,
|
||||
"--yes",
|
||||
})
|
||||
if strings.Contains(warning, "same sheet") {
|
||||
t.Errorf("freezes on different sheets are independent, got %q", warning)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("a single freeze warns about nothing", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
warning := dryRunWarning(t, BatchUpdate, []string{
|
||||
"--url", testURL,
|
||||
"--operations", `[{"shortcut":"+dim-freeze","input":{"sheet_name":"S1","rows":1,"cols":2}}]`,
|
||||
"--yes",
|
||||
})
|
||||
if warning != "" {
|
||||
t.Errorf("one combined freeze is the correct form, got warning %q", warning)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// TestBatchOpAliasCollidesWithTarget pins the message for a sub-op carrying
|
||||
// BOTH an intuitive alias and the flag it aliases. The key is recognized, so
|
||||
// reporting it as "unknown input key" (which it did, because keys are walked
|
||||
// in sorted order and "size" sorts before "width", leaving nothing to conflict
|
||||
// with yet) sent the caller looking for a typo that was not there.
|
||||
func TestBatchOpAliasCollidesWithTarget(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
t.Run("conflicting values name both spellings", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
input := map[string]interface{}{"sheet_name": "S1", "range": "A:C", "size": float64(100), "width": float64(120)}
|
||||
err := normalizeSubOpInputKeys("+cols-resize", input)
|
||||
if err == nil {
|
||||
t.Fatal("want an error for size + width with different values")
|
||||
}
|
||||
for _, want := range []string{`"size"`, `"width"`, "same flag"} {
|
||||
if !strings.Contains(err.Error(), want) {
|
||||
t.Errorf("error should contain %q, got %q", want, err.Error())
|
||||
}
|
||||
}
|
||||
if strings.Contains(err.Error(), "unknown input key") {
|
||||
t.Errorf("an aliased key is not unknown, got %q", err.Error())
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("same value under both spellings drops the alias", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
input := map[string]interface{}{"sheet_name": "S1", "range": "A:C", "size": float64(120), "width": float64(120)}
|
||||
if err := normalizeSubOpInputKeys("+cols-resize", input); err != nil {
|
||||
t.Fatalf("identical values are harmless, got %v", err)
|
||||
}
|
||||
if _, still := input["size"]; still {
|
||||
t.Errorf("the alias should be dropped, got %#v", input)
|
||||
}
|
||||
if input["width"] != float64(120) {
|
||||
t.Errorf("width = %#v, want 120", input["width"])
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// TestBatchUpdate_AggregatedErrorsKeepHints pins that folding several bad
|
||||
// sub-ops into one message does not cost the caller the per-shortcut key
|
||||
// contract each single-op error carries — otherwise the more mistakes you
|
||||
// make, the less guidance you get.
|
||||
func TestBatchUpdate_AggregatedErrorsKeepHints(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
_, _, err := runShortcutCapturingErr(t, BatchUpdate, []string{
|
||||
"--url", testURL, "--yes",
|
||||
"--operations", `[
|
||||
{"shortcut":"+cells-set","input":{"sheet_name":"S1","bogus":1}},
|
||||
{"shortcut":"+cells-clear","input":{"sheet_name":"S1","nope":2}}
|
||||
]`,
|
||||
})
|
||||
ve := requireValidation(t, err, "2 of 2 operations failed validation")
|
||||
for _, want := range []string{
|
||||
"+cells-set input keys:",
|
||||
"+cells-clear input keys:",
|
||||
} {
|
||||
if !strings.Contains(ve.Message, want) {
|
||||
t.Errorf("aggregated message should inline %q, got %q", want, ve.Message)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -139,8 +139,17 @@ var DimInsert = common.Shortcut{
|
||||
sheetID, sheetName, _ := resolveSheetSelector(runtime)
|
||||
input, _ := dimInsertInput(runtime, token, sheetID, sheetName)
|
||||
dr := invokeToolDryRun(token, ToolKindWrite, "modify_sheet_structure", input)
|
||||
if dimInsertNeedsBeforeStyleWarning(runtime) {
|
||||
switch {
|
||||
case dimInsertNeedsBeforeStyleWarning(runtime):
|
||||
dr.Set("warning_message", dimInsertBeforeStyleWarning)
|
||||
case dimInsertAnchorShifted(runtime, input):
|
||||
// --inherit-style before anchors one unit earlier (see
|
||||
// dimInsertInput), so the previewed body carries a position the
|
||||
// caller never typed. Unexplained, that reads as an off-by-one bug in
|
||||
// exactly the artifact people dry-run to check for off-by-one bugs.
|
||||
dr.Set("warning_message", fmt.Sprintf(
|
||||
"note: the previewed position is %q, not the %q you passed — this is not an off-by-one. --inherit-style before is emulated by anchoring one row/column earlier and inserting after it, which lands in the same place while copying the PRECEDING style. The row/column still appears at %q.",
|
||||
input["position"], strings.TrimSpace(runtime.Str("position")), strings.TrimSpace(runtime.Str("position"))))
|
||||
}
|
||||
return dr
|
||||
},
|
||||
@@ -176,6 +185,16 @@ var DimInsert = common.Shortcut{
|
||||
// such edge — a plain before-insert always has a following row/column.)
|
||||
const dimInsertBeforeStyleWarning = "warning: --inherit-style before cannot copy the preceding row/column's style at the first row/column (no preceding row/column exists); inserting before --position without style inheritance. Copy styles separately if needed."
|
||||
|
||||
// dimInsertAnchorShifted reports whether the built body carries an anchor
|
||||
// position different from the one the caller passed — true exactly when the
|
||||
// --inherit-style before emulation moved it back one unit. Compared against the
|
||||
// built input rather than recomputed, so the note can never claim a shift the
|
||||
// request does not have.
|
||||
func dimInsertAnchorShifted(runtime flagView, input map[string]interface{}) bool {
|
||||
built, ok := input["position"].(string)
|
||||
return ok && built != strings.TrimSpace(runtime.Str("position"))
|
||||
}
|
||||
|
||||
func dimInsertNeedsBeforeStyleWarning(runtime flagView) bool {
|
||||
if !runtime.Changed("inherit-style") || runtime.Str("inherit-style") != "before" {
|
||||
return false
|
||||
@@ -230,9 +249,15 @@ func dimInsertInput(runtime flagView, token, sheetID, sheetName string) (map[str
|
||||
// before → side=after at P-1: the blank still lands at P (insert-after-(P-1)
|
||||
// == insert-before-P) and anchor P-1 becomes the *preceding*
|
||||
// neighbour, so the blank copies it.
|
||||
//
|
||||
// The flag is documented as defaulting to `after`, so the omitted case takes
|
||||
// the same branch instead of leaving `side` off the request: relying on the
|
||||
// backend's own default would make "omit" and "--inherit-style after" agree
|
||||
// only by coincidence, and if that default were `after` the insert would
|
||||
// land AFTER --position — breaking the "always inserts before --position"
|
||||
// contract silently. Sending it explicitly makes the documented default the
|
||||
// real one. Pinned by TestDimInsertOmittedMatchesAfter.
|
||||
switch runtime.Str("inherit-style") {
|
||||
case "after":
|
||||
input["side"] = "before"
|
||||
case "before":
|
||||
if prev, ok := a1PositionBefore(position); ok {
|
||||
input["side"] = "after"
|
||||
@@ -240,6 +265,8 @@ func dimInsertInput(runtime flagView, token, sheetID, sheetName string) (map[str
|
||||
}
|
||||
// First row/column: no preceding row/column exists, so fall back to a
|
||||
// plain before-insert (dimInsertNeedsBeforeStyleWarning surfaces this).
|
||||
default: // "after", and the omitted case it is the default for.
|
||||
input["side"] = "before"
|
||||
}
|
||||
return input, nil
|
||||
}
|
||||
@@ -525,14 +552,52 @@ func dimFreezeLegacyNote(runtime flagView) string {
|
||||
// --dimension/--count call, so the deprecation note carries the exact
|
||||
// replacement instead of a generic pointer.
|
||||
func dimFreezeEquivalent(runtime flagView) string {
|
||||
count := runtime.Int("count")
|
||||
if count == 0 {
|
||||
return "--rows 0 --cols 0"
|
||||
rows, cols, _ := dimFreezeAxes(runtime)
|
||||
return dimFreezeSpelling(rows, cols)
|
||||
}
|
||||
|
||||
// dimFreezeAxes maps either request form onto the (rows, cols) freeze state it
|
||||
// asks for. Pure mapping, no validation — dimFreezeInput validates first and
|
||||
// then calls this, so the request body, the deprecation note and the batch
|
||||
// collision note can never disagree about what a call means. ok is false when
|
||||
// the flags name no state at all, or when the legacy pair is half-given
|
||||
// (--count without --dimension); dimFreezeInput reports both.
|
||||
func dimFreezeAxes(runtime flagView) (rows, cols int, ok bool) {
|
||||
pairForm := runtime.Changed("dimension") || runtime.Changed("count")
|
||||
axisForm := runtime.Changed("rows") || runtime.Changed("cols")
|
||||
switch {
|
||||
case axisForm && !pairForm:
|
||||
return runtime.Int("rows"), runtime.Int("cols"), true
|
||||
case pairForm && !axisForm:
|
||||
if !runtime.Changed("dimension") || !runtime.Changed("count") {
|
||||
return 0, 0, false
|
||||
}
|
||||
// A zero count clears BOTH axes — it is the bare unfreeze operation,
|
||||
// which carries no dimension.
|
||||
if count := runtime.Int("count"); count > 0 {
|
||||
if runtime.Str("dimension") == "row" {
|
||||
return count, 0, true
|
||||
}
|
||||
return 0, count, true
|
||||
}
|
||||
return 0, 0, true
|
||||
}
|
||||
if runtime.Str("dimension") == "row" {
|
||||
return fmt.Sprintf("--rows %d", count)
|
||||
return 0, 0, false
|
||||
}
|
||||
|
||||
// dimFreezeSpelling renders a freeze state as the --rows/--cols flags that
|
||||
// produce it. Single source of the replacement wording, shared by the
|
||||
// deprecation note and the batch collision note.
|
||||
func dimFreezeSpelling(rows, cols int) string {
|
||||
switch {
|
||||
case rows > 0 && cols > 0:
|
||||
return fmt.Sprintf("--rows %d --cols %d", rows, cols)
|
||||
case rows > 0:
|
||||
return fmt.Sprintf("--rows %d", rows)
|
||||
case cols > 0:
|
||||
return fmt.Sprintf("--cols %d", cols)
|
||||
}
|
||||
return fmt.Sprintf("--cols %d", count)
|
||||
return "--rows 0 --cols 0"
|
||||
}
|
||||
|
||||
// dimFreezeInput builds the freeze body for both the standalone shortcut and
|
||||
@@ -561,18 +626,19 @@ func dimFreezeInput(runtime flagView, token, sheetID, sheetName string) (map[str
|
||||
return nil, sheetsValidationForFlag("rows",
|
||||
"give either --rows/--cols or --dimension/--count, not both — they are two ways to say the same thing; --rows/--cols is the one that can hold both axes at once")
|
||||
case !pairForm && !axisForm:
|
||||
// Prescribes only --rows/--cols: --dimension/--count is retired
|
||||
// (DEPRECATED(phase-2)) and steering a caller into it here would earn
|
||||
// them a deprecation note on the very next call.
|
||||
return nil, sheetsValidationForFlag("rows",
|
||||
"nothing to freeze: pass --rows N and/or --cols N (e.g. --rows 1 --cols 2 holds the header row and the first 2 columns), or the single-axis --dimension row --count 1")
|
||||
"nothing to freeze: pass --rows N and/or --cols N — e.g. --rows 1 holds the header row, --rows 1 --cols 2 holds it plus the first 2 columns, --rows 0 --cols 0 unfreezes everything")
|
||||
}
|
||||
|
||||
rows, cols := 0, 0
|
||||
if axisForm {
|
||||
for _, name := range []string{"rows", "cols"} {
|
||||
if runtime.Changed(name) && runtime.Int(name) < 0 {
|
||||
return nil, sheetsValidationForFlag(name, "--%s must be >= 0 (0 leaves that axis unfrozen)", name)
|
||||
}
|
||||
}
|
||||
rows, cols = runtime.Int("rows"), runtime.Int("cols")
|
||||
} else {
|
||||
if !runtime.Changed("dimension") {
|
||||
return nil, sheetsValidationForFlag("dimension", "--dimension is required alongside --count (or use --rows/--cols to set both axes at once)")
|
||||
@@ -583,12 +649,10 @@ func dimFreezeInput(runtime flagView, token, sheetID, sheetName string) (map[str
|
||||
if runtime.Int("count") < 0 {
|
||||
return nil, sheetsValidationForFlag("count", "--count must be >= 0")
|
||||
}
|
||||
if runtime.Str("dimension") == "row" {
|
||||
rows = runtime.Int("count")
|
||||
} else {
|
||||
cols = runtime.Int("count")
|
||||
}
|
||||
}
|
||||
// Validation done; the flags-to-state mapping is dimFreezeAxes', shared with
|
||||
// the deprecation and collision notes so the three cannot disagree.
|
||||
rows, cols, _ := dimFreezeAxes(runtime)
|
||||
|
||||
// An all-zero target is the bare "unfreeze" operation, which carries no
|
||||
// dimension and clears everything — the same request the old --count 0
|
||||
|
||||
@@ -265,10 +265,14 @@ func TestDimInsertInheritStyleSideMapping(t *testing.T) {
|
||||
wantSideSet: true,
|
||||
},
|
||||
{
|
||||
name: "default (flag omitted) omits side, backend inherits the following row/column",
|
||||
// The flag documents `after` as its default, so omitting it must
|
||||
// build the same body rather than leaving `side` to the backend's
|
||||
// own default — see TestDimInsertOmittedMatchesAfter.
|
||||
name: "default (flag omitted) sends the same side as --inherit-style after",
|
||||
position: "D",
|
||||
wantPosition: "D",
|
||||
wantSideSet: false,
|
||||
wantSide: "before",
|
||||
wantSideSet: true,
|
||||
},
|
||||
}
|
||||
|
||||
@@ -301,6 +305,33 @@ func TestDimInsertInheritStyleSideMapping(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestDimInsertOmittedMatchesAfter pins the contract --inherit-style's flag
|
||||
// description states: omitting it is the same call as passing `after`.
|
||||
//
|
||||
// It used to hold only if the backend happened to default `side` to "before".
|
||||
// That is not something the docs can promise on the backend's behalf — and if
|
||||
// the default were "after", omitting the flag would insert AFTER --position,
|
||||
// silently breaking +dim-insert's "always inserts before --position" contract.
|
||||
// So the CLI sends `side` explicitly and this test locks the two bodies
|
||||
// together, byte for byte.
|
||||
func TestDimInsertOmittedMatchesAfter(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
for _, position := range []string{"1", "3", "A", "D"} {
|
||||
t.Run("position "+position, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
base := []string{"--url", testURL, "--sheet-id", testSheetID, "--position", position, "--count", "1"}
|
||||
omitted := decodeToolInput(t, parseDryRunBody(t, DimInsert, base), "modify_sheet_structure")
|
||||
explicit := decodeToolInput(t,
|
||||
parseDryRunBody(t, DimInsert, append(append([]string{}, base...), "--inherit-style", "after")),
|
||||
"modify_sheet_structure")
|
||||
if !reflect.DeepEqual(omitted, explicit) {
|
||||
t.Fatalf("omitted --inherit-style built %#v, --inherit-style after built %#v; they must be identical", omitted, explicit)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestDimRange_Validation covers the A1 range parser's edge cases routed
|
||||
// through +dim-hide (any --range shortcut works; we just need to exercise
|
||||
// the validator).
|
||||
|
||||
@@ -924,7 +924,11 @@ type workbookCreateStylePayload struct {
|
||||
}
|
||||
|
||||
// workbookCreateFreezeOp freezes the first Rows rows / Cols columns.
|
||||
// Zero means "leave that dimension alone".
|
||||
// Zero means "that axis ends up UNFROZEN", not "leave it alone": freeze is
|
||||
// full-state replacement server-side (see workbookCreateVisualOpInput's freeze
|
||||
// branch), so a declarative spec that omits an axis is stating it should not be
|
||||
// frozen. parseWorkbookCreateFreezeOp rejects an all-zero op, so at least one
|
||||
// axis is always positive here.
|
||||
type workbookCreateFreezeOp struct {
|
||||
Rows int
|
||||
Cols int
|
||||
@@ -1092,11 +1096,15 @@ func parseWorkbookCreateStyleItem(item map[string]interface{}, path string) (*wo
|
||||
probs = append(probs, common.ValidationErrorf("%s", msg))
|
||||
}
|
||||
// Normalize "Sheet!" range prefixes before the section parsers see them:
|
||||
// every carrier names the target sheet on the item, so a prefix is at best
|
||||
// redundant and at worst a silent retarget.
|
||||
if name, _ := item["name"].(string); strings.TrimSpace(name) != "" {
|
||||
probs = append(probs, normalizeStyleItemRangePrefixes(item, path, strings.TrimSpace(name))...)
|
||||
}
|
||||
// the target sheet is named by the item (or, on +workbook-create --values,
|
||||
// by the single sheet being created), so a prefix is at best redundant and
|
||||
// at worst a silent retarget. Stripping is unconditional — an item without
|
||||
// a name (the --values path, where name is optional) must not be left with
|
||||
// prefixed ranges the section parsers then reject as malformed. Only the
|
||||
// "names a DIFFERENT sheet" report needs a name to compare against, so it
|
||||
// is skipped when there is none.
|
||||
name, _ := item["name"].(string)
|
||||
probs = append(probs, normalizeStyleItemRangePrefixes(item, path, strings.TrimSpace(name))...)
|
||||
if raw, ok := item["cell_styles"]; ok {
|
||||
var errsHere []error
|
||||
payload.CellStyles, errsHere = parseWorkbookCreateCellStyleOps(raw, path+".cell_styles")
|
||||
@@ -1151,6 +1159,12 @@ var styleItemRangeSections = []string{"cell_styles", "row_sizes", "col_sizes", "
|
||||
// (name "Summary" + range "Detail!A1:D1" applying to Summary). It is stripped
|
||||
// anyway so the section parser reports the entry's own issues instead of piling
|
||||
// a redundant syntax error on top of the mismatch.
|
||||
//
|
||||
// name is "" on +workbook-create --values, whose single styles item needs no
|
||||
// name (the workbook has exactly one sheet, still unnamed at spec time). There
|
||||
// is then no sheet to disagree with, so ranges are stripped without the
|
||||
// mismatch report — stripping still has to happen, or the section parsers see
|
||||
// a prefixed range and reject it as malformed.
|
||||
func normalizeStyleItemRangePrefixes(item map[string]interface{}, path, name string) []error {
|
||||
var probs []error
|
||||
rewrite := func(section, rangeStr string) (string, bool) {
|
||||
@@ -1159,7 +1173,7 @@ func normalizeStyleItemRangePrefixes(item map[string]interface{}, path, name str
|
||||
return "", false
|
||||
}
|
||||
prefix := strings.Trim(strings.TrimSpace(rangeStr[:idx]), "'")
|
||||
if prefix != name {
|
||||
if name != "" && prefix != name {
|
||||
probs = append(probs, common.ValidationErrorf(
|
||||
"%s.%s range %q names sheet %q but the item targets %q — drop the prefix, or move the entry into the item for %q",
|
||||
path, section, rangeStr, prefix, name, prefix))
|
||||
@@ -1254,21 +1268,19 @@ func joinStyleValidationErrors(probs []error) error {
|
||||
// Re-attribute to the outer flag even for a single issue: the inner
|
||||
// error is scoped to a nested path and carries no Param, so an agent
|
||||
// would have to parse prose to learn which flag to fix. Message text
|
||||
// is preserved; only the typed attribution is added.
|
||||
msg := probs[0].Error()
|
||||
if p, ok := errs.ProblemOf(probs[0]); ok {
|
||||
msg = p.Message
|
||||
// is preserved; only the typed attribution is added — and the inner
|
||||
// hint rides along, since a lone issue has the outer Hint slot free.
|
||||
msg, hint := aggregatedIssueParts(probs[0])
|
||||
verr := sheetsValidationForFlag("styles", "%s", msg).WithCause(probs[0])
|
||||
if hint != "" {
|
||||
verr = verr.WithHint("%s", hint)
|
||||
}
|
||||
return sheetsValidationForFlag("styles", "%s", msg).WithCause(probs[0])
|
||||
return verr
|
||||
}
|
||||
const maxShown = 8
|
||||
msgs := make([]string, 0, len(probs))
|
||||
for _, e := range probs {
|
||||
if p, ok := errs.ProblemOf(e); ok {
|
||||
msgs = append(msgs, p.Message)
|
||||
continue
|
||||
}
|
||||
msgs = append(msgs, e.Error())
|
||||
msgs = append(msgs, aggregatedIssueText(e))
|
||||
}
|
||||
suffix := ""
|
||||
if len(msgs) > maxShown {
|
||||
@@ -1602,8 +1614,13 @@ func normalizeWorkbookCreateStyleObject(in map[string]interface{}, path string)
|
||||
return out, nil
|
||||
}
|
||||
|
||||
// workbookCreateCellStyleFieldList is the canonical style vocabulary plus the
|
||||
// two border carriers, in display order for the unknown-field hint.
|
||||
// workbookCreateCellStyleFieldList is what a caller may WRITE in a cell_styles
|
||||
// item, in display order for the unknown-field hint — the canonical scalar
|
||||
// vocabulary (workbookCreateCellStyleField) plus the two border carriers.
|
||||
// "border" is the documented four-sides shorthand rather than a field the
|
||||
// switch above ever sees: foldBorderFamilyAliases folds it into border_styles
|
||||
// first. It belongs in this list because the list answers "what may I write",
|
||||
// not "what survives normalization".
|
||||
var workbookCreateCellStyleFieldList = []string{
|
||||
"font_color", "font_family", "font_size", "font_weight", "font_style", "font_line",
|
||||
"background_color", "horizontal_alignment", "vertical_alignment",
|
||||
|
||||
@@ -748,19 +748,49 @@ func TestStyleItemRangePrefixNormalization(t *testing.T) {
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("unnamed item keeps its ranges untouched", func(t *testing.T) {
|
||||
// +workbook-create's untyped initial fill has no sheet name to compare
|
||||
// against, so there is nothing to validate a prefix as redundant.
|
||||
t.Run("unnamed item still gets its prefixes stripped", func(t *testing.T) {
|
||||
// +workbook-create --values' styles item needs no name (one sheet, not
|
||||
// yet named), but stripping must not be conditional on having one: the
|
||||
// section parsers feed ranges to parseA1Range, so a surviving prefix
|
||||
// turns an unambiguous spec into a malformed-range error. With no name
|
||||
// there is simply nothing to disagree with, so no mismatch is reported.
|
||||
t.Parallel()
|
||||
item := map[string]interface{}{
|
||||
"cell_styles": []interface{}{map[string]interface{}{"range": "Sheet1!A1:D1", "font_weight": "bold"}},
|
||||
"row_sizes": []interface{}{map[string]interface{}{"range": "Sheet1!1:1", "size": float64(30)}},
|
||||
}
|
||||
payload, probs := parseWorkbookCreateStyleItem(item, "--styles.styles[0]")
|
||||
if len(probs) > 0 {
|
||||
t.Fatalf("unexpected probs: %v", probs)
|
||||
}
|
||||
if payload.CellStyles[0].Range != "Sheet1!A1:D1" {
|
||||
t.Fatalf("range = %q, want it left as written", payload.CellStyles[0].Range)
|
||||
if payload.CellStyles[0].Range != "A1:D1" {
|
||||
t.Fatalf("cell_styles range = %q, want the prefix stripped", payload.CellStyles[0].Range)
|
||||
}
|
||||
if payload.RowSizes[0].Range != "1:1" {
|
||||
t.Fatalf("row_sizes range = %q, want the prefix stripped", payload.RowSizes[0].Range)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("all three carriers accept a prefixed row_sizes range", func(t *testing.T) {
|
||||
// The regression this guards: prefix stripping used to live only on the
|
||||
// named-item path, so +workbook-create --values (whose item carries no
|
||||
// name) still failed on "Sheet1!2:3" while +table-put / +styles-put
|
||||
// accepted it.
|
||||
t.Parallel()
|
||||
for _, name := range []string{"", "Sheet1"} {
|
||||
item := map[string]interface{}{
|
||||
"row_sizes": []interface{}{map[string]interface{}{"range": "Sheet1!2:3", "size": float64(30)}},
|
||||
}
|
||||
if name != "" {
|
||||
item["name"] = name
|
||||
}
|
||||
payload, probs := parseWorkbookCreateStyleItem(item, "--styles.styles[0]")
|
||||
if len(probs) > 0 {
|
||||
t.Fatalf("name=%q: unexpected probs: %v", name, probs)
|
||||
}
|
||||
if payload.RowSizes[0].Range != "2:3" {
|
||||
t.Fatalf("name=%q: range = %q, want %q", name, payload.RowSizes[0].Range, "2:3")
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
@@ -170,7 +170,12 @@ func cellsSetWritesOps(runtime *common.RuntimeContext, token string) ([]interfac
|
||||
sheetName := strings.TrimSpace(fv.Str("sheet-name"))
|
||||
input, err := cellsSetInput(fv, token, sheetID, sheetName)
|
||||
if err != nil {
|
||||
probs = append(probs, common.ValidationErrorf("--writes[%d]: %v", i, err))
|
||||
// Prefix with the item index WITHOUT flattening: cellsSetInput's
|
||||
// errors carry the domain's prescriptions in Hint (requireSheetSelector's
|
||||
// "+workbook-info" pointer, for one) and "%v" would render only the
|
||||
// message, silently costing exactly the guidance this path exists to
|
||||
// deliver. joinWritesValidationErrors re-reads both fields.
|
||||
probs = append(probs, prefixValidationIssue(fmt.Sprintf("--writes[%d]", i), err))
|
||||
continue
|
||||
}
|
||||
if cells, ok := input["cells"].([]interface{}); ok {
|
||||
@@ -205,21 +210,19 @@ func joinWritesValidationErrors(probs []error) error {
|
||||
// Re-attribute to the outer flag even for a single issue: the inner
|
||||
// error is scoped to a nested path and carries no Param, so an agent
|
||||
// would have to parse prose to learn which flag to fix. Message text
|
||||
// is preserved; only the typed attribution is added.
|
||||
msg := probs[0].Error()
|
||||
if p, ok := errs.ProblemOf(probs[0]); ok {
|
||||
msg = p.Message
|
||||
// is preserved; only the typed attribution is added — and the inner
|
||||
// hint rides along, since a lone issue has the outer Hint slot free.
|
||||
msg, hint := aggregatedIssueParts(probs[0])
|
||||
verr := sheetsValidationForFlag("writes", "%s", msg).WithCause(probs[0])
|
||||
if hint != "" {
|
||||
verr = verr.WithHint("%s", hint)
|
||||
}
|
||||
return sheetsValidationForFlag("writes", "%s", msg).WithCause(probs[0])
|
||||
return verr
|
||||
}
|
||||
const maxShown = 8
|
||||
msgs := make([]string, 0, len(probs))
|
||||
for _, e := range probs {
|
||||
if p, ok := errs.ProblemOf(e); ok {
|
||||
msgs = append(msgs, p.Message)
|
||||
continue
|
||||
}
|
||||
msgs = append(msgs, e.Error())
|
||||
msgs = append(msgs, aggregatedIssueText(e))
|
||||
}
|
||||
suffix := ""
|
||||
if len(msgs) > maxShown {
|
||||
|
||||
@@ -40,10 +40,17 @@ const outputPathReadLimit = 20_000_000
|
||||
// tool. A cap the user set explicitly always binds — --output-path only
|
||||
// raises the default (to the bounded outputPathReadLimit) when --max-chars
|
||||
// was left alone, so a full read lands in the file without silently
|
||||
// discarding a requested limit. The second return is false when nothing
|
||||
// should be sent (max-chars <= 0), in which case the tool's own default
|
||||
// applies. Note the tool truncates at ~50000 even when max_chars is omitted,
|
||||
// so callers that want an explicit cap should pass a positive default.
|
||||
// discarding a requested limit.
|
||||
//
|
||||
// --max-chars 0 (or negative) means "no cap of my own", and is deliberately
|
||||
// NOT passed through as "send nothing": omitting max_chars makes the tool
|
||||
// apply its own ~50000 fallback, i.e. a caller asking for no limit would get
|
||||
// the SMALLEST one — the opposite of the request, and silently. It resolves
|
||||
// to the same ceiling an unset flag would: the offload limit when writing to
|
||||
// a file, otherwise the flag's declared default.
|
||||
//
|
||||
// The second return is false only when there is no cap to send at all, which
|
||||
// today means the flag is absent from this shortcut.
|
||||
func maxCharsInput(runtime *common.RuntimeContext) (int, bool) {
|
||||
if n := runtime.Int("max-chars"); n > 0 && runtime.Changed("max-chars") {
|
||||
return n, true
|
||||
@@ -51,12 +58,25 @@ func maxCharsInput(runtime *common.RuntimeContext) (int, bool) {
|
||||
if readOutputPath(runtime) != "" {
|
||||
return outputPathReadLimit, true
|
||||
}
|
||||
// The flag's own default (500000) — reached both when it is unset and when
|
||||
// it was explicitly zeroed.
|
||||
if n := runtime.Int("max-chars"); n > 0 {
|
||||
return n, true
|
||||
}
|
||||
if runtime.Changed("max-chars") {
|
||||
return maxCharsFallback, true
|
||||
}
|
||||
return 0, false
|
||||
}
|
||||
|
||||
// maxCharsFallback is the ceiling used when a caller explicitly asks for no
|
||||
// cap (--max-chars 0) without redirecting to a file. It matches the flag's
|
||||
// declared default rather than the tool's much smaller omitted-value
|
||||
// fallback, and stays well inside the non-streaming read path's memory
|
||||
// budget (see outputPathReadLimit); a caller who wants more says so with a
|
||||
// positive --max-chars or --output-path.
|
||||
const maxCharsFallback = 500_000
|
||||
|
||||
// maxCharsBudget returns the char cap that bounds a whole multi-sheet read
|
||||
// (0 when no cap is in play). Callers that read several sheets in one command
|
||||
// spend this budget across all of them rather than per sheet.
|
||||
|
||||
@@ -67,3 +67,46 @@ func TestReadResultTruncated_AllLevels(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestMaxCharsInput_ExplicitZero pins that asking for "no cap of my own" does
|
||||
// not land on the SMALLEST cap. Omitting max_chars makes the read tool apply
|
||||
// its own ~50000 fallback, so passing the request straight through would give
|
||||
// --max-chars 0 a tighter limit than leaving the flag alone — the opposite of
|
||||
// what it reads like, and silently.
|
||||
func TestMaxCharsInput_ExplicitZero(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
t.Run("resolves to the flag default, not the tool fallback", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
input := cellsGetToolInput(t, []string{"--max-chars", "0"})
|
||||
got, ok := input["max_chars"]
|
||||
if !ok {
|
||||
t.Fatalf("max_chars must be sent, or the tool's ~50000 fallback binds: %#v", input)
|
||||
}
|
||||
if got != float64(maxCharsFallback) {
|
||||
t.Errorf("max_chars = %v, want %d", got, maxCharsFallback)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("--output-path still raises it to the offload limit", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
input := cellsGetToolInput(t, []string{"--max-chars", "0", "--output-path", "./o.json"})
|
||||
if got := input["max_chars"]; got != float64(outputPathReadLimit) {
|
||||
t.Errorf("max_chars = %v, want %d", got, outputPathReadLimit)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("a positive explicit cap still wins over --output-path", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
input := cellsGetToolInput(t, []string{"--max-chars", "1234", "--output-path", "./o.json"})
|
||||
if got := input["max_chars"]; got != float64(1234) {
|
||||
t.Errorf("max_chars = %v, want 1234", got)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func cellsGetToolInput(t *testing.T, extra []string) map[string]interface{} {
|
||||
t.Helper()
|
||||
args := append([]string{"--url", testURL, "--sheet-name", "S1", "--range", "A1:B2"}, extra...)
|
||||
return decodeToolInput(t, parseDryRunBody(t, CellsGet, args), "get_cell_ranges")
|
||||
}
|
||||
|
||||
@@ -87,7 +87,8 @@ func callTool(
|
||||
// The recovery prescription depends on the execution mode the batch
|
||||
// was sent with; non-batch tools simply lack the key (false).
|
||||
continueOnError, _ := input["continue_on_error"].(bool)
|
||||
return nil, errs.NewAPIError(errs.SubtypeServerError, "tool %q failed: [%d] %s", toolName, int(code), flattenToolErrorMsg(msg, continueOnError)).
|
||||
flat := flattenToolErrorMsg(msg, continueOnError, callerAuthoredOperations(runtime.Command()))
|
||||
return nil, errs.NewAPIError(errs.SubtypeServerError, "tool %q failed: [%d] %s", toolName, int(code), flat).
|
||||
WithCode(int(code))
|
||||
}
|
||||
data, _ := envelope["data"].(map[string]interface{})
|
||||
@@ -116,7 +117,17 @@ func callTool(
|
||||
// continueOnError is the execution mode the batch was sent with: it decides
|
||||
// the recovery prescription, because a single listed failure only implies
|
||||
// "nothing after it ran" under fail-fast.
|
||||
func flattenToolErrorMsg(msg string, continueOnError bool) string {
|
||||
//
|
||||
// callerAuthoredOps says whether the operations array the server indexes into
|
||||
// is the one the CALLER wrote. Only +batch-update's --operations is; every
|
||||
// other batch_update user (+styles-put, +cells-set --writes, +dim-delete
|
||||
// --ranges, the fan-out stampers) synthesizes the array client-side, and
|
||||
// +styles-put coalesces while +dim-delete deliberately re-sorts descending —
|
||||
// so "operations[3]" there names nothing the caller can find, and
|
||||
// "resend operations[3:]" is not a command they can issue. Those callers get
|
||||
// the per-op detail (still the best available description of what failed) plus
|
||||
// a generic no-rollback warning, never an index-based resend instruction.
|
||||
func flattenToolErrorMsg(msg string, continueOnError, callerAuthoredOps bool) string {
|
||||
trimmed := strings.TrimSpace(msg)
|
||||
if !strings.HasPrefix(trimmed, "{") {
|
||||
return msg
|
||||
@@ -158,9 +169,14 @@ func flattenToolErrorMsg(msg string, continueOnError bool) string {
|
||||
// alone; prescribing the tail there would double-apply the successes.
|
||||
if strings.Contains(detail.Message, "succeeded") &&
|
||||
!strings.Contains(detail.Message, " 0 succeeded") {
|
||||
if !continueOnError && len(detail.Failures) == 1 {
|
||||
switch {
|
||||
case !callerAuthoredOps:
|
||||
// Client-side expansion: the indexes above are internal, so
|
||||
// prescribe a read-back instead of an un-issuable resend.
|
||||
out += "; note: this command expands into the operations above client-side, so their indexes are not something you can resend directly. Succeeded operations stay applied (no rollback) — read the affected area back (+sheet-info / +cells-get), then re-issue only the part that did not land"
|
||||
case !continueOnError && len(detail.Failures) == 1:
|
||||
out += fmt.Sprintf("; note: succeeded operations stay applied (no rollback) — fix the failure and resend only operations[%d:] onward, do not resend the whole batch", firstFailed)
|
||||
} else {
|
||||
default:
|
||||
out += "; note: succeeded operations stay applied (no rollback) — fix and resend only the failed operations listed above, do not resend the whole batch"
|
||||
}
|
||||
}
|
||||
@@ -169,6 +185,12 @@ func flattenToolErrorMsg(msg string, continueOnError bool) string {
|
||||
return inner
|
||||
}
|
||||
|
||||
// callerAuthoredOperations reports whether `command` is the one shortcut whose
|
||||
// batch_update operations array the caller wrote by hand. Everything else
|
||||
// synthesizes it, so server-reported operation indexes are internal detail
|
||||
// there (see flattenToolErrorMsg).
|
||||
func callerAuthoredOperations(command string) bool { return command == "+batch-update" }
|
||||
|
||||
// invokeToolDryRun renders the One-OpenAPI request the shortcut would send.
|
||||
// The wire-format body (with input serialized to a JSON string) is preserved
|
||||
// for fidelity, and a decoded tool_input map is surfaced alongside so humans
|
||||
|
||||
@@ -17,7 +17,7 @@ func TestFlattenToolErrorMsg(t *testing.T) {
|
||||
t.Run("batch failures flatten to one line", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
msg := `{"error":"{\"message\":\"batch_update: 0 succeeded, 1 failed\",\"succeeded\":0,\"failed\":1,\"failures\":[{\"index\":0,\"tool_name\":\"manage_chart_object\",\"error\":\"invalid snapshot.data.dim1.serie.index: 0, must be >= 1 (index is 1-based)\",\"errorType\":\"param_error\"}]}","errorType":"param_error","data":{"total":2,"succeeded":0,"failed":1}}`
|
||||
got := flattenToolErrorMsg(msg, false)
|
||||
got := flattenToolErrorMsg(msg, false, true)
|
||||
for _, want := range []string{
|
||||
"batch_update: 0 succeeded, 1 failed",
|
||||
"operations[0] (manage_chart_object): invalid snapshot.data.dim1.serie.index",
|
||||
@@ -33,7 +33,7 @@ func TestFlattenToolErrorMsg(t *testing.T) {
|
||||
|
||||
t.Run("plain-string inner error unwraps", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
got := flattenToolErrorMsg(`{"error":"sheet \"s\" not found","errorType":"param_error"}`, false)
|
||||
got := flattenToolErrorMsg(`{"error":"sheet \"s\" not found","errorType":"param_error"}`, false, true)
|
||||
if got != `sheet "s" not found` {
|
||||
t.Errorf("got %q", got)
|
||||
}
|
||||
@@ -42,7 +42,7 @@ func TestFlattenToolErrorMsg(t *testing.T) {
|
||||
t.Run("non-JSON msg passes through", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
msg := `cell at row 0, col 1 is inside a merged region (top-left: A1)`
|
||||
if got := flattenToolErrorMsg(msg, false); got != msg {
|
||||
if got := flattenToolErrorMsg(msg, false, true); got != msg {
|
||||
t.Errorf("got %q", got)
|
||||
}
|
||||
})
|
||||
@@ -50,8 +50,63 @@ func TestFlattenToolErrorMsg(t *testing.T) {
|
||||
t.Run("JSON without error field passes through", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
msg := `{"detail":"x"}`
|
||||
if got := flattenToolErrorMsg(msg, false); got != msg {
|
||||
if got := flattenToolErrorMsg(msg, false, true); got != msg {
|
||||
t.Errorf("got %q", got)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// TestFlattenToolErrorMsg_OperationIndexProvenance pins who may be told to
|
||||
// "resend operations[N:]". Only +batch-update's --operations is written by the
|
||||
// caller; +styles-put, +cells-set --writes, +dim-delete --ranges and the
|
||||
// fan-out stampers synthesize the array — +styles-put even coalesces adjacent
|
||||
// stamps and +dim-delete deliberately re-sorts descending, so an index there
|
||||
// names nothing the caller can locate, let alone resend.
|
||||
func TestFlattenToolErrorMsg_OperationIndexProvenance(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
const msg = `{"error":"{\"message\":\"batch_update: 4 succeeded, 1 failed\",\"failures\":[{\"index\":4,\"tool_name\":\"set_cell_range\",\"error\":\"cells is required\"}]}","errorType":"param_error"}`
|
||||
|
||||
t.Run("caller-authored operations get the index-based resend", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
got := flattenToolErrorMsg(msg, false, true)
|
||||
if !strings.Contains(got, "resend only operations[4:] onward") {
|
||||
t.Errorf("want the index-based prescription, got %q", got)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("client-side expansion gets a read-back instead", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
got := flattenToolErrorMsg(msg, false, false)
|
||||
if strings.Contains(got, "resend only operations[") {
|
||||
t.Errorf("must not prescribe an index the caller never wrote, got %q", got)
|
||||
}
|
||||
for _, want := range []string{
|
||||
"expands into the operations above client-side",
|
||||
"stay applied (no rollback)",
|
||||
"read the affected area back",
|
||||
} {
|
||||
if !strings.Contains(got, want) {
|
||||
t.Errorf("want %q in the prescription, got %q", want, got)
|
||||
}
|
||||
}
|
||||
// The per-op detail is still the best description of what failed.
|
||||
if !strings.Contains(got, "operations[4] (set_cell_range): cells is required") {
|
||||
t.Errorf("per-op detail must survive, got %q", got)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// TestCallerAuthoredOperations names the one shortcut whose operations array
|
||||
// is the caller's own. Every other batch_update user builds it.
|
||||
func TestCallerAuthoredOperations(t *testing.T) {
|
||||
t.Parallel()
|
||||
if !callerAuthoredOperations("+batch-update") {
|
||||
t.Error("+batch-update writes its own --operations")
|
||||
}
|
||||
for _, sc := range []string{"+styles-put", "+cells-set", "+dim-delete", "+cells-batch-clear", "+dropdown-update"} {
|
||||
if callerAuthoredOperations(sc) {
|
||||
t.Errorf("%s synthesizes its operations array client-side", sc)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -6,6 +6,7 @@ package sheets
|
||||
import (
|
||||
"fmt"
|
||||
"slices"
|
||||
"sort"
|
||||
"strings"
|
||||
|
||||
"github.com/larksuite/cli/shortcuts/common"
|
||||
@@ -42,6 +43,19 @@ import (
|
||||
// corpus (every observed model spelling either normalizes or
|
||||
// prescribes). New eval finding → corpus row → fix HERE → locked.
|
||||
|
||||
// sortedKeys returns a map's keys in sorted order, so any loop that can abort
|
||||
// with an error reports a deterministic one. Used throughout this file: the
|
||||
// acceptance layer is all map-shaped vocabulary, and "which of my three bad
|
||||
// fields did it complain about" must not change between runs.
|
||||
func sortedKeys[V any](m map[string]V) []string {
|
||||
keys := make([]string, 0, len(m))
|
||||
for k := range m {
|
||||
keys = append(keys, k)
|
||||
}
|
||||
sort.Strings(keys)
|
||||
return keys
|
||||
}
|
||||
|
||||
// ─── style flags (shared by +cells-set-style and +cells-batch-set-style) ─
|
||||
|
||||
// buildCellStyleFromFlags reads the 12 flat style flags and returns the
|
||||
@@ -225,7 +239,12 @@ func normalizeCellStyleAliases(style map[string]interface{}, path string) error
|
||||
// describes the outer envelope, not each cell_styles object), so assert the
|
||||
// declared type here — otherwise {"font_weight": true} sails through
|
||||
// normalization and reaches the server as a boolean.
|
||||
for field, want := range cellStyleScalarTypes() {
|
||||
// Both loops below can abort with an error, so they walk their vocabulary in
|
||||
// sorted order: map iteration would let the same bad payload report a
|
||||
// different field on every run.
|
||||
scalarTypes := cellStyleScalarTypes()
|
||||
for _, field := range sortedKeys(scalarTypes) {
|
||||
want := scalarTypes[field]
|
||||
raw, has := style[field]
|
||||
if !has || raw == nil {
|
||||
continue
|
||||
@@ -235,7 +254,9 @@ func normalizeCellStyleAliases(style map[string]interface{}, path string) error
|
||||
path, field, want, got, formatJSONValue(raw))
|
||||
}
|
||||
}
|
||||
for field, enum := range cellStyleEnumFields() {
|
||||
enumFields := cellStyleEnumFields()
|
||||
for _, field := range sortedKeys(enumFields) {
|
||||
enum := enumFields[field]
|
||||
raw, has := style[field]
|
||||
if !has {
|
||||
continue
|
||||
@@ -448,7 +469,13 @@ func requireAnyStyleFlag(runtime flagView) error {
|
||||
// habitual Excel word) sets weight and defaults style to solid: a "thin
|
||||
// border" always means a thin solid line. Conflicts with an explicitly given
|
||||
// border_styles error out instead of picking a side.
|
||||
// Every walk over a set here goes through an ORDERED slice, never a map range:
|
||||
// each branch below can abort with an error, so map iteration order would
|
||||
// decide which of several bad fields gets reported and the same payload would
|
||||
// produce different messages run to run (same reason parseWorkbookCreateFreezeOp
|
||||
// sorts its keys).
|
||||
func foldBorderFamilyAliases(in map[string]interface{}, path string) error {
|
||||
attrNames := []string{"color", "style", "weight"}
|
||||
sides := map[string]bool{"top": true, "bottom": true, "left": true, "right": true, "all": true}
|
||||
attrs := map[string]bool{"style": true, "color": true, "weight": true}
|
||||
borderWeights := map[string]bool{"thin": true, "medium": true, "thick": true}
|
||||
@@ -482,11 +509,11 @@ func foldBorderFamilyAliases(in map[string]interface{}, path string) error {
|
||||
if !ok {
|
||||
return common.ValidationErrorf("%s.%s must be an object like {\"style\":\"solid\",\"color\":\"#000000\"}", path, from)
|
||||
}
|
||||
for attr, av := range obj {
|
||||
for _, attr := range sortedKeys(obj) {
|
||||
if !attrs[attr] {
|
||||
return common.ValidationErrorf("%s.%s.%s is not a border attribute (want style/weight/color)", path, from, attr)
|
||||
}
|
||||
if err := setSideAttr(side, attr, av, from); err != nil {
|
||||
if err := setSideAttr(side, attr, obj[attr], from); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
@@ -522,11 +549,11 @@ func foldBorderFamilyAliases(in map[string]interface{}, path string) error {
|
||||
}
|
||||
}
|
||||
if sideKeyed {
|
||||
for side, sv := range obj {
|
||||
for _, side := range sortedKeys(obj) {
|
||||
if !sides[side] {
|
||||
return common.ValidationErrorf("%s.%s.%s is not a valid side (want top/bottom/left/right/all)", path, key, side)
|
||||
}
|
||||
if err := setSide(side, sv, key); err != nil {
|
||||
if err := setSide(side, obj[side], key); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
@@ -546,7 +573,7 @@ func foldBorderFamilyAliases(in map[string]interface{}, path string) error {
|
||||
delete(in, key)
|
||||
}
|
||||
}
|
||||
for attr := range attrs {
|
||||
for _, attr := range attrNames {
|
||||
for _, key := range []string{"border_" + side + "_" + attr, side + "_border_" + attr} {
|
||||
if v, has := in[key]; has {
|
||||
if err := setSideAttr(side, attr, v, key); err != nil {
|
||||
@@ -557,7 +584,7 @@ func foldBorderFamilyAliases(in map[string]interface{}, path string) error {
|
||||
}
|
||||
}
|
||||
}
|
||||
for attr := range attrs {
|
||||
for _, attr := range attrNames {
|
||||
key := "border_" + attr
|
||||
if v, has := in[key]; has {
|
||||
if err := setAllScalar(attr, v, key); err != nil {
|
||||
|
||||
@@ -480,3 +480,36 @@ func TestSingleIssueStillAttributesFlag(t *testing.T) {
|
||||
t.Error("single-issue aggregate should keep the underlying error as Cause")
|
||||
}
|
||||
}
|
||||
|
||||
// TestAggregatedIssuesKeepPrescriptions pins that folding per-item failures
|
||||
// into one flag error does not drop the Hint the inner error carried. The
|
||||
// prescriptions this domain adds (requireSheetSelector's "+workbook-info"
|
||||
// pointer, for one) live in Hint, and a Problem has exactly one Hint slot —
|
||||
// so a lone issue must inherit it and a folded list must inline each one.
|
||||
func TestAggregatedIssuesKeepPrescriptions(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
t.Run("single --writes issue inherits the inner hint", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
_, _, err := runShortcutCapturingErr(t, CellsSet, []string{
|
||||
"--url", testURL,
|
||||
"--writes", `[{"range":"A1","cells":[[{"value":1}]]}]`,
|
||||
})
|
||||
ve := requireValidation(t, err, "specify at least one of --sheet-id or --sheet-name")
|
||||
if !strings.Contains(ve.Hint, "+workbook-info") {
|
||||
t.Errorf("the inner prescription must survive the fold, got hint %q", ve.Hint)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("folded --writes issues inline each hint", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
_, _, err := runShortcutCapturingErr(t, CellsSet, []string{
|
||||
"--url", testURL,
|
||||
"--writes", `[{"range":"A1","cells":[[{"value":1}]]},{"range":"B1","cells":[[{"value":2}]]}]`,
|
||||
})
|
||||
ve := requireValidation(t, err, "--writes has 2 issues")
|
||||
if strings.Count(ve.Message, "+workbook-info") != 2 {
|
||||
t.Errorf("each issue should carry its own prescription inline, got %q", ve.Message)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user