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

Rename MessageContext to FluentContext #222

Closed
stasm opened this issue Jun 6, 2018 · 13 comments
Closed

Rename MessageContext to FluentContext #222

stasm opened this issue Jun 6, 2018 · 13 comments

Comments

@stasm
Copy link
Contributor

stasm commented Jun 6, 2018

I propose to make the Fluent API more explicit about relating to Fluent.

Originally we chose the MessageContext name as an evolution of MessageFormat thinking about the possible standardization in the future. This is a noble goal but we can cross that bridge when we get there. It's probably even better to not use this name right now in production code. I also hope that it will make the code easier to work with (debug, read stack traces) and easier to name other APIs consistently if we use the Fluent prefix.

  • MessageContextFluentContext
  • MessageArgumentFluentType
  • MessageNumberArgumentFluentNumber
  • MessageDateTimeArgumentFluentDateTime
@zbraniecki
Copy link
Collaborator

I agree that we can rename them. The only reservation I have is about the context.

There's a difference between what we think of a context and what consumers use. It's been bothering me every since we wrote it that we use the term "context" to describe a low-level, hidden object that collects/flattens a bunch of resources together, which makes us have to come up with different term for the thing that people actually use (which is now called Localization).

It came back as we work on the nsIDocument API since it brings back the concept that we may want to allow for multiple "things" on a single document. Hand waving:

<window>
  <linkset name="main">
    <link rel="localization" href="brand.ftl"/>
    <link rel="localization" href="browser/menu.ftl"/>
  </linkset>
  <linkset name="findbar">
    <link rel="localization" href="brand.ftl"/>
    <link rel="localization" href="browser/findbar.ftl"/>
  </linkset>
</window>

What is it that we have one of here, and what is it that we have two of here? Using current naming, we have a single DocumentLocalization, which has two DOMLocalizations, with each having an iterator over MessageContext objects inside.

That means we have to teach this vocabulary to our users, who just want to do <container data-l10n-foo="main"/> and <container data-l10n-foo="findbar"/> and maybe document.l10n.foos["findbar"].formatValues.

It feels very natural to replace foo with ctx here. They want values out of findbar localization context, or from the main one. They want to mark some part of DOM to be covered by findbar localization context etc.

I don't have solutions yet, but I know that trying to talk about getting values out of findbar localization feels awkward.

Is it something you'd be willing to add to this round of naming conversations?

@Pike
Copy link
Contributor

Pike commented Jun 7, 2018

We can also hide complexity on the developer-API level, and lie. I agree that we're likely going to name that thing Context to the developer. I don't think that there's value in telling the developer that in between the API and the Context there's the LazyListOfContextsThatKnowYourFallbacks. Because at the other interaction level that the developer has is the list of Fluent files that they want, and that has to avoid mismatching IDs etc.

I think not talking about the LLOCTKYF is a good thing in practice, and highlights the pieces that the developer cares about.

@stasm
Copy link
Contributor Author

stasm commented Jun 7, 2018

I was hoping for this issue to be a matter of a simple search and replace. I didn't mean to (re-)start this discussion.

I'm quite happy with FluentContext, Localization and LocalizationManager, in the order of growing abstraction. document.l10n is an object implementing the LocalizationManager interface. I still like Localization for the thing which juggles single-language FluentContexts. I think it works well with prefixes like GeckoLocalization, DOMLocalization and ReactLocalization. The word context doesn't seem to work this well here. Would we use it as ReactLocalizationContext?

Another way out of this is to side step this discussion and drop context entirely. We could then use it as a concept to explain the architecture. The API names would not use it, however:

FluentResource
FluentBundle
{Gecko,DOM,React}Localization
LocalizationManager

@stasm
Copy link
Contributor Author

stasm commented Jun 7, 2018

One more thing about the subjectivity of some arguments.

It feels very natural to replace foo with ctx here. They want values out of findbar localization context, or from the main one. They want to mark some part of DOM to be covered by findbar localization context etc.

I'd like to emphasize that this is subjective. For example to me, it feels very natural to say:

They want values out of findbar localization, or from the main one. They want to mark some part of DOM to be covered by findbar localization etc.

And I think we can teach people this vocabulary without a problem.

@stasm
Copy link
Contributor Author

stasm commented Jun 21, 2018

CC @6a68, @fzzzy, @flodolo, @Gregoor, @hkasemir, @ianb, @jimporter, @juliandescottes, @julienw.

I'm looking for opinions about FluentContext vs. FluentBundle. I have a feeling that semantically they are quite close. As we all know, naming is hard. One way to move forward is to just pick one :) Instead, I'd like to open this up for more feedback first. If there is a visible preference for one of the options, it will make it easier to decide. I can't promise your preferred name will end up being picked, but your input is appreciated and will be taken into account in the final decision.

You've all worked with Fluent in some capacity in the past. You've all written your share of new MessageContext() and function * generateContexts(). We're considering renaming MessageContext to FluentContext in this issue, but I'd also like to hear your personal opinion on whether the word context is the right one here. Do you feel FluentBundle would be a better name? Do you have other suggestions for a name describing something that stores translations from multiple Fluent resource files (*.ftl), all in the same language, with the goal of making it possible for one translation to reference, or interpolate, a translation from another resource?

Thanks!

@Gregoor
Copy link
Contributor

Gregoor commented Jun 26, 2018

Sorry, I don't have much of a preference. Both seem like sensible options to me 🙂

@juliandescottes
Copy link

Renaming MessageContext to FluentContext is a good move, it will make it clear that the object we are dealing with is fluent specific.

From my perspective bundle makes more sense than context, but that's probably just because I have come across the term "message bundle" in other stacks. But I don't have a strong preference, because as a user, after creating contexts I pass them as messages to a LocalizationProvider. So it doesn't have a huge impact on my application code.

@jimporter
Copy link

jimporter commented Jun 26, 2018

Renaming MessageContext to FluentContext is a good move, it will make it clear that the object we are dealing with is fluent specific.

I would say the opposite actually (though I don't care that strongly); we already know it's a Fluent thing because we imported it from Fluent. Modules generally don't reiterate their name in the names of the symbols inside the module (e.g. React has Component, not ReactComponent). In fact, simply calling it Context (or Bundle) might be sufficient; Bundle has the benefit of being a less "generic" term, whereas Context pops up fairly often in code, so I might find myself thinking "wait, which kind of Context is this now?"

The Message prefix might be useful as a way of indicating that it handles messages though (as opposed to some other kind of thing Fluent might care about, e.g. language codes). I wouldn't have an issue if it went away though (or really, if were replaced by Fluent; as mentioned, I don't have strong feelings about any of this).

@stasm
Copy link
Contributor Author

stasm commented Aug 9, 2018

This dropped off my radar. I'd like to find a way to move forward. It looks like there is a slight preference to using Bundle and I know @zbraniecki has been an advocate of not using Context for a long time. I also see the argument about the namespacing (i.e. calling it a FluentSomething rather that just Something), but it's become a sort of a convention in other Fluent code at this point, and it has a potential to make debugging easier at times.

I think what this issue needs is a decision, so I'm going to make it :) Let's do the following renames:

MessageContext → FluentBundle
MessageArgument → FluentType
MessageNumberArgument → FluentNumber
MessageDateTimeArgument → FluentDateTime

I'm going to leave this issue open for another week, until Thursday, August 16. If I don't hear any significant opposition to the above plan by then, I'm going to implement the new names and land.

Thanks to everyone for taking time to share your opinions!

@zbraniecki
Copy link
Collaborator

I'm a fan of those changes!

The only one I'm not sure is the MessageArgument -> FluentType - why not FluentArgument?

Would that means that we can then do Localization -> FluentContext? :)

@stasm
Copy link
Contributor Author

stasm commented Aug 9, 2018

Thanks, @zbraniecki!

The only one I'm not sure is the MessageArgument -> FluentType - why not FluentArgument?

Because the base class is called FluentType, and we only export it as MessageArgument:

FluentType as MessageArgument,

While not a goal, it's also nice that the same vocabulary is used in Rust and in Python: FluentType.

Would that means that we can then do Localization -> FluentContext? :)

This is out of scope of this issue :) Please open a new one if you'd like to have this conversation.

@stasm
Copy link
Contributor Author

stasm commented Aug 17, 2018

I opened #276 to implement this.

@stasm
Copy link
Contributor Author

stasm commented Aug 20, 2018

#276 has been merged and fluent 0.8.0 has been published with the following changes:

MessageContext → FluentBundle
MessageArgument → FluentType
MessageNumberArgument → FluentNumber
MessageDateTimeArgument → FluentDateTime

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

6 participants