projectfluent / fluent

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

New value type: Variant List #102

Closed stasm closed 6 years ago

stasm commented 6 years ago

We currently allow Select Expressions to have a null selector. This is mostly useful in Terms to define facets of the term's value:

-brand-name =
    {
       *[nominative] Firefox
        [genitive] Firefox's
    }

The grammar defines the above syntax as:

Term
    id: Identifier
        name: "-brand-name"
    value: Pattern
        elements:
            - Placeable
                expression: SelectExpression
                    expression: null
                    variants: […]

Variants defined this way can be retrieved inside of Placeable with the -term[variant name] syntax. The runtime checks if the term's value is a pattern with a single element: a placeable which is a selector-less select expression. If the conditions are met, the variant is retrieved successfully.

The fact that we need this complex runtime check indicates to me that we're abusing Patterns for a logic-less (no selector) store for variants.

I'd like to make the design of Variant List more explicit by making them a new value type. Grammar-wise, values of Messages, Terms and Variants could either be Patterns or Variant Lists. Value List would not be expressions. Static analysis tools can then very easily tell when Variant Lists are mis-used: they shouldn't appear in Message at all, and they should only be nested inside of other Variant Lists.

The FTL snippet above would parse as follows:

Term
    id: Identifier
        name: "-brand-name"
    value: VariantList
        variants: […]
stasm commented 6 years ago

There still remains an inconsistency: a ValueList is not an expression so it cannot be inside of a Placeable.

# This wouldn't be allowed.
key =
    Foo Bar {
       *[name] Variant
    } Baz

But because they would be legal as values of other variants, this would be OK:

# This is OK.
key =
    Foo Bar { $num -> 
       *[other] {
           *[name] Variant
        }
    } Baz

To solve this, we'd need to make the variants in VariantList be a different grammar production than the variants in SelectExpression. Only variants in VariantList would allow nested VariantLists as their values.

I'm not sure if such distinction is necessary nor if it's worth the effort and the increase of complexity. From the use-cases point of view, a simple Message VariantList and SelectExpression VariantList CSS-like queries should be enough to warn against misuse.

Pike commented 6 years ago

There are aspects here that are not about if we do it or not, but where. Is it Syntax or Semantics that are statically checked.

I'd like to see both variants (pun intended), and see which of the two is easier to learn. We'll need to document both, one will be solely in the syntax, the other will be split between syntax and semantics.

zbraniecki commented 6 years ago

I like this proposal. I see a potential in the VariantList in the future so I'm happy to see it being formalized.

Pike commented 6 years ago

Taking a question about intent here instead of the PR:

@stasm , I see the semantically correct usage of Variants in Terms, but when it comes down to Messages it seems weird.

I've filed #130 for private terms, which would be a way for a localization to hack around the lack of a Term. But I also don't like the idea of making a work-around that much work to use that in a context like:

A developer uses a message reference, but the string as is doesn't work as is in a particular locale. Say, because literal quoting isn't a thing in that language. To work around it, the locale would either drop the message ref, or add variants to the message. Or add a private term as proposed in #130.

Maybe this is more of a documentation problem than anything else?

stasm commented 6 years ago

I think we talked about forbidding VariantList in Messages all together. I implemented #129 based on the Syntax Semantics wiki page, but it didn't have any rules for Messages drafted. Thanks for the reminder. I now edited it with a proposal which I hope reflects what we talked about. I'll work on implementing these rules in #129 on Monday and I'll re-request review. It would help to land #128 first to avoid conflicts.

stasm commented 6 years ago

@Pike On Friday we talked about allowing VariantLists as values of Messages to support the use-case you mentioned two comments above, in https://github.com/projectfluent/fluent/issues/102#issuecomment-392018228. Should this extend to attributes of messages? Arguably, it would support the same use-case. However, in case of attributes, it would require a new syntax, message.attr[variant] which is a non-trivial change to the grammar. No member expressions currently allow other member expressions to be the parent. They only allow simple reference expressions in this position.

Because of this added complexity, I suggest we move this entire feature (VariantLists as values of Messages) out of Syntax 0.6. I'll file a new issue if you agree.

Pike commented 6 years ago

sgtm.