-
Notifications
You must be signed in to change notification settings - Fork 77
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
SelectExpression as a selector for another SelectExpression #416
Comments
Why is it not valid? The value of your message is a All according to the spec. In Rust we use https://github.com/projectfluent/fluent-rs/blob/master/fluent-syntax/src/ast.rs and have the AST inspector: https://github.com/projectfluent/fluent-rs/tree/master/fluent-cli You can launch it with |
You’re right, it is allowed by the grammar. But it will be rejected during validation step, since |
Ah, you're right! Wondering if we should tighten the ast or relax the resolver :) I'd be down to allow for placeable as selector |
Inability to put a placeable here seems like a purely synthetic limitation to me. A user can work around this by moving the inner What for this:
—it’s just syntax sugar. Pretty useless, I agree, but still cannot see a reason to forbid it. For the sake of completeness. |
Oh, I’ve just found out that it will allow to bypass restriction that
However, it’s also possible to do right now:
By the way, could you tell me why this restriction exists? An extra measure of safety against typos (because a user will unlikely want to do this)? |
Hi Nickolas, welcome to the world of Fluent. Did you start publishing your D implementation yet? We're generally curious to see how other folks implement Fluent. Also, I'd be happy to link to your implementation on https://github.com/projectfluent/fluent#other-implementations. To your question, you're encountering an unspecified specified unspecified behavior. I like to create confusing sequences of words. Anyway, the In your implementation, you should avoid those shortcuts, if possible. To this concrete point, changing the specification for select expressions would be a change to the Fluent syntax specification. That's quite a hefty hammer, which I'd rather not wield. As for the reason to reduce the options of selectors, that conversation was in projectfluent/fluent#97. For this particular question, using messages as selectors is not allowed to protect localizers from developers. Developers would probably use that to create delicate contraptions, which we don't expect localizers to re-implement in their language. This is along the lines of WET over DRY, that we detail in https://github.com/projectfluent/fluent/wiki/Good-Practices-for-Developers#prefer-wet-over-dry. AFAICT, we also don't have a test for this in the spec repo, which means that our implementations might not catch this the way they should. At least I didn't see it in the rust impl that zibi linked to, and |
Thank you for such a detailed reply! To sum up, it is invalid in Fluent 1.0 and might or might not become part of Fluent 1.1. The spec is an authoritative source of information and should be preferred in the event of discrepancy. Since you are asking, I pushed the repo to GitHub. There’s no much to look at yet, though. The parser isn’t even finished: (I hope) it correctly parses everything except |
…uent#416 This uplifts the tests from the spec PR, projectfluent/fluent#280 and implements the selector validation to follow what `abstract.js` does in the spec.
…uent#416 This uplifts the tests from the spec PR, projectfluent/fluent#280 and implements the selector validation to follow what `abstract.js` does in the spec. In @fluent/bundle, the invalid selectors are just accepted. Might be worth revisiting.
Put up #417 now, and also the PR on tests on the spec. Thanks for digging in, this bug is actually pretty wide spread. I've glanced at the repo, and I'll raise questions there. Forgot to ask, have you come across https://discourse.mozilla.org/c/fluent ? That's where we have cross-implementation conversations. |
Glad I was able to help. Thanks for the link. No, I hadn’t discovered that. |
According to the spec (abstract.js), this is well-formed yet not valid Fluent. However, this case is not enforced in getExpression, and thus the code is correctly parsed and evaluates to
It’s X.
.I’m currently implementing Fluent in D Programming Language and wonder whether it is a bug in the spec or the implementation (i.e. whether I need to support it or not). Could you clarify it, please?
The text was updated successfully, but these errors were encountered: