Remove the PLURAL builtin function

RESOLVED FIXED

Status

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: stas, Assigned: stas)

Tracking

Details

Attachments

(1 attachment)

40 bytes, text/x-github-pull-request
gandalf
: review+
Pike
: review+
Details | Review | Splinter Review
(Assignee)

Description

2 years ago
In bug 1310148 we started talking about removing the PLURAL builtin function.  Because numbers can be used as selectors anyways PLURAL() duplicates a functionality that already exists for little added value.

  foo = { $num ->
      [1]     One
      [few]   Few
     *[other] Other
  }

When the member's key is a keyword, $num will be converted into a CLDR category implicitly.

The benefit of keeping PLURAL is that it's explicit.  That said, it may actually be confusing since we want to allow comparison to numeral values anyways:

  foo = { PLURAL($num) ->
      [1]     One
      [few]   Few
     *[other] Other
  }

In the above example we explicitly use the PLURAL function and yet the comparison against 1 works fine.

PLURAL is currently implemented as an alias to NUMBER which also means that

  foo = { PLURAL(1) }

will result in foo being "1".

Let's remove it!
(Assignee)

Updated

2 years ago
Blocks: 1310148
(Assignee)

Updated

2 years ago
No longer blocks: 1310148
(Assignee)

Comment 1

2 years ago
Created attachment 8802882 [details] [review]
Pull request
Assignee: nobody → stas
Status: NEW → ASSIGNED
Attachment #8802882 - Flags: review?(l10n)
Attachment #8802882 - Flags: review?(gandalf)

Comment 2

2 years ago
Comment on attachment 8802882 [details] [review]
Pull request

I left a comment at https://github.com/l20n/l20n.js/pull/161/files#diff-6e11a933c4a7cf21927c19a36791393eR170 to add a bit more detail on when to explicitly use NUMBER() in the selector, I think we should add that.

Probably both in the syntax doc, and the builtins doc, too.

r=me on the general removal, though.

On that, I'm not strongly opinionated on this, I see confusion in both directions. This is more of a "less is more" point, and mostly something to be solved in documentation.
Attachment #8802882 - Flags: review?(l10n) → review+
Comment on attachment 8802882 [details] [review]
Pull request

PLURAL will be able to take parameters that will influence the behavior - much like Intl.PluralRules.

Main example is style cardinal/ordinal.

I don't think we can remove it.
Attachment #8802882 - Flags: review?(gandalf) → review-
(Assignee)

Comment 4

2 years ago
That's a good point.

That said, cardinal and ordinal are so separate from each other that I wonder if we should maybe drop PLURAL in favor of ORDINAL (and RANGE).  None of which we support right now, btw.
At this point, I'd prefer us keep in sync with the Intl APIs rather than create separate built-ins for each style.

I know it's not perfect, but I just feel like it's easier for us to do the right thing if we follow the scheme rather than trying to invent our own.

Because once we start mingling, why not NUMBER_LIST and and UNIT_LIST and UNIT_SHORT_LIST ?
(Assignee)

Comment 6

2 years ago
(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #5)
> At this point, I'd prefer us keep in sync with the Intl APIs rather than
> create separate built-ins for each style.
> 
> I know it's not perfect, but I just feel like it's easier for us to do the
> right thing if we follow the scheme rather than trying to invent our own.

Yes, I very much subscribe to the same point of view.

Would you keep PLURAL as currently implemented, i.e. as an alias to NUMBER?

Will Intl.PluralRules implement type=ordinal and/or type=range?

> Because once we start mingling, why not NUMBER_LIST and and UNIT_LIST and
> UNIT_SHORT_LIST ?

Because some of these are not helpful to the user :)  We have an opportunity to be a buffer layer between the API and the user, and I think we should take advantage of it when it makes sense.  For instance, we decided to implicitly convert numbers to their plural category if required by the context, which is a divergence from the pure Intl API and I think it's a great addition!

To reiterate, I prefer to keep things aligned with the Intl API as much as possible.
(In reply to Staś Małolepszy :stas from comment #6)
> Would you keep PLURAL as currently implemented, i.e. as an alias to NUMBER?

Until we land PluralRules, sure (which is going to happen soon :)).
 
> Will Intl.PluralRules implement type=ordinal and/or type=range?

type=ordinal, definitely, type=range, we're still discussing, it may come later.
(Assignee)

Comment 8

2 years ago
We talked about this on the call today and IIUC, the consensus was to remove PLURAL for the time being until Intl.PluralRules is implemented and we figure out the common API for using ICU/Intl in FTL.

Is that right?
Comment on attachment 8802882 [details] [review]
Pull request

yes!
Attachment #8802882 - Flags: review- → review+
(Assignee)

Comment 11

2 years ago
https://hg.mozilla.org/projects/larch/rev/03c8f612cf5a53eb8a0625ba178580e9aeab7935
Bug 1311662 - Remove the PLURAL builtin function. r=Pike r=gandalf
(Assignee)

Comment 12

2 years ago
When the Intl.PluralRules API is ready (bug 1270146) we can revisit the FTL API for it.
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.