Repository navigation
Conversation
…rgument An MF1 argument may drive several selectors of different types, e.g. a selectordinal in one arm of a select and a plural in another. Each of them was lifted into an .input declaration named after the argument, so the resulting data model was rejected with a duplicate-declaration error. Selectors that share an argument now get unique names and are declared as .local expressions of the external variable, and the octothorpe of a plural or selectordinal resolves to the declaration it was selected by rather than to the argument name. Messages where every argument drives at most one selector are unchanged. Addresses messageformat#472
This branch has not been deployed
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.
Addresses #472.
What's broken
An MF1 argument may legally drive selectors of different types in different arms.
findSelectArgs()merges selectors only whenarg,typeandpluralOffsetall match (equalSelectArgs), which is right for selection — one entry per selector — butargToInputDeclaration()then named every entry after the argument, and MF2 forbids declaring a variable twice, somf1ToMessageDataemitted.input {$count …}twice and the model died:Reproduced shapes (all now in the test table):
{type, select, …}+{count, selectordinal, …}from one argument; two plurals of the same argument with differentoffset:; plural arms containing a nestedselect/selectordinalon the same or another argument; and a group whose natural rename suffix collides with a real MF1 argument (count_1is an actual argument, so the group must skip tocount_2/count_3).The fix
In
mf2/icu-messageformat-1/src/mf1-to-message-data.ts:selectorNames(args)counts how many entries share eacharg. A group of one keeps the bare name, so every message that does not hit the bug produces a byte-identical model. In a larger group each entry getsarg_1,arg_2, …, skipping any suffix already used by another argument..local $count_1 = {$count :number select=ordinal}over the external argument instead of.input, so.matchnever repeats a variable.names[idx]— the declaration its selector was chosen by — rather than totoken.arg, which keeps#at value − offset for offset plurals.The shape suggested on the issue (keep one selector as
.input, chain the rest as.localoff it) was measured and rejected: a base carryingselect=ordinaloroffsetbreaks the.localchained off it, and groups need not contain an option-free member at all ({n, plural, offset:1 …} {n, plural, offset:2 …}has none). Declaring every member.localof the external argument is one code path and mirrors MF1, where each selector formats the raw argument independently. If a base+locals shape is preferred, the change is local toargToDeclaration/selectorNames.Two limits, stated plainly:
format()call, so an argument used both asselectand asselectordinalmust be numeric; MF1 evaluates only the taken arm, so an exotic message can newly reportbad-operand/bad-selectorat format time. The test table uses numeric values accordingly.Intl.PluralRulesis untouched; single-selector shapes are byte-identical before and after this fix.Validation
Multiple selectors per argumenttable plus two structural tests inmf1.test.ts. Red first:Tests 23 failedon the pristine tree, allduplicate-declaration; with the fix, all green.expvalue was taken from the MF1 reference implementation (@messageformat/core,new MF1('en').compile(src)(params)) via a differential harness that compares, per parameter set, MF1 output, the MF2 model built from the data object, and the model re-parsed from its ownstringifyMessage()source — 26/26 parameter sets identical across the six shapes.614f28f→Tests 2792 passed | 4 skipped; with the fix →Tests 2820 passed | 4 skipped,Test Files 37 passed (37)— exactly the +28 new cases, and no existing expectation changed, which is the mechanical proof that single-selector messages are untouched.#keepingtoken.arg(wrong offset values), bare-arg names (back toduplicate-declaration), all-.inputdeclarations, dropping the collision skip, and a.localreadingnameinstead ofarg.arg.npm test,npm run build,npm run lint,npx prettier --check .Prepared and tested with AI assistance; no human review is claimed.