Skip to content
This repository has been archived by the owner on Dec 17, 2018. It is now read-only.

argStyleText is not supported #1

Closed
nkovacs opened this issue Jun 7, 2017 · 6 comments · Fixed by #3
Closed

argStyleText is not supported #1

nkovacs opened this issue Jun 7, 2017 · 6 comments · Fixed by #3

Comments

@nkovacs
Copy link
Contributor

nkovacs commented Jun 7, 2017

The second string is valid according to ICU, but can't be parsed by messageformat-parser:

var parse = require('messageformat-parser').parse;

console.log(parse("{0,date,long}"));
console.log(parse("{0,date,y-M-d HH:mm:ss zzzz}"));

I know messageformat.js doesn't support the second one, but I'm using only the parser for globalize.js, which would be able to support the second one.

@nkovacs
Copy link
Contributor Author

nkovacs commented Jun 7, 2017

This is also valid in ICU:

{var,date,y-M-d,HH:mm:ss,zzzz}

In this case, argStyle should be y-M-d,HH:mm:ss,zzzz (there can only be one argStyle), but messageformat-parser instead parses it as three separate arguments.

nkovacs added a commit to nkovacs/icu-messageformat-parser that referenced this issue Jun 30, 2017
Apostrophes are now handled correctly, emulating
ICU's default DOUBLE_OPTIONAL behavior.

Octothorpe is handled correctly, and can be escaped using
apostrophes, but only inside plural
(depending on strictNumberSign).

A single apostrophe only starts quoted literal
text if it immediately precedes a curly brace ({}),
or, if inside a plural, an octothorpe (#).
The parser now supports the strictNumberSign option,
since that determines whether a quoted octothorpe
is parsed as `'#'` or just `#`.

Since choice format isn't supported, the pipe symbol
never causes an apostrophe to start quoted literal text.

Parameters to functions may contain whitespace and
quoted special characters, but argStyle is still trimmed
and split into multiple parameters.
A new option, strictFunctionParams, activates ICU-compatible
parsing, which parses everything from the second
comma to the closing curly brace as a single "argStyleText"
parameter.

Fixes messageformat#1, fixes messageformat#2
nkovacs added a commit to nkovacs/icu-messageformat-parser that referenced this issue Jun 30, 2017
Apostrophes are now handled correctly, emulating
ICU's default DOUBLE_OPTIONAL behavior.

Octothorpe is handled correctly, and can be escaped using
apostrophes, but only inside plural
(depending on strictNumberSign).

A single apostrophe only starts quoted literal
text if it immediately precedes a curly brace ({}),
or, if inside a plural, an octothorpe (#).
The parser now supports the strictNumberSign option,
since that determines whether a quoted octothorpe
is parsed as `'#'` or just `#`.

Since choice format isn't supported, the pipe symbol
never causes an apostrophe to start quoted literal text.

Parameters to functions may contain whitespace and
quoted special characters, but argStyle is still trimmed
and split into multiple parameters.
A new option, strictFunctionParams, activates ICU-compatible
parsing, which parses everything from the second
comma to the closing curly brace as a single "argStyleText"
parameter.

Fixes messageformat#1, fixes messageformat#2
nkovacs added a commit to nkovacs/icu-messageformat-parser that referenced this issue Jul 3, 2017
Apostrophes are now handled correctly, emulating
ICU's default DOUBLE_OPTIONAL behavior.

Octothorpe is handled correctly, and can be escaped using
apostrophes, but only inside plural
(depending on strictNumberSign).

A single apostrophe only starts quoted literal
text if it immediately precedes a curly brace ({}),
or, if inside a plural, an octothorpe (#).
The parser now supports the strictNumberSign option,
since that determines whether a quoted octothorpe
is parsed as `'#'` or just `#`.

Since choice format isn't supported, the pipe symbol
never causes an apostrophe to start quoted literal text.

Parameters to functions may contain whitespace and
quoted special characters, but argStyle is still trimmed
and split into multiple parameters.
A new option, strictFunctionParams, activates ICU-compatible
parsing, which parses everything from the second
comma to the closing curly brace as a single "argStyleText"
parameter.

Fixes messageformat#1, fixes messageformat#2
nkovacs added a commit to nkovacs/icu-messageformat-parser that referenced this issue Jul 17, 2017
Parameters to functions may contain whitespace and
quoted special characters, but they are still trimmed
and split into multiple parameters.
A new option, strictFunctionParams, activates ICU-compatible
parsing, which parses everything from the second
comma to the closing curly brace as a single "argStyleText"
parameter.

Fixes messageformat#1
@eemeli eemeli closed this as completed in #4 Jul 17, 2017
@eemeli
Copy link
Member

eemeli commented Mar 17, 2018

Re-opening this, because tbh we should drop the current default .split(',').trim() processing, and assume that the current strictFunctionParams is always true. That would make us more spec-compliant, let us get rid of an option, and make issues like messageformat/messageformat#185 much easier to solve. It should be up to the called function to handle the string input it receives.

One open question here is whether the output should be trimmed of white space. The spec is unclear on this. I think we should trim, as that would match what we're doing everywhere else as well.

@eemeli eemeli reopened this Mar 17, 2018
@SlexAxton
Copy link
Member

SlexAxton commented Mar 17, 2018 via email

@eemeli
Copy link
Member

eemeli commented Mar 17, 2018

I left the messageformat-parser output untrimmed for now, but added a trimmer in the messageformat compiler. This way if someone else wants to use the parser, they get to make up their own mind on this, but our formatters don't need to all trim their inputs separately.

@eemeli eemeli closed this as completed Mar 17, 2018
@mhuebert
Copy link

Overall this change made my life easier, thanks!

One question; since the string is trimmed, is there a way to escape leading or trailing space characters to have them included? I don't see any mention of this in the formatting docs.

@SlexAxton
Copy link
Member

SlexAxton commented Aug 17, 2018 via email

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants