l20n / spec

Specification and design documents for L20n
http://l20n.github.com/spec/
10 stars 6 forks source link

too strict EBNF definition for a placeable-list #14

Open GlenDC opened 7 years ago

GlenDC commented 7 years ago

The placeable-list is too strictly defined. Currently it is defined as follows

placeable-list       ::= placeable-expression (__ ',' __ placeable-list)?;

With the current definition in mind, the following L20n.org/learn example is not valid:

liked-photo = { LEN($people) ->
    [1]     { $people } likes
    [2]     { $people } like
    [3]     { TAKE(2, $people), "one more person" } like

   *[other] { TAKE(2, $people),
              "{ LEN(DROP(2, $people)) ->
                  [1]    one more person like
                 *[other]  { LEN(DROP(2, $people)) } more people like
               }"
            }

Example Source: https://github.com/l20n/l20n.org/blob/gh-pages/_posts/2016-04-13-complex-example.md

The reason being because we can see that there is a ( NL __ ) in between the call-expression and quoted-pattern. Personally I think this is reasonable, as it makes hand-written FTL files a lot more readable. Therefore I suggest to change the definition of a placeable-list to the following:

placeable-list       ::= placeable-expression (__ ',' __ ( NL __ )? placeable-list)?;
GlenDC commented 7 years ago

Related to the given l20n.org/learn example, it is still invalid because of another very strict definition for a placeable.

You can find the placeable definition here: https://github.com/l20n/spec/blob/master/grammar.ebnf#L32

For my current C# I changed that definition to:

placeable            ::= '{' __ placeable-list __ ( NL __ )? '}';

That's what I've found so far. But there might be more such cases lurking around unfound. So perhaps this might need to be investigated, as I would love to try to stay 100% compatible with the master L20n EBNF, but for now it is a bit too strict, to even follow your own examples.

If you want, I can do a careful check of the EBNF Grammar, make the definitions more flexible where needed, and add a PR for that.

stasm commented 7 years ago

Good catch, @GlenDC! Thanks. In https://github.com/projectfluent/syntax/issues/2 we want to simplify the grammar of placeables but I think the same problem will still apply to arglist.

If you want, I can do a careful check of the EBNF Grammar, make the definitions more flexible where needed, and add a PR for that.

We desperately need that :) Thanks for all the work you've put in so far.

GlenDC commented 7 years ago

No problem, it's a great project. Also is it now officially Project Fluent, rather then L20n?

I don't mind doing the work, @stasm, but how does this relate to https://github.com/projectfluent/syntax/issues/9? I am asking as that issue seems to be going into the opposite direction of what this issue is tackling.

stasm commented 7 years ago

Also is it now officially Project Fluent, rather then L20n?

Project Fluent is the low-level API (the syntax, the parser and the formatter) which L20n can build on top of to implement a full-fledged localization library. See https://groups.google.com/forum/#!topic/mozilla.tools.l10n/NPmsJD4IGjQ for a brief announcement.

how does this relate to projectfluent/syntax#9? I am asking as that issue seems to be going into the opposite direction of what this issue is tackling.

Isn't projectfluent/syntax#9 about non-new-line white space in member keys? Even if we take on the more lenient approach to whitespace, we probably wouldn't accept new lines in member keys. Do you think we should?

GlenDC commented 7 years ago

No, you're right in that newlines seems like a bad idea. However, that issue doesn't seem to be describing newlines at all, it just explains to not allow whitespace characters. I agree that you might want to have a new Syntax element, which does not include newline chars, and use that as an optional element around the member keys. But in that issue it seems to describe that any non whitespace character should not be allowed. Do I understand that wrong?