Files
xiongyuanwen-byted c2afcce1b5 feat(sheets): accept the payload spellings the 08-29..31 reflow rejected (#2611)
* feat(sheets): accept the payload spellings the 08-29..31 reflow rejected

The 08-29..31 reflow report attributes 2839 failures across 31388 sheets
calls, and a large share of them carry well-formed intent in a spelling the
flag does not document: a numeric column whose cells arrived as JSON strings
(155), a row cut short at its last non-empty cell (25), `columns` written as
one object per column (35), the sub-sheet list without its {"sheets":...}
envelope (10), a quoted font_size (30). Each rejection cost a retry that
rewrote the same data differently, and each shape has exactly one reading.

Accept them, on the acceptance-layer contract already stated in
style_vocab.go: one canonical form documented, a wide layer undocumented, and
no rewrite whose meaning is in doubt. A non-numeric string in a numeric
column, a row wider than `columns`, and a column-less sheet that does carry
rows all still fail. `dtypes` additionally reads the writer's own type
vocabulary (number, date), which used to fall through to the string default
and silently write figures as text -- a wrong result rather than a rejection,
so it never surfaced as an error at all.

Three more commands get the same treatment. --properties on
+cond-format-create renames `operator` to compare_type under the {value} /
{text} shapes, canonicalizes symbol and abbreviation comparisons, stringifies
a numeric threshold to match the schema, joins a two-element between list,
and lifts a bare attrs object into the one-entry list; rules whose own
contract spells `operator` keep it. +csv-put answers to --data / --content /
--csv-file, and a path-shaped --csv value naming a real file is read as
@<path> would, reported in the result's warnings rather than silently --
requiring the prefix there was an inconsistency of that flag's own making,
since every sibling path-valued flag takes the same value bare. --font-line
takes the CSS text-decoration words.

Where a fix would have to guess, the error carries the fix instead:
+cells-get rejects an Excel multi-area range client-side with the enclosing
rectangle spelled out (54 rejections the backend answered with a bare
"invalid range"), and row_height / col_width / wrap / unmerge_cells inside a
cell_styles item name where they really live.

Everything stays inside shortcuts/sheets. Deliberately unchanged: shrinking
--range to fit --cells (the dimension check documents why), chunking a
--styles spec past the 100-operation cap (it would drop the atomicity of one
batch_update), and the server-side merged-region conflicts.

* feat(sheets): answer the reflow report's full error breakdown

The 08-29..31 reflow report gained two sections: a 58-row table covering
every long-tail command, and a per-command breakdown of the "other" buckets
its first pass had summarized as a count. Together they name ~1000 failures
the earlier commit could not see. This works through them on the same rule:
accept what has one reading, prescribe what does not, and leave alone what
needs a guess.

The one defect among them is on --cells. A cell carrying style fields at its
top level ({"value":"x","font_weight":"bold","border":{…}}) passed every
client check and reached the backend verbatim, which answered
`[cells[0][0].border] unexpected property "border" is not defined` -- 33
rejections on payloads the --styles path accepts, which is exactly the
vocabulary-parity break style_vocab.go's contract forbids. Style fields now
fold into the carrier that holds them (border into border_styles, scalars
into cell_styles) and their values canonicalize on the same pass, so a
boolean word_wrap no longer dies on the schema's "expected type string" one
step later. A key this domain does not know is still passed through: the
tool contract may gain fields between builds.

Accepted, each with the rejection count it removes: the two object spellings
of --writes, the {"writes":[…]} envelope and a lone write object (54);
`type` as the line-kind slot inside a border spec, which is the Lark OpenAPI
and openpyxl spelling (29); a blank column heading, which is a spacer column
or the cells under a merged title (13); blank text in a date column, mirroring
the numeric rule (7); `custom` as a row/col sizing type when the op carries a
size (Excel's and Lark's own UI word); and cell-style vocabulary inside a
conditional-format rule's style block, where background_color / font_color /
font_weight / font_line fold onto back_color / fore_color / font /
text_decoration (9).

Prescribed, where the fix changes the shape: --position on +dim-delete, whose
sibling +dim-insert does take it (18); --ranges on +cells-unmerge (7);
--include on +csv-get (3); the sheet selectors on +styles-put; --payload on
+table-put; a whole-column or whole-row range where a cell style needs a
rectangle, naming row_sizes / col_sizes as the carrier that does take that
form (28); and border keys written on a styles item instead of a cell_styles
entry. Four invented subcommands (+cells-get-style, +dropdown-list, +meta,
+sheets-list) name the command they meant.

Renamed silently, where the command already has the input under another
spelling: --title on +workbook-import (16), --file / --outdir on
+workbook-export (15), --replace on +cells-replace (7), --output on +csv-get,
plus --sheet-name on +sheet-create and --new-title on +sheet-rename.

Finally, the Excel multi-area --range moves from +cells-get's own Validate to
the shared --range chain: 5 more of those landed on +csv-get and 4 on
+cells-set-style, so the habit is not specific to reads. It is chained after
the sheet-prefix rewrite, so a sheet name containing a comma cannot be read
as several areas.

* feat(sheets): retry transient reads, and answer three stale-state failures

Four fixes from the reflow report's long-tail table that need no protocol
decision, in descending order of the rejections they remove.

A read tool call is now reissued up to twice on a transient failure. The
report's single largest cause on +csv-get was "API call failed: server time
out error" -- 25 of its 71 rejections -- with more on +cells-get and
+workbook-info, each on a command that was written correctly and succeeded
when the agent reissued it by hand. Reads only: this API has no idempotency
key, so a create that timed out after the backend committed it would be
committed twice, which is why the shared RetryTransport is installed with
MaxRetries at 0 and why this sits where the read/write classification is
already known. A rate limit is excluded even though the classifier marks it
retryable -- the server is asking for less traffic, and a fixed sub-second
backoff answers that by sending more.

+filter-create accepted its own documented contract only in the flag
description. "The flag is optional overall -- if omitted, an empty filter is
created on --range" was false: `rules` is required at the properties root, so
omitting --properties failed with `required property "rules" is missing`, an
error about a flag the caller deliberately did not pass. The empty rule set
is now the default, which is what a filter with no column conditions is.

A "not found" on an id-addressed update or delete now points at the list
command that reports the live ids. The backend answers with the id alone
("conditional format iXGbyDwC not found"), which reads like a transport
problem rather than a stale reference; +cond-format-update and
+cond-format-delete contributed 14 rejections between them, and the same
prescription covers every id-addressed object.

Not done here, and not because they are hard: the ~480 rejections behind the
four protocol decisions (a --title default, shrinking --range to fit --cells,
merged-region conflicts, chunking past the operation cap) are the caller's
call, not mine; the ~185 PowerShell argv failures are destroyed before the
process starts and can only be answered by the Windows reference; and write
retries need an idempotency key from the backend.

* feat(sheets): narrow an oversized --range, prescribe merge conflicts, split large style specs

Three of the four decisions the reflow report left open, as decided by the
caller. The fourth, defaulting --title from the first sub-sheet name, is
declined: the title is what the workbook is called in Drive, and deriving it
would produce a shelf of spreadsheets named after their first tab. That one is
answered in the skill reference instead.

--cells that FITS inside --range now narrows the write to the payload instead
of failing. The anchor is unchanged, so every cell lands where the caller put
it, and no cell outside the payload is touched -- the stated range only ever
said how far the caller thought the payload reached. The dominant shape is a
one-cell title against the range it will occupy once merged, where writing the
top-left is what a merged region needs anyway. A payload that OVERFLOWS its
range is still rejected: growing the range would write over cells nobody
named. The narrowing rides back in the success envelope's warnings, because
the caller did state an extent. 122 rejections.

A merged-cell rejection now carries the commands that resolve it. The backend
names the obstacle -- the top-left of the region a write landed inside, or the
0-based bounds of the region a merge would overlap -- but never in A1
notation, and never with the command that clears it; "[0,0-0,6]" becomes
`+cells-unmerge --range "A1:G1"`. The message is read, never used to rewrite
the request: auto-redirecting a write would put data where the caller did not
ask for it, and auto-unmerging would discard a merge nobody agreed to lose. A
parse that finds nothing adds no hint. 193 rejections, which stay failures but
become one-retry failures.

+styles-put splits a spec past the per-request cap instead of refusing it. The
100 was never a server contract -- it is our own materialization guard -- so
the fix is to keep each request at that known-good size and send several,
which is the same license coalesceStyleStamps already takes when it fuses
adjacent stamps and the same thing +table-put does when it slices a large
write. The whole-spec ceiling moves to 1000, the request count rides in the
result, and a mid-way failure names the chunks that already applied and says
re-running the whole spec is safe. --writes keeps its cap: it is one atomic
batch_update by contract. 48 rejections.

* fix(sheets): make the payload prescriptions correct on PowerShell

Every composite flag in this domain takes its payload three ways -- inline, as
a relative @file, or on stdin -- and the prescriptions named the third one in
a spelling PowerShell does not have. `--cells - < cells.json` fails there with
"The '<' operator is reserved for future use": an error about the shell, on a
line this CLI told the caller to run. The pipe is no better, since PowerShell
5 re-encodes non-ASCII on the way through and turns a CJK payload into the
same invalid JSON by another route; the windows-compat skill reference rules
both out, and this code was contradicting it.

On windows every prescription now lands on @file, quoted -- a bare @ opens a
splatting expression -- and says why inlining failed in the first place: the
shell splits a JSON argument on the quotes and commas inside it, and single
quotes do not prevent that. The caller quoted the argument and has no reason
to suspect the quoting, which is what made these 89 rejections cost more than
one retry apiece. POSIX shells keep the redirection form they do have.

The other half of that class arrived as "positional arguments are not
supported": once the quote stack is split, the tail of the JSON is a
positional argument. The framework's message ("pass values via flags") is
correct and useless there -- the caller DID pass a flag -- but it is one
message for every command on every platform, so the cause is named here
instead, where the domain knows both the shell and which of its flags carry a
payload big enough to split. Chained onto cobra's Args the same way the
unknown-flag and enum rewrites chain onto their hooks; non-windows keeps the
framework wording, where a positional argument usually is one. 35 rejections.

* fix(sheets): answer the review of the reflow leniency PR

Nine defects the review found in the acceptance layer, plus the lint /
deadcode failure that blocked CI.

Silent wrong values. A `between` threshold list of any length was joined
into a comma string, so one or three thresholds passed the local shape
check and failed at the backend. The literal "null" decodes into any
destination without an error and leaves it at the zero value: in a
numeric column it marshalled back out as 0, and under `font_size` it
became a zero-point font. Both now reject it. `outer` and `all` in one
border spec dropped `outer` even when the two named different boxes,
applying half the caller's intent; a duplicate is still folded away, a
conflict now reaches the invalid-side check.

Rejections the acceptance layer meant to absorb. The multi-area `--range`
prescription took its rectangle from the first and last area, so
"A3,J3,G3" prescribed "A3:G3" and dropped J3; it now spans every area on
both axes. `type:"custom"` was rewritten to pixel only when the op
carried `size`, so the `height` / `width` alias the same parser accepts
everywhere else failed the enum check first.

False reports. `+cells-set` reconstructed the stated range without the
payload, so every bare `--range A1` with a multi-cell payload reported
itself as narrowed. `+table-put --dry-run` skipped the style expansion in
append mode, hiding a style-only write the execute path performs.

Contract. `+styles-put` still promised one fail-fast batch request after
gaining chunking, and its partial-failure advice called a whole-spec
retry safe — replaying an applied `merge_cells` is rejected as an
overlap. Both now state that a large spec can partially apply, and the
merge route is to read back and resend only what did not land.

The windows positional-argument annotation flattened the framework's
error to text; it keeps it as the cause, and the annotation is split out
so its typed shape is asserted on any host rather than only on windows.

CI: `payloadStdinForm` was unreachable — only its `goos` variant is used.

* fix(sheets): answer the second review round

Six findings, one of them a revert.

P1, --csv. A path-shaped value naming a real file was read as @<path>
would. That makes the flag's meaning depend on the working directory: a
caller writing the literal "./data.csv" into a cell uploaded a same-named
local file instead, and learned of the substitution only from a warning
that arrived after the remote write. File intent has to be explicit, so
the read is gone and the existing prescription -- @<path> or stdin --
stands on its own again. The substitution machinery goes with it.

--outdir and --output-dir were aliased onto --output-path, which reads a
value as a directory only when one already exists there. "--outdir
./exports" with no ./exports therefore wrote a file named "exports".
Both spellings say directory in their own name, so they now carry that
intent through the alias-provenance channel already built for +csv-put's
--file, and applyWorkbookOutputPath honors it without probing disk.

Range narrowing reached only the standalone +cells-set. The narrowing
happens in the shared input builder, so a --writes item or a
+batch-update sub-op shipped the shrunken range in silence. The builder
now returns the note fitCellsRange already produced, and all three paths
report it; the standalone path stops reconstructing it from its flags.

A conditional-format style folded any existing `font` value that was not
the incoming word into "bold italic", so {"font":"normal",
"font_weight":"bold"} became valid input with a slant nobody wrote. Only
bold and italic combine now; anything else keeps its value for the schema
to reject.

The stale-object-ID hint matched any "not found", including one about the
sheet selector, and told the caller to refresh rule ids when the ids were
the one input that was right. It now requires the message to name the id
the command addressed, and skips sheet/workbook-scoped misses.

P3: a first-chunk failure in +styles-put reported "requests 1-0 already
applied". That case says nothing landed instead, without the merge
caveat, which does not apply when the sheet is untouched.

Co-authored-by: TRAE CLI <traecli@bytedance.com>

* fix(sheets): answer the third review round

Three follow-ups on the previous round, one of them a correction to it.

The narrowing note reached only the success envelope. A batch is
fail-fast but not transactional, and a transport failure leaves a single
write's outcome unknown, so a narrowed write can be on the sheet after an
error. Both branches now attach the note to the failure as well; a caller
reconciling against the range they stated rather than the one written is
exactly the confusion the note exists to prevent.

A `font` already reading "bold italic" sent the redundant flat spelling
along with it: the previous round stopped combining unrecognized values
but reached the same branch for this one, so the alias was left in the
outgoing style and the schema had no field for it. It is dropped now,
while a genuinely unknown `font` value still keeps its flat sibling
visible for the schema to reject.

The first-chunk case overcorrected. A failed FIRST request does not mean
an untouched sheet: operations before the failing one inside that request
stay applied, and a timeout settles nothing at all. Claiming otherwise
sent the caller into a whole-spec retry that can replay an applied
merge_cells. Only the backend's own "0 succeeded" now earns that message
-- the same test flattenToolErrorMsg uses, so the two cannot disagree
about what a failed batch left behind. Every other first-request failure
keeps the read-back route and the merge caveat.

Co-authored-by: TRAE CLI <traecli@bytedance.com>

---------

Co-authored-by: TRAE CLI <traecli@bytedance.com>
2026-09-07 21:29:15 +08:00

1106 lines
40 KiB
Go

// Copyright (c) 2026 Lark Technologies Pte. Ltd.
// SPDX-License-Identifier: MIT
package sheets
import (
"encoding/json"
"errors"
"fmt"
"os"
goruntime "runtime"
"strings"
"testing"
"github.com/larksuite/cli/errs"
"github.com/larksuite/cli/internal/cmdutil"
"github.com/larksuite/cli/shortcuts/common"
"github.com/spf13/cobra"
)
func TestUnknownFlagFromParseError(t *testing.T) {
t.Parallel()
cases := []struct {
in string
name string
ok bool
}{
{"unknown flag: --cols", "cols", true},
{"unknown flag: --with-styles", "with-styles", true},
{"unknown shorthand flag: 'z' in -z", "", false},
{"flag needs an argument: --find", "", false},
{`invalid argument "x" for "--count"`, "", false},
}
for _, c := range cases {
name, ok := unknownFlagFromParseError(errors.New(c.in))
if name != c.name || ok != c.ok {
t.Errorf("unknownFlagFromParseError(%q) = (%q,%v), want (%q,%v)", c.in, name, ok, c.name, c.ok)
}
}
}
// TestSheetsFlagErrorFunc_SemanticGuessListsValidFlags pins the sheets
// override of the root unknown-flag error: --cols is a semantic guess for
// --range that edit distance can't rank, so the hint must inline the full
// valid-flag list instead of deferring to a --help round trip.
func TestSheetsFlagErrorFunc_SemanticGuessListsValidFlags(t *testing.T) {
t.Parallel()
c := &cobra.Command{Use: "demo"}
c.Flags().String("range", "", "")
c.Flags().Int("width", 0, "")
err := sheetsFlagErrorFunc(c, errors.New("unknown flag: --cols"))
var verr *errs.ValidationError
if !errors.As(err, &verr) {
t.Fatalf("expected *errs.ValidationError, got %T", err)
}
if verr.Subtype != errs.SubtypeInvalidArgument {
t.Errorf("subtype = %q, want invalid_argument", verr.Subtype)
}
if len(verr.Params) != 1 || verr.Params[0].Name != "--cols" {
t.Errorf("Params = %v, want one entry named --cols", verr.Params)
}
if strings.Contains(verr.Hint, "--help") {
t.Errorf("hint should not defer to --help when flags fit inline, got %q", verr.Hint)
}
for _, want := range []string{"--range", "--width"} {
if !strings.Contains(verr.Hint, want) {
t.Errorf("hint should inline valid flag %s, got %q", want, verr.Hint)
}
}
}
// TestSheetsFlagErrorFunc_TypoKeepsSuggestion pins that the root behavior
// (did-you-mean suggestion, machine-readable Suggestions) is preserved by
// the sheets override, with the valid-flag list appended.
func TestSheetsFlagErrorFunc_TypoKeepsSuggestion(t *testing.T) {
t.Parallel()
c := &cobra.Command{Use: "demo"}
c.Flags().String("range", "", "")
c.Flags().Bool("dry-run", false, "")
err := sheetsFlagErrorFunc(c, errors.New("unknown flag: --rang"))
var verr *errs.ValidationError
if !errors.As(err, &verr) {
t.Fatalf("expected *errs.ValidationError, got %T", err)
}
found := false
for _, s := range verr.Params[0].Suggestions {
if s == "--range" {
found = true
}
}
if !found {
t.Errorf("Suggestions should include --range, got %v", verr.Params[0].Suggestions)
}
for _, want := range []string{"did you mean", "--range", "--dry-run"} {
if !strings.Contains(verr.Hint, want) {
t.Errorf("hint should contain %q, got %q", want, verr.Hint)
}
}
}
// TestSheetsFlagErrorFunc_BatchUpdateSheetLocator pins the targeted fix: a
// top-level --sheet-id / --sheet-name on +batch-update points the caller at
// the per-op locator contract instead of offering a misleading fuzzy guess.
func TestSheetsFlagErrorFunc_BatchUpdateSheetLocator(t *testing.T) {
t.Parallel()
for _, name := range []string{"sheet-id", "sheet-name", "sheet_id", "sheet_name"} {
t.Run(name, func(t *testing.T) {
t.Parallel()
c := &cobra.Command{Use: "+batch-update"}
c.Flags().String("operations", "", "")
err := sheetsFlagErrorFunc(c, errors.New("unknown flag: --"+name))
var verr *errs.ValidationError
if !errors.As(err, &verr) {
t.Fatalf("expected *errs.ValidationError, got %T", err)
}
if !strings.Contains(verr.Message, "put sheet_id/sheet_name inside each operation's input") {
t.Errorf("message should name the per-op locator contract, got %q", verr.Message)
}
if strings.Contains(verr.Hint, "did you mean") {
t.Errorf("must not offer a fuzzy guess here, got hint %q", verr.Hint)
}
if len(verr.Params) != 1 || verr.Params[0].Name != "--"+name {
t.Errorf("Params should carry the offending flag, got %v", verr.Params)
}
if len(verr.Params[0].Suggestions) != 0 {
t.Errorf("no suggestions expected, got %v", verr.Params[0].Suggestions)
}
})
}
}
// TestSheetsFlagErrorFunc_BatchUpdateOtherUnknownStillSuggests confirms the
// special case is scoped to the two sheet-locator flags: any other unknown
// flag on +batch-update keeps the normal did-you-mean behaviour.
func TestSheetsFlagErrorFunc_BatchUpdateOtherUnknownStillSuggests(t *testing.T) {
t.Parallel()
c := &cobra.Command{Use: "+batch-update"}
c.Flags().String("operations", "", "")
err := sheetsFlagErrorFunc(c, errors.New("unknown flag: --operation"))
var verr *errs.ValidationError
if !errors.As(err, &verr) {
t.Fatalf("expected *errs.ValidationError, got %T", err)
}
if strings.Contains(verr.Message, "no top-level sheet locator") {
t.Errorf("non-locator unknown flag must not hit the special case, got %q", verr.Message)
}
}
func TestSheetsFlagErrorFunc_OtherErrorStaysGeneric(t *testing.T) {
t.Parallel()
c := &cobra.Command{Use: "demo"}
err := sheetsFlagErrorFunc(c, errors.New("flag needs an argument: --find"))
var verr *errs.ValidationError
if !errors.As(err, &verr) {
t.Fatalf("expected *errs.ValidationError, got %T", err)
}
if verr.Param != "" || len(verr.Params) != 0 {
t.Errorf("Param=%q Params=%v, want both empty for generic flag error", verr.Param, verr.Params)
}
if strings.Contains(verr.Hint, "did you mean") {
t.Errorf("generic flag error must not produce a did-you-mean hint, got %q", verr.Hint)
}
}
func TestInlineFlagList_TruncatesPastLimit(t *testing.T) {
t.Parallel()
if got := inlineFlagList(nil); got != "" {
t.Errorf("inlineFlagList(nil) = %q, want empty", got)
}
names := make([]string, inlineFlagListLimit+5)
for i := range names {
names[i] = fmt.Sprintf("flag-%02d", i)
}
got := inlineFlagList(names)
if !strings.Contains(got, "5 more") || !strings.Contains(got, "--help") {
t.Errorf("truncated list should count the overflow and defer to --help, got %q", got)
}
if strings.Contains(got, names[inlineFlagListLimit]) {
t.Errorf("list should stop at the limit, got %q", got)
}
}
func TestCanonicalEnumValue(t *testing.T) {
t.Parallel()
cases := []struct {
val string
enum []string
want string
}{
{"SUM", []string{"sum", "count"}, "sum"}, // casing
{"center", []string{"top", "middle", "bottom"}, "middle"}, // alias: CSS vertical center
{"middle", []string{"left", "center", "right"}, "center"}, // alias: horizontal middle
{"overwite", []string{"append", "overwrite"}, ""}, // typo is NOT canonical
{"delete", []string{"append", "overwrite"}, ""}, // nothing close
}
for _, c := range cases {
if got := canonicalEnumValue(c.val, c.enum); got != c.want {
t.Errorf("canonicalEnumValue(%q, %v) = %q, want %q", c.val, c.enum, got, c.want)
}
}
}
func TestClosestEnumValue(t *testing.T) {
t.Parallel()
cases := []struct {
val string
enum []string
want string
}{
{"SUM", []string{"sum", "count"}, "sum"}, // casing
{"center", []string{"top", "middle", "bottom"}, "middle"}, // alias
{"overwite", []string{"append", "overwrite"}, "overwrite"}, // edit distance
{"delete", []string{"append", "overwrite"}, ""}, // nothing close
}
for _, c := range cases {
if got := closestEnumValue(c.val, c.enum); got != c.want {
t.Errorf("closestEnumValue(%q, %v) = %q, want %q", c.val, c.enum, got, c.want)
}
}
}
// TestChainEnumNormalization_UnitContract pins the PreRunE stage in
// isolation: canonical vocabulary is auto-applied, typos error with a
// suggestion (never applied), the framework PreRunE keeps running first,
// and --print-schema skips enum gating entirely.
func TestChainEnumNormalization_UnitContract(t *testing.T) {
t.Parallel()
newCmd := func() (*cobra.Command, *bool) {
cmd := &cobra.Command{Use: "+cells-set-style"}
cmd.Flags().String("vertical-alignment", "", "")
cmd.Flags().Bool("print-schema", false, "")
prevCalled := false
cmd.PreRunE = func(*cobra.Command, []string) error {
prevCalled = true
return nil
}
chainEnumNormalization(cmd)
return cmd, &prevCalled
}
// Alias auto-applied, framework PreRunE preserved.
cmd, prevCalled := newCmd()
cmd.Flags().Set("vertical-alignment", "center")
if err := cmd.PreRunE(cmd, nil); err != nil {
t.Fatalf("center should normalize and pass, got: %v", err)
}
if got, _ := cmd.Flags().GetString("vertical-alignment"); got != "middle" {
t.Errorf("vertical-alignment = %q, want rewritten to %q", got, "middle")
}
if !*prevCalled {
t.Error("framework PreRunE must keep running first")
}
// Typo: error with suggestion, value untouched.
cmd, _ = newCmd()
cmd.Flags().Set("vertical-alignment", "botom")
err := cmd.PreRunE(cmd, nil)
var verr *errs.ValidationError
if !errors.As(err, &verr) {
t.Fatalf("typo should fail with *errs.ValidationError, got %T: %v", err, err)
}
if !strings.Contains(verr.Hint, `"bottom"`) {
t.Errorf("hint should suggest bottom for the typo, got %q", verr.Hint)
}
if got, _ := cmd.Flags().GetString("vertical-alignment"); got != "botom" {
t.Errorf("typo must not be rewritten, got %q", got)
}
// --print-schema skips enum gating (pure local introspection).
cmd, _ = newCmd()
cmd.Flags().Set("vertical-alignment", "not-a-value")
cmd.Flags().Set("print-schema", "true")
if err := cmd.PreRunE(cmd, nil); err != nil {
t.Errorf("--print-schema must skip enum gating, got: %v", err)
}
}
// shortcutFromRegistry returns the fully wired shortcut (PostMount
// ergonomics included) as Shortcuts() exposes it to the framework.
func shortcutFromRegistry(t *testing.T, command string) common.Shortcut {
t.Helper()
for _, sc := range Shortcuts() {
if sc.Command == command {
return sc
}
}
t.Fatalf("shortcut %q not found in Shortcuts()", command)
return common.Shortcut{}
}
// TestShortcuts_FlagErgonomicsMounted verifies the ergonomics ride every
// mounted sheets command end-to-end: enum vocabulary normalizes on a real
// invocation, and unknown flags answer with the inlined valid-flag list.
func TestShortcuts_FlagErgonomicsMounted(t *testing.T) {
t.Parallel()
t.Run("enum alias normalizes through a real run", func(t *testing.T) {
t.Parallel()
sc := shortcutFromRegistry(t, "+cells-set-style")
stdout, _, err := runShortcutCapturingErr(t, sc, []string{
"--url", testURL,
"--sheet-name", "s",
"--range", "A1:A1",
"--vertical-alignment", "center",
"--dry-run",
})
if err != nil {
t.Fatalf("center should normalize to middle and pass, got: %v", err)
}
if !strings.Contains(stdout, "middle") || strings.Contains(stdout, "center") {
t.Errorf("dry-run body should carry the normalized value, got %q", stdout)
}
})
t.Run("enum typo errors with suggestion", func(t *testing.T) {
t.Parallel()
sc := shortcutFromRegistry(t, "+cells-set-style")
_, _, err := runShortcutCapturingErr(t, sc, []string{
"--url", testURL,
"--sheet-name", "s",
"--range", "A1:A1",
"--vertical-alignment", "botom",
"--dry-run",
})
ve := requireValidation(t, err, `invalid value "botom" for --vertical-alignment`)
if !strings.Contains(ve.Hint, `"bottom"`) {
t.Errorf("hint should suggest bottom, got %q", ve.Hint)
}
})
t.Run("unknown flag inlines valid flags", func(t *testing.T) {
t.Parallel()
sc := shortcutFromRegistry(t, "+cols-resize")
_, _, err := runShortcutCapturingErr(t, sc, []string{
"--url", testURL,
"--sheet-name", "s",
"--col-size", "A:D",
})
ve := requireValidation(t, err, `unknown flag "--col-size"`)
for _, want := range []string{"valid flags:", "--range", "--width", "--widths"} {
if !strings.Contains(ve.Hint, want) {
t.Errorf("hint should contain %q, got %q", want, ve.Hint)
}
}
})
}
// TestShortcuts_IntuitiveFlagAliases verifies the silent-alias tier: a
// habitual name with identical value semantics parses as the real flag on a
// mounted command, costing zero round trips (eval: --cols, --file, --name,
// --source/--target each burned an unknown-flag failure plus a --help call).
func TestShortcuts_IntuitiveFlagAliases(t *testing.T) {
t.Parallel()
t.Run("cols-resize --cols parses as --range", func(t *testing.T) {
t.Parallel()
sc := shortcutFromRegistry(t, "+cols-resize")
stdout, _, err := runShortcutCapturingErr(t, sc, []string{
"--url", testURL,
"--sheet-name", "s",
"--cols", "A:D",
"--width", "100",
"--dry-run",
})
if err != nil {
t.Fatalf("--cols should alias to --range and pass, got: %v", err)
}
if !strings.Contains(stdout, "A:D") {
t.Errorf("dry-run body should carry the aliased range, got %q", stdout)
}
})
t.Run("sheet-create --name parses as --title", func(t *testing.T) {
t.Parallel()
sc := shortcutFromRegistry(t, "+sheet-create")
stdout, _, err := runShortcutCapturingErr(t, sc, []string{
"--url", testURL,
"--name", "汇总",
"--dry-run",
})
if err != nil {
t.Fatalf("--name should alias to --title and pass, got: %v", err)
}
if !strings.Contains(stdout, "汇总") {
t.Errorf("dry-run body should carry the aliased title, got %q", stdout)
}
})
t.Run("sheet-rename --new-name parses as --title", func(t *testing.T) {
t.Parallel()
sc := shortcutFromRegistry(t, "+sheet-rename")
stdout, _, err := runShortcutCapturingErr(t, sc, []string{
"--url", testURL,
"--sheet-name", "s",
"--new-name", "授权需求清单",
"--dry-run",
})
if err != nil {
t.Fatalf("--new-name should alias to --title and pass, got: %v", err)
}
if !strings.Contains(stdout, "授权需求清单") {
t.Errorf("dry-run body should carry the aliased title, got %q", stdout)
}
})
t.Run("range-fill --source/--target parse as ranges", func(t *testing.T) {
t.Parallel()
sc := shortcutFromRegistry(t, "+range-fill")
stdout, _, err := runShortcutCapturingErr(t, sc, []string{
"--url", testURL,
"--sheet-name", "s",
"--source", "B2",
"--target", "B3:B10",
"--dry-run",
})
if err != nil {
t.Fatalf("--source/--target should alias to the -range flags, got: %v", err)
}
for _, want := range []string{"B2", "B3:B10"} {
if !strings.Contains(stdout, want) {
t.Errorf("dry-run body should carry %q, got %q", want, stdout)
}
}
})
t.Run("csv-put --file parses as --csv", func(t *testing.T) {
t.Parallel()
sc := shortcutFromRegistry(t, "+csv-put")
stdout, _, err := runShortcutCapturingErr(t, sc, []string{
"--url", testURL,
"--sheet-name", "s",
"--start-cell", "A1",
"--file", "a,b\n1,2",
"--dry-run",
})
if err != nil {
t.Fatalf("--file with CSV text should alias to --csv and pass, got: %v", err)
}
if !strings.Contains(stdout, "a,b") {
t.Errorf("dry-run body should carry the CSV text, got %q", stdout)
}
})
t.Run("cols-resize --size parses as --width", func(t *testing.T) {
t.Parallel()
sc := shortcutFromRegistry(t, "+cols-resize")
stdout, _, err := runShortcutCapturingErr(t, sc, []string{
"--url", testURL,
"--sheet-name", "s",
"--range", "A:C",
"--size", "120",
"--dry-run",
})
if err != nil {
t.Fatalf("--size should alias to --width (styles-protocol vocabulary), got: %v", err)
}
if !strings.Contains(stdout, "120") {
t.Errorf("dry-run body should carry the pixel width 120, got %q", stdout)
}
})
t.Run("rows-resize --size parses as --height", func(t *testing.T) {
t.Parallel()
sc := shortcutFromRegistry(t, "+rows-resize")
_, _, err := runShortcutCapturingErr(t, sc, []string{
"--url", testURL,
"--sheet-name", "s",
"--range", "1:3",
"--size", "36",
"--dry-run",
})
if err != nil {
t.Fatalf("--size should alias to --height (styles-protocol vocabulary), got: %v", err)
}
})
t.Run("cells-set --values parses as --cells", func(t *testing.T) {
t.Parallel()
sc := shortcutFromRegistry(t, "+cells-set")
stdout, _, err := runShortcutCapturingErr(t, sc, []string{
"--url", testURL,
"--sheet-name", "s",
"--range", "C1",
// The gspread payload verbatim: a plain values matrix, not cell
// objects. It rides through unchanged because the alias fixes the
// name and normalizeCellsFlagValue lifts the bare scalar — the two
// halves have to hold together for the silent tier to be correct.
"--values", `[["工作内容"]]`,
"--dry-run",
})
if err != nil {
t.Fatalf("--values should alias to --cells and pass, got: %v", err)
}
input := decodeToolInput(t, decodeDryRunFirstCall(t, stdout), "set_cell_range")
cells, _ := json.Marshal(input["cells"])
if string(cells) != `[[{"value":"工作内容"}]]` {
t.Errorf("cells = %s, want the lifted [[{\"value\":\"工作内容\"}]]", cells)
}
})
t.Run("alias never shadows a registered flag", func(t *testing.T) {
t.Parallel()
c := &cobra.Command{Use: "+csv-put"}
c.Flags().String("csv", "", "")
c.Flags().String("file", "", "") // hypothetical real flag wins
chainFlagAliases(c)
if err := c.ParseFlags([]string{"--file", "x"}); err != nil {
t.Fatalf("parse: %v", err)
}
if got, _ := c.Flags().GetString("file"); got != "x" {
t.Errorf("registered --file should keep its own value, got %q", got)
}
if got, _ := c.Flags().GetString("csv"); got != "" {
t.Errorf("--csv must stay empty when --file is a real flag, got %q", got)
}
})
}
func TestMapFlagView_AliasResolutionIsDeterministic(t *testing.T) {
t.Parallel()
t.Run("multiple aliases use stable lexical precedence", func(t *testing.T) {
t.Parallel()
fv := newMapFlagViewForCommand("+sheet-rename", map[string]interface{}{
"name": "first",
"new-name": "second",
})
for range 100 {
if got := fv.Str("title"); got != "first" {
t.Fatalf("title = %q, want stable alias value %q", got, "first")
}
}
})
t.Run("underscored canonical name resolves alias", func(t *testing.T) {
t.Parallel()
fv := newMapFlagViewForCommand("+chart-config-update", map[string]interface{}{
"x-axis": "Month",
})
if got := fv.Str("x_axis_title"); got != "Month" {
t.Fatalf("x_axis_title = %q, want %q", got, "Month")
}
})
}
// TestShortcuts_IntuitiveFlagHints verifies the prescription tier: habitual
// names whose fix is not a rename answer with the exact correct form, so the
// retry needs no --help round trip (eval: +sheet-copy burned 3/3 post-error
// --help calls, +dim-insert kept failing even after reading help).
func TestShortcuts_IntuitiveFlagHints(t *testing.T) {
t.Parallel()
cases := []struct {
command string
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",
args: []string{"--url", testURL, "--sheet-name", "s", "--dimension", "row"},
wrong: "--dimension",
wantHint: []string{"--position", "--count"},
},
{
command: "+dim-freeze",
args: []string{"--url", testURL, "--sheet-name", "s", "--frozen-rows", "2"},
wrong: "--frozen-rows",
// 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",
args: []string{"--url", testURL, "--sheet-name", "s", "--range", "A1", "--bold", "true"},
wrong: "--bold",
wantHint: []string{"--font-weight bold"},
},
{
command: "+sheet-copy",
args: []string{"--url", testURL, "--sheet-name", "s", "--new-sheet-name", "副本"},
wrong: "--new-sheet-name",
wantHint: []string{"--title", "source sheet"},
},
{
command: "+table-put",
args: []string{"--url", testURL, "--sheets", "{}", "--start-cell", "B2"},
wrong: "--start-cell",
wantHint: []string{`"start_cell"`, "+csv-put"},
},
{
command: "+dim-freeze",
args: []string{"--url", testURL, "--sheet-name", "s", "--frozen-row-count", "1"},
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{"--rows N"},
rejectHint: []string{"--dimension", "--count"},
},
{
command: "+cells-set-style",
args: []string{"--url", testURL, "--sheet-name", "s", "--range", "A1", "--font-bold", "true"},
wrong: "--font-bold",
wantHint: []string{"--font-weight bold"},
},
{
command: "+cells-set-style",
args: []string{"--url", testURL, "--sheet-name", "s", "--range", "A1", "--bg-color", "#FFF"},
wrong: "--bg-color",
wantHint: []string{"--background-color"},
},
{
command: "+cells-set-style",
args: []string{"--url", testURL, "--sheet-name", "s", "--range", "A1", "--wrap-strategy", "overflow"},
wrong: "--wrap-strategy",
wantHint: []string{"--word-wrap"},
},
{
command: "+cells-set-style",
args: []string{"--url", testURL, "--sheet-name", "s", "--range", "A1", "--border-all", "thin"},
wrong: "--border-all",
wantHint: []string{"--border-styles", `"all"`},
},
{
command: "+cells-set-style",
args: []string{"--url", testURL, "--sheet-name", "s", "--range", "A1", "--border-top", "thin"},
wrong: "--border-top",
wantHint: []string{"--border-styles", `"top"`},
},
{
command: "+cells-set-style",
args: []string{"--url", testURL, "--sheet-name", "s", "--range", "A1", "--border-color", "#000"},
wrong: "--border-color",
wantHint: []string{"--border-styles", "color"},
},
{
command: "+chart-create-basic",
args: []string{"--url", testURL, "--sheet-name", "s", "--position", "F2"},
wrong: "--position",
wantHint: []string{"--anchor-cell F2", "--width", "--height"},
},
{
command: "+chart-create-basic",
args: []string{"--url", testURL, "--sheet-name", "s", "--show-labels", "true"},
wrong: "--show-labels",
wantHint: []string{"--data-labels value", "value_category_percentage", "series", "none"},
},
{
command: "+chart-config-update",
args: []string{"--url", testURL, "--sheet-name", "s", "--show-labels", "true"},
wrong: "--show-labels",
wantHint: []string{"--data-labels value", "value_category_percentage", "series", "none"},
},
}
for _, tc := range cases {
t.Run(tc.command+" "+tc.wrong, func(t *testing.T) {
t.Parallel()
sc := shortcutFromRegistry(t, tc.command)
_, _, err := runShortcutCapturingErr(t, sc, tc.args)
ve := requireValidation(t, err, "unknown flag \""+tc.wrong+"\"")
for _, want := range tc.wantHint {
if !strings.Contains(ve.Hint, want) {
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).
for _, p := range ve.Params {
if len(p.Suggestions) > 0 {
t.Errorf("curated prescription should drop edit-distance suggestions, got %v", p.Suggestions)
}
}
})
}
}
// TestShortcuts_RequiredFlagsMarkedInHelp pins the required marker on the
// mounted commands. It is a sheets-local decoration (chainRequiredFlagHelp),
// so the assertions live here rather than against the framework: cobra renders
// no marker of its own, and the two commands that relax the annotation after
// mounting must still be answered from flag-defs.
func TestShortcuts_RequiredFlagsMarkedInHelp(t *testing.T) {
t.Parallel()
usageOf := func(t *testing.T, command, flag string) string {
t.Helper()
parent, _, _, _ := newTestRig(t, shortcutFromRegistry(t, command))
cmd, _, err := parent.Find([]string{command})
if err != nil {
t.Fatalf("Find(%q) error = %v", command, err)
}
fl := cmd.Flags().Lookup(flag)
if fl == nil {
t.Fatalf("%s has no --%s", command, flag)
}
return fl.Usage
}
t.Run("a required flag says so", func(t *testing.T) {
t.Parallel()
if got := usageOf(t, "+workbook-create", "title"); !strings.HasPrefix(got, "(required) ") {
t.Errorf("--title usage = %q, want the required marker", got)
}
})
t.Run("an optional flag is left alone", func(t *testing.T) {
t.Parallel()
if got := usageOf(t, "+workbook-create", "folder-token"); strings.Contains(got, "(required)") {
t.Errorf("--folder-token usage = %q, want no required marker", got)
}
})
t.Run("a relaxed-but-still-required flag says so", func(t *testing.T) {
t.Parallel()
// +chart-create clears the cobra annotation so --print-example can run
// without it; --properties is still required on every other path.
if got := usageOf(t, "+chart-create", "properties"); !strings.HasPrefix(got, "(required) ") {
t.Errorf("--properties usage = %q, want the required marker", got)
}
})
t.Run("a one-required pair marks neither member", func(t *testing.T) {
t.Parallel()
// +csv-put takes --start-cell OR its --range alias, so neither is
// individually required — saying otherwise would be a false statement.
for _, flag := range []string{"start-cell", "range"} {
if got := usageOf(t, "+csv-put", flag); strings.Contains(got, "(required)") {
t.Errorf("--%s usage = %q, want no required marker", flag, got)
}
}
if got := usageOf(t, "+csv-put", "csv"); !strings.HasPrefix(got, "(required) ") {
t.Errorf("--csv usage = %q, want the required marker", got)
}
})
}
// TestCsvPut_FileAliasProvenance pins which spelling a value is attributed to
// when both are on one command line. The record is committed by the flag's
// Value on each real occurrence, so the LAST occurrence wins — pflag also
// normalizes names on Lookup and Set (the framework's own input resolution
// looks --csv up before Validate runs), and attributing those would either
// lose a legitimate --file or steal an explicit --csv.
func TestCsvPut_FileAliasProvenance(t *testing.T) {
dir := t.TempDir()
cmdutil.TestChdir(t, dir)
if err := os.WriteFile("data.csv", []byte("a,b\n1,2\n"), 0644); err != nil {
t.Fatal(err)
}
// Not path-shaped, so --csv never reads it on its own (see
// csvValueLooksLikePath): the probe for "which spelling supplied this".
if err := os.WriteFile("notes.txt", []byte("a,b\n1,2\n"), 0644); err != nil {
t.Fatal(err)
}
run := func(t *testing.T, extra ...string) (string, error) {
t.Helper()
args := append([]string{"--url", testURL, "--sheet-name", "s", "--start-cell", "A1"}, extra...)
stdout, _, err := runShortcutCapturingErr(t, shortcutFromRegistry(t, "+csv-put"), append(args, "--dry-run"))
return stdout, err
}
t.Run("--file alone reads the file", func(t *testing.T) {
stdout, err := run(t, "--file", "./data.csv")
if err != nil {
t.Fatalf("--file should read the path, got: %v", err)
}
if !strings.Contains(stdout, `a,b`) {
t.Errorf("dry-run body should carry the file contents, got %q", stdout)
}
})
t.Run("a later --csv occurrence keeps its own semantics", func(t *testing.T) {
// The last occurrence supplied the value and it was typed --csv, so the
// name must hit the --csv guard rather than being read as a file. The
// probe is a name --csv does NOT read on its own: a path-shaped one is
// now read under either spelling, which is what makes the two
// semantics observable only here.
_, err := run(t, "--file", "notes.txt", "--csv", "notes.txt")
requireValidation(t, err, "is an existing file, not inline CSV")
})
t.Run("a later --file occurrence reads the file", func(t *testing.T) {
stdout, err := run(t, "--csv", "./data.csv", "--file", "./data.csv")
if err != nil {
t.Fatalf("the last occurrence was --file, so it should read the path, got: %v", err)
}
if !strings.Contains(stdout, `a,b`) {
t.Errorf("dry-run body should carry the file contents, got %q", stdout)
}
})
}
// TestCsvPut_FileAliasProvenance_DoubleMount pins the staging guard. The
// ergonomics chain looks its aliases up while installing, and pflag normalizes
// a name on Lookup — so composing PostMount twice replays "file" through an
// already-installed normalizer at mount time. Without the Parsed() guard that
// arms the pending spelling before parsing starts, and the next real --csv
// occurrence commits it: an explicit --csv path would be read from disk instead
// of meeting its guard.
func TestCsvPut_FileAliasProvenance_DoubleMount(t *testing.T) {
dir := t.TempDir()
cmdutil.TestChdir(t, dir)
if err := os.WriteFile("data.csv", []byte("a,b\n1,2\n"), 0644); err != nil {
t.Fatal(err)
}
// Not path-shaped, so --csv never reads it on its own (see
// csvValueLooksLikePath): the probe for "which spelling supplied this".
if err := os.WriteFile("notes.txt", []byte("a,b\n1,2\n"), 0644); err != nil {
t.Fatal(err)
}
t.Run("mounted twice before parsing", func(t *testing.T) {
sc := shortcutFromRegistry(t, "+csv-put")
sc.PostMount = withFlagErgonomics(sc.PostMount) // a second, redundant pass
_, _, err := runShortcutCapturingErr(t, sc, []string{
"--url", testURL, "--sheet-name", "s", "--start-cell", "A1",
"--csv", "notes.txt", "--dry-run",
})
requireValidation(t, err, "is an existing file, not inline CSV")
})
t.Run("remounted after a parse", func(t *testing.T) {
// Parsed() stays true once parsing has started, so a remount at that
// point can stage through the normalizer the first pass installed.
// Re-running the install resets staging, which is what keeps the next
// parse's --csv occurrence from inheriting it.
parent, _, _, _ := newTestRig(t, shortcutFromRegistry(t, "+csv-put"))
parent.SetArgs([]string{"+csv-put", "--url", testURL, "--sheet-name", "s",
"--start-cell", "A1", "--file", "./data.csv", "--dry-run"})
if err := parent.Execute(); err != nil {
t.Fatalf("first run: %v", err)
}
cmd, _, err := parent.Find([]string{"+csv-put"})
if err != nil {
t.Fatalf("Find: %v", err)
}
withFlagErgonomics(nil)(cmd)
parent.SetArgs([]string{"+csv-put", "--url", testURL, "--sheet-name", "s",
"--start-cell", "A1", "--csv", "notes.txt", "--dry-run"})
requireValidation(t, parent.Execute(), "is an existing file, not inline CSV")
})
}
// TestReflowFlagVocabulary pins the flag-name and enum-value acceptances added
// after the 08-29..31 reflow: the payload-flag names +csv-put answers to, the
// CSS text-decoration words for --font-line, and the wrap / style
// prescriptions on +cells-set-style.
func TestReflowFlagVocabulary(t *testing.T) {
t.Parallel()
t.Run("csv-put payload flag names", func(t *testing.T) {
t.Parallel()
for _, name := range []string{"--csv", "--data", "--content"} {
t.Run(name, func(t *testing.T) {
t.Parallel()
stdout, _, err := runShortcutCapturingErr(t, shortcutFromRegistry(t, "+csv-put"), []string{
"--url", testURL, "--sheet-name", "s", "--start-cell", "A1",
name, "a,b\n1,2", "--dry-run",
})
if err != nil {
t.Fatalf("%s should carry the CSV payload, got: %v", name, err)
}
if !strings.Contains(stdout, `a,b`) {
t.Errorf("dry-run body should carry the CSV, got %q", stdout)
}
})
}
})
t.Run("csv-file names a path and reports itself by that name", func(t *testing.T) {
t.Parallel()
_, _, err := runShortcutCapturingErr(t, shortcutFromRegistry(t, "+csv-put"), []string{
"--url", testURL, "--sheet-name", "s", "--start-cell", "A1",
"--csv-file", "./missing.csv", "--dry-run",
})
ve := requireValidation(t, err, "names no file under the current directory")
if !strings.Contains(ve.Message, "--csv-file") {
t.Errorf("the error should name the spelling the caller typed, got %q", ve.Message)
}
})
t.Run("font-line takes the CSS text-decoration words", func(t *testing.T) {
t.Parallel()
for value, want := range map[string]string{
"strikethrough": "line-through",
"strike": "line-through",
"underlined": "underline",
} {
t.Run(value, func(t *testing.T) {
t.Parallel()
stdout, _, err := runShortcutCapturingErr(t, shortcutFromRegistry(t, "+cells-set-style"), []string{
"--url", testURL, "--sheet-name", "s", "--range", "A1:A10",
"--font-line", value, "--dry-run",
})
if err != nil {
t.Fatalf("%s should normalize, got: %v", value, err)
}
if !strings.Contains(strings.ReplaceAll(stdout, `\"`, `"`), `"font_line":"`+want+`"`) {
t.Errorf("body should carry font_line %q, got %q", want, stdout)
}
})
}
})
t.Run("unsupported font-line values still fail with the enum", func(t *testing.T) {
t.Parallel()
_, _, err := runShortcutCapturingErr(t, shortcutFromRegistry(t, "+cells-set-style"), []string{
"--url", testURL, "--sheet-name", "s", "--range", "A1:A10",
"--font-line", "wavy", "--dry-run",
})
requireValidation(t, err, "allowed: none, underline, line-through")
})
t.Run("wrap and style spellings get prescriptions", func(t *testing.T) {
t.Parallel()
for _, tc := range []struct{ flag, want string }{
{"--wrap-text", "use --word-wrap"},
{"--text-wrap", "use --word-wrap"},
{"--style", "there is no single --style flag"},
} {
t.Run(tc.flag, func(t *testing.T) {
t.Parallel()
_, _, err := runShortcutCapturingErr(t, shortcutFromRegistry(t, "+cells-set-style"), []string{
"--url", testURL, "--sheet-name", "s", "--range", "A1:A10",
tc.flag, "x", "--dry-run",
})
ve := requireValidation(t, err, "unknown flag")
if !strings.Contains(ve.Hint, tc.want) {
t.Errorf("hint should carry %q, got %q", tc.want, ve.Hint)
}
})
}
})
}
// TestReflowLongTailVocabulary pins the long-tail table from the 08-29..31
// reflow: names that already exist on the command under another spelling get
// aliased, names whose fix changes the shape get a prescription, and a
// multi-area --range is answered on every command that takes one.
func TestReflowLongTailVocabulary(t *testing.T) {
t.Parallel()
t.Run("aliases", func(t *testing.T) {
t.Parallel()
for _, tc := range []struct {
name string
command string
args []string
}{
{"workbook-import names the new spreadsheet", "+workbook-import",
[]string{"--file", "./data.csv", "--title", "Q3"}},
{"workbook-export names the local destination", "+workbook-export",
[]string{"--url", testURL, "--file", "./out.xlsx"}},
{"cells-replace names the replacement text", "+cells-replace",
[]string{"--url", testURL, "--sheet-name", "s", "--find", "a", "--replace", "b"}},
} {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
if _, _, err := runShortcutCapturingErr(t, shortcutFromRegistry(t, tc.command),
append(tc.args, "--dry-run")); err != nil {
t.Fatalf("the habitual spelling should parse, got: %v", err)
}
})
}
})
t.Run("prescriptions", func(t *testing.T) {
t.Parallel()
for _, tc := range []struct {
name, command, want string
args []string
}{
// 18 rejections, the largest long-tail entry: +dim-insert does
// take --position, so the habit carries to its sibling.
{"dim-delete has no position", "+dim-delete", `--range: "3:5" deletes rows`,
[]string{"--url", testURL, "--sheet-name", "s", "--position", "3"}},
{"cells-unmerge takes one span", "+cells-unmerge", "one span per call",
[]string{"--url", testURL, "--sheet-name", "s", "--ranges", `["A1:B2"]`}},
{"csv-get returns values only", "+csv-get", "+cells-get --include",
[]string{"--url", testURL, "--sheet-name", "s", "--range", "A1:B2", "--include", "formula"}},
{"styles-put has no sheet selector", "+styles-put", "no sheet selector",
[]string{"--url", testURL, "--sheet-name", "s", "--styles", `{"styles":[]}`}},
} {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
_, _, err := runShortcutCapturingErr(t, shortcutFromRegistry(t, tc.command),
append(tc.args, "--dry-run"))
ve := requireValidation(t, err, "unknown flag")
if !strings.Contains(ve.Hint, tc.want) {
t.Errorf("hint should carry %q, got %q", tc.want, ve.Hint)
}
})
}
})
t.Run("a multi-area range is answered on write commands too", func(t *testing.T) {
t.Parallel()
// The habit is not specific to reads: 5 more on +csv-get and 4 on
// +cells-set-style, which is why the check rides the --range chain.
for _, tc := range []struct {
command string
args []string
}{
{"+cells-set-style", []string{"--url", testURL, "--sheet-name", "s", "--range", "K14,K19", "--font-weight", "bold"}},
{"+csv-get", []string{"--url", testURL, "--sheet-name", "s", "--range", "A5:B11, AJ5:AP11"}},
} {
t.Run(tc.command, func(t *testing.T) {
t.Parallel()
_, _, err := runShortcutCapturingErr(t, shortcutFromRegistry(t, tc.command),
append(tc.args, "--dry-run"))
requireValidation(t, err, "separate areas")
})
}
})
}
// TestPositionalArgsCauseIsWindowsOnly pins the platform split on the
// positional-argument prescription: the cause it names is a PowerShell one,
// so it must not appear where a positional argument really is one.
func TestPositionalArgsCauseIsWindowsOnly(t *testing.T) {
t.Parallel()
_, _, err := runShortcutCapturingErr(t, shortcutFromRegistry(t, "+cells-set"), []string{
"--url", testURL, "--sheet-name", "s", "--range", "A1",
"--cells", `[[{"value":"x"}]]`, "stray", "--dry-run",
})
if err == nil {
t.Fatal("a positional argument should still be rejected")
}
if goruntime.GOOS == "windows" {
p, ok := errs.ProblemOf(err)
if !ok || !strings.Contains(p.Hint, "PowerShell") {
t.Errorf("windows should name the shell as the cause, got %v", err)
}
return
}
if strings.Contains(err.Error(), "PowerShell") {
t.Errorf("non-windows must keep the framework wording, got %v", err)
}
}
// TestPositionalArgsCausePreservesTheValidatorError pins the typed shape of
// the windows annotation on every host. The test above can only assert the
// message on a non-windows runner, which is where CI runs — so the subtype and
// the wrapped cause would otherwise go unchecked.
func TestPositionalArgsCausePreservesTheValidatorError(t *testing.T) {
t.Parallel()
parent, _, _, _ := newTestRig(t, shortcutFromRegistry(t, "+cells-set"))
cmd, _, findErr := parent.Find([]string{"+cells-set"})
if findErr != nil {
t.Fatalf("Find: %v", findErr)
}
sentinel := errors.New(`unknown command "stray" for "sheets +cells-set"`)
ve := requireValidation(t, annotatePositionalArgsCause(cmd, sentinel), "stray")
if !strings.Contains(ve.Hint, "PowerShell") {
t.Errorf("hint = %q, want it to name the shell as the cause", ve.Hint)
}
if !strings.Contains(ve.Hint, "--cells") {
t.Errorf("hint = %q, want it to inline the command's payload flags", ve.Hint)
}
// The annotation adds a hint to the framework's own argument error; a
// caller must still be able to reach that error underneath.
if !errors.Is(annotatePositionalArgsCause(cmd, sentinel), sentinel) {
t.Error("the prior validator error must stay reachable via errors.Is")
}
}
// TestPayloadFlagNames pins the list the windows prescription inlines: the
// flags whose values are large enough for a shell to split.
func TestPayloadFlagNames(t *testing.T) {
t.Parallel()
parent, _, _, _ := newTestRig(t, shortcutFromRegistry(t, "+cells-set"))
cmd, _, err := parent.Find([]string{"+cells-set"})
if err != nil {
t.Fatalf("Find: %v", err)
}
got := payloadFlagNames(cmd)
for _, want := range []string{"--cells", "--writes"} {
if !strings.Contains(got, want) {
t.Errorf("payload flags = %q, want it to carry %s", got, want)
}
}
}