Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rules for using expressions in text and as selectors are too complex #97

Closed
stasm opened this issue Apr 25, 2018 · 10 comments
Closed

Rules for using expressions in text and as selectors are too complex #97

stasm opened this issue Apr 25, 2018 · 10 comments

Comments

@stasm
Copy link
Contributor

stasm commented Apr 25, 2018

Terms can currently have values with variants as well as attributes. Variants can be accessed with -term[variant name] and attributes with -term.attr. Variants are supposed to be different facets of the same value (e.g. grammatical cases of the same noun) while attributes can provide additional grammatical information about the value, e.g. the gender, which can be used in selectors.

Messages can also define variant lists and attributes. However, accessing variants on messages is not allowed. Attributes can be access with message.attr.

To enforce this design we introduced rules for using the accessors in different contexts:

Expression Allowed in text Allowed as selector
message Yes No
message.attr Yes No
message[variant]1
-term Yes No
-term.attr No Yes
-term[variant] Yes No

1 Right now, this is not a valid expression.

The rules are complex. I'd like to make this part of the syntax design simpler.

@stasm
Copy link
Contributor Author

stasm commented Apr 25, 2018

Relaxing these rules and making them linter checks would make it easier to write well-formed FTL.

@zbraniecki
Copy link
Collaborator

When we designed it we did plan to enable message[variant] in the future, initially for a single-variantlist case, and if we see that being used message[variant][subvariant] or message[variant, subvariant] later.

@stasm
Copy link
Contributor Author

stasm commented Apr 26, 2018

There still might be valid usecases for the message[variant] syntax and I'm not opposed to introducing it in the future. This issue to me is more about our stance towards grammar rules which govern which expressions may appear where. I'm leaning towards allowing all (currently supported) expressions everywhere where an expression may appear. Linters, rather than grammar, can protect against misuse and the runtime can use its fallback heuristics to ensure the translation will resolve to a string.

@Pike
Copy link
Contributor

Pike commented Apr 27, 2018

I think I've come full circle on this once more (I'm getting dizzy ;-) )

I agree that removing options from accessors would make it easier to write well-formed FTL.

I do think that there's a level at which "simple" syntax helpers in editing modes and tools can help to write valid FTL.

Thinking about this in this context:

  • messages don't have variants
  • terms can have multi-depth variants (plurals, capitalization, case)
  • attributes can have selectors (PLATFORM()-dependent gender), but no variants

I think we can have more powerful tools, and also human thinking, to help people to write working fluent, by having visual patterns and syntactic rules to assist.

As the attribute removal #98 proposal is more concrete, I've left two scenarios in a comment, which exemplify my thinking here.

I think that the scenarios in which Term variants and attributes work as intended are complex, and I think syntax differences make those semantic differences more readily available to tooling and humans.

Terminology:

Well-formed
The content parses without any parser errors
Valid
The linters (TBD) run w/out errors, but warnings are OK?
Working
The content is well-formed and valid, and does what the humans intended to happen

@stasm
Copy link
Contributor Author

stasm commented Apr 27, 2018

I'd like to try relaxing our grammar to allow more expressions as well-formed selectors and placeables. In fact, I'd probably start by:

  • allowing all inline expressions as selectors, and
  • allowing all expressions as placeables.

@Pike suggested that having a concrete patch to look at would make it easier to discuss this proposal. I'll prepare a PR once the grammar spec changes over to the reference parser in #101, and we'll taking from there.

@zbraniecki
Copy link
Collaborator

which level of breakage would then constitute a breaking change? Currently, if message[variant] is not allowed in text, we know we can give it meaning in the future version of Fluent without backward incompatibility.

In the new world, if the parser allows, but the linter rejects, would it be a breaking change?

@stasm
Copy link
Contributor Author

stasm commented Apr 28, 2018

I didn't make it clear in my previous comment: I meant relaxing the rules of using expressions in different places (placeable, selector) rather than the rules of what constitutes a valid expression. message[variant] is currently an invalid accessor and it follows that you can't use it anywhere. I changed the table in this issue's description to reflect that.

That said, thanks for bringing up the topic of compatibility. For Firefox and xchannel, it might make sense to relax the syntax of VariantExpression to allow any other Expression as the parent. Resolver's fallback would apply in case something weird is used as the parent. In the future, when we decide to give message[variant] concrete meaning, we wouldn't break FTL files in older parsers.

@Pike
Copy link
Contributor

Pike commented Apr 28, 2018

A comment that zibi made on the linters gave me another idea:

In the kombi parser, we have two different steps to express things being OK or not:
In parsing the text, and in creating the AST. For example, the creation of a Message can do the the value vs attribute checks on the syntax side like right now, but it could also through at the .spreadInto when the data model is violated. Or we could even make that an explicit step so that it's easier to digest.

That's kinda like type-checking in a compilation step, which isn't actually at the parsing level, but still something that complains at parse time.

@stasm
Copy link
Contributor Author

stasm commented Apr 28, 2018

Interesting! This sounds like something I'd like us to try out.

@stasm stasm added this to the Syntax 0.6 milestone May 16, 2018
@stasm stasm removed this from the Syntax 0.6 milestone May 23, 2018
@stasm
Copy link
Contributor Author

stasm commented Jun 5, 2018

#128 landed and it introduced a validation step in syntax/abstract.mjs which transforms a well-formed AST into a valid AST.

I also filed #131 to discuss allowing the message[variant] syntax in a future milestone.

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

No branches or pull requests

3 participants