fix(cli): accept --props comma batch form on mutating verbs - #383
Open
ylz92871-dotcom wants to merge 1 commit into
Open
ylz92871-dotcom wants to merge 1 commit into
ylz92871-dotcom wants to merge 1 commit into
Conversation
What: --props "k=v,k2=v2" is now a registered option on set/add/remove/
move/mark, parsed with the same key=value semantics as repeating --prop.
A value that contains a comma is protected by single quotes
(--props "text='a,b',bold=true" → text=a,b, bold=true); --prop values
are never split, so comma-bearing values (data="S1:1,2,3") behave
exactly as before. Malformed input (segment without '=', stray comma,
unterminated quote) is rejected up front with code=invalid_argument and
a --prop suggestion — never partially applied.
Why: agents habitually guess the plural form. Because --props was not a
registered option, System.CommandLine shunted the whole value into
unmatched tokens, where it was dropped with at best a warning and at
worst a misleading downstream error (an add chart batch lost
dataRange/categories entirely and reported "Chart requires a 'data'
property" — repro'd against 1.0.148). The BUG-BT-R6 typo-catch only
turned the silent drop into a warning; the properties still never
reached the handlers. The suggestion text also doubled the flag
("Use: --prop --prop value=...") because the claimed token was
already prefixed; the prefix is removed so every consumer adds it
exactly once.
How to verify:
officecli add data.xlsx /Sheet1 --type chart --props "type=line,anchor=H20,dataRange=Sheet1!B2:B3,categories=Sheet1!A2:A3" --json
→ Added chart (previously: Chart requires a 'data' property)
officecli set data.xlsx /Sheet1/A1 --props "value='a,b',bold=true" --json → text 'a,b', bold
officecli set data.xlsx /Sheet1/A1 --props "bold" --json → invalid_argument + suggestion
Resident and standalone paths share the merge point and were both
exercised (local harness suite: 9/9 green, incl. OFFICECLI_NO_AUTO_RESIDENT=1).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
--props "k=v,k2=v2"is now a registered option onset/add/remove/move/mark, parsed with the same key=value semantics as repeating--prop. A value that itself contains a comma is protected by single quotes (--props "text='a,b',bold=true"→text=a,b,bold=true).--propvalues are never split, so comma-bearing values (--prop data="S1:1,2,3") behave exactly as before. Malformed input (a segment without=, a stray comma, an unterminated quote) is rejected up front withcode=invalid_argumentand a--propsuggestion — never partially applied.Why
Agents habitually guess the plural form. Because
--propswas not a registered option, System.CommandLine shunted the entire value into unmatched tokens, where it was dropped — at best with a warning, at worst via a misleading downstream error. Real-world repro against 1.0.148: a chart add lostdataRange/categoriesentirely and reported "Chart requires a 'data' property", which misleads the caller into "fixing" the wrong thing. The existing BUG-BT-R6 typo-catch only turned the silent drop into a warning; the properties still never reached the handler. The warning's suggestion also doubled the flag (Use: --prop --prop value=...) because the claimed token was pre-prefixed; the prefix is now added exactly once by consumers.Implementation
CommandBuilder.CreatePropsBatchOption()— shared--propsoption factory;MergePropFlags()merges--propentries with quote-aware comma-split--propsentries;SplitPropsBatch()does the splitting + up-front validation. All inCommandBuilder.csnext to the existingDetectUnmatchedKeyValuesmachinery.--propregistrations (set/add/remove/move/mark). The merge happens at the single point where each verb reads its props, so resident and standalone routes share it with no behavior difference.-props/--prop=spellings and verbs without the option) but no longer injects a--propprefix into claimed tokens.How to verify
Local regression harness (shells the built exe; resident +
OFFICECLI_NO_AUTO_RESIDENT=1standalone): 9/9 green — comma-form chart add, quote protection,--proppassthrough, three rejection paths withcode+suggestion, mixed--prop --propsmerge, both execution modes.dotnet build -c Release: 0 errors (1 pre-existing CS8602 warning, untouched by this change).dotnet publishsingle-file smoke-verified end-to-end on the published exe (atomic batch rollback, formula evaluation readback, chart adds, validate 0 errors).