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

Remove tags #67

Closed
3 tasks done
stasm opened this issue Nov 10, 2017 · 11 comments
Closed
3 tasks done

Remove tags #67

stasm opened this issue Nov 10, 2017 · 11 comments
Milestone

Comments

@stasm
Copy link
Contributor

stasm commented Nov 10, 2017

This is a counter-proposal to #59. If #62 is approved we could go further than only allowing tags on glossary/private messages and replace tags with attributes defined on private messages.

The main reason why tags exist is to encode language-specific data like grammatical genders etc. We decided to add a separate data structure so that it's easy to distinguish between data which is part of the message's public interface and data which is private. In other words, the use-case for tags is tooling.

With private/glossary messages we satisfy this use-case by marking the message as special. Tools can choose to check for existence of glossary messages but ignore their attributes. In such scenario, those attributes can be used to store language-specific information about the translation.

By removing tags we:

  • remove an entire data type which makes Fluent easier to understand,
  • free up the tag's sigil making the syntax easier to learn,
  • simplify the message lookup in selectors.

Wrt. the last point, right now tag lookup simply uses the message reference in the selector. Messages are not matched to variant keys by value but rather by their tags. In case more than one tag are present, any matching tag will select the variant. We haven't yet defined a way to AND tags in variant keys.

brand-name = Aurora
    #żeński

has-updated =
    { brand-name ->
        [męski] { brand-name} został zaktualizowany.
        [żeński] { brand-name } została zaktualizowana.
       *[inny] Program { brand-name } został zaktualizowany.
    }

Without tags, the above becomes a selector with an AttributeExpression on a private message:

-brand-name = Aurora
    .gender = żeński

has-updated =
    { -brand-name.gender ->
        [męski] { -brand-name} został zaktualizowany.
        [żeński] { -brand-name } została zaktualizowana.
       *[inny] Program { -brand-name } został zaktualizowany.
    }

Once we implement #4 it will become easy and consistent with the rest of the syntax to use more than one private attribute for the selection logic.

Signoffs

@Pike
Copy link
Contributor

Pike commented Nov 16, 2017

One thing we're doing with tags is that we're taking them out of the data space that can be exposed publicly. You can't return the żeński to the calling program.

So I disagree that this is just a tooling feature.

@stasm
Copy link
Contributor Author

stasm commented Nov 16, 2017

Do we really need to enforce this level of protection in the runtime? I could see a linter rule which warns against interpolating attribute expressions on private messages (foo = { -brand-name.gender }), but even then I think it should not be an error.

@Pike
Copy link
Contributor

Pike commented Nov 16, 2017

I strongly think that exposing language-internal traits is an error. We've had people try to abuse language-internal traits over and over, and I also think that making it clear on the syntax level that that's not OK is a win.

Sorry for using traits, but we're not using that word right now, so I use it to mean tags/attributes/something.

@zbraniecki
Copy link
Collaborator

One cannot retrieve a private message from MessageContext, so the only way to someone could attempt to retrieve an attribute on a private message is via interpolation.

If that's the case, we could address Pike's concern by simply forbidding AttributeExpressions on private messages outside of SelectExpressions selector.

@stasm
Copy link
Contributor Author

stasm commented Nov 20, 2017

I disagree on "simply" :) There is a lot of conditions in forbid AttributeExpressions on private messages outside of SelectExpressions selector which makes this rule hard to internalize. I'd prefer to move this check into a linter.

The syntax should help localizers create good localizations without imposing too many strict rules. At any point a localizer can create a bad translation: they can write About { -brand-name.gender } just as they can write About Totally Not Brand Name. Not everything that parses must make immediate sense.

@zbraniecki
Copy link
Collaborator

I'm totally fine with having this be a linter check.

@Pike
Copy link
Contributor

Pike commented Nov 20, 2017

Do we really need to enforce this level of protection in the runtime?

Yes, we need to handle this at runtime. If it's legal syntax, people can and likely will put it in files, and we need to have defined runtime behavior.

Linters are things that we run optionally. Hopefully people run it, but we can't enforce that they do in any way.

@stasm
Copy link
Contributor Author

stasm commented Nov 23, 2017

Before we discuss the implementation in #69, I'd like to get sign-offs on this proposal from the majority of the core team. Please comment in this thread and tick the box next to your name if you give this proposal a green light.

@stasm
Copy link
Contributor Author

stasm commented Nov 23, 2017

r+

1 similar comment
@zbraniecki
Copy link
Collaborator

r+

@Pike
Copy link
Contributor

Pike commented Nov 29, 2017

We talked about this on the tech call yesterday, and zibi and I share the concern about runtime leakage, though we have different severities for it.

As we've seen folks try to do interesting stuff with hashes in l20n files, we'll want a runtime catch for this.

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

No branches or pull requests

3 participants