projectfluent / fluent

Fluent — planning, spec and documentation
https://projectfluent.org
Apache License 2.0
1.42k stars 45 forks source link

Forbid patterns as values of call-expression's key-value arguments #8

Closed zbraniecki closed 7 years ago

zbraniecki commented 7 years ago

I believe it would be valuable to separate the full Pattern with its elements, from a simple string.

key = Hello { $world }

key2 = { LEN($num, style: "long") }

Hello { $world } is a Pattern, but "long" could be just a simple string.

stasm commented 7 years ago

Thanks for filing this, it was going to be the last issue I wanted to file during my spree :) Here's my proposal:

  1. make pattern an expression; this will allow it in the following cases:

    abc1 = a { "b { foo }" } c
    abc2 = a { "b { foo }" -> … } c
    abc3 = a { LIST($b, "other things") } c

    abc3 is the reason I think this is necessary. To fully take advantage of List formatting, we need to allow full patterns as the last item of the list. Think "Mary, Anna, and 3 other people":

    foo = { LIST($user1, $user2, "{ $num } other people") }
  2. in key-value args, make the value be a member-key (likely to be renamed), not a pattern. This also means no quotes and making sure keywords cannot contain commas:

    foo = { NUMBER($num, style: long, minimumIntegerDigits: 2) }
zbraniecki commented 7 years ago

My take:

  1. I'd like to delay it for now. It requires a lot of testing for edge cases in the parser and will likely introduce the biggest vector of confusing code for readability.

I believe we will end up needing it, but I'd like to get the parser and error reporting in good shape first.

  1. From a purely stylistic perspective, I think I prefer "long" over long. It also allows for white spaces in the value:
foo = { DATE($num, timeZone: "New York") }
stasm commented 7 years ago

Re. #1, the iterative approach sounds good. Although from the sound of it, it might be that we need to iterate in the implementation rather than in the spec :)

Re. #2, if the argument's value is a keyword we could make { DATE($num, timeZone: New York) }work as well. To me both versions look good. That said, this will be rare enough that we shouldn't optimize for how well it looks. If you don't like the idea of re-using keyword here (which would help avoid introducing a new class), I'm OK with using "…" here as well, with the 'simple string' semantics.

zbraniecki commented 7 years ago
  1. Sure, adding "Pattern(Pattern)" to expressions in AST is easy. I just wanted to get some time before I enable it in parser.

  2. Yeah, I think I am opinionated here because of the readability and error spotting:

key = { DATE($num, time: zone style: long) }
stasm commented 7 years ago

Let's start by being more strict in call-expressions key-value args. All examples given in this issue were about them anyways and it's seems clear that we don't see use-cases for making them more complex.