ticket:13073 Adding API for setting custom compact data, for CLDR Survey Tool.

ticket:13073 Adding API for setting custom compact data, for CLDR Survey Tool.

[sffc, 2017-05-12 20:38:58]

Prepared by Mark's request for CLDR Survey Tool

[sffc, 2017-05-12 20:40:25]

On 2017/05/12 20:38:58, sffc wrote:
> Prepared by Mark's request for CLDR Survey Tool

Note: See the test case file for an example call site.  I can add a convenience
API (like the old constructor), but if there's only one user, it doesn't seem
necessary to make any extra API clutter.

[mark_macchiato.com, 2017-05-12 20:45:57]

Thanks for jumping on this.

Looks good except for one question I have. Is CompactDecimalFingerprint
externally accessible (don't have code here).

Mark

On Fri, May 12, 2017 at 1:40 PM, <sffc@google.com> wrote:

> On 2017/05/12 20:38:58, sffc wrote:
>
>> Prepared by Mark's request for CLDR Survey Tool
>>
>
> Note: See the test case file for an example call site.  I can add a
> convenience API (like the old constructor), but if there's only one
> user, it doesn't seem necessary to make any extra API clutter.
>
> https://codereview.appspot.com/320570044/
>

[sffc, 2017-05-12 21:17:32]

On 2017/05/12 20:45:57, mark_macchiato.com wrote:
> Thanks for jumping on this.
> 
> Looks good except for one question I have. Is CompactDecimalFingerprint
> externally accessible (don't have code here).
> 
> Mark
> 
> On Fri, May 12, 2017 at 1:40 PM, <mailto:sffc@google.com> wrote:
> 
> > On 2017/05/12 20:38:58, sffc wrote:
> >
> >> Prepared by Mark's request for CLDR Survey Tool
> >>
> >
> > Note: See the test case file for an example call site.  I can add a
> > convenience API (like the old constructor), but if there's only one
> > user, it doesn't seem necessary to make any extra API clutter.
> >
> > https://codereview.appspot.com/320570044/
> >

CompactDecimalFingerprint is a private static class which is used as a cache
key.  The compactCustomData property bypasses the cache though, so there
shouldn't be any reason to need to deal with CompactDecimalFingerprints for this
use case.

[mark_macchiato.com, 2017-05-12 21:19:54]

On Fri, May 12, 2017 at 2:17 PM, <sffc@google.com> wrote:

> CompactDecimalFingerprint


​So will passing in null work?​

Mark

[sffc, 2017-05-12 21:26:54]

I don't understand the question?

If you set the new compactCustomData property to null, you will get the
default data from the resource bundle, and if you set the property to your
custom *Map<String,Map<String,String>>*, you will get the data from the
custom map without going to the resource bundle.

To be clear, this is what the API for this feature looks like:

        CompactDecimalFormat cdf =
            CompactDecimalFormat.getInstance(
                ULocale.ENGLISH,
                CompactStyle.SHORT);
        // Java 7 style:
        cdf.setProperties(new PropertySetter() {
            @Override
            public void set(Properties props) {
                props.setCompactCustomData(customData);
            }
        });
        // Java 8 style:
        cdf.setProperties((Properties props) => {
            props.setCompactCustomData(customData);
        });

In other words, the option is not in the constructor.  I can make a
constructor like it was before, but I feel like we shouldn't clutter the
@internal API especially if there is only going to be one user.

On Fri, May 12, 2017 at 2:19 PM, Mark Davis ☕️ <mark@macchiato.com> wrote:

>
> On Fri, May 12, 2017 at 2:17 PM, <sffc@google.com> wrote:
>
>> CompactDecimalFingerprint
>
>
> ​So will passing in null work?​
>
> Mark
>

[mark_macchiato.com, 2017-05-12 21:31:29]

Sorry, user error. I was looking at the following, but that is not
relevant. (too much multitasking during UTC)

 538   static CompactDecimalData createDataFromCustom(
 539       DecimalFormatSymbols symbols,
 540       CompactDecimalFingerprint fingerprint,
 541       Map<String, Map<String, String>> powersToPluralsToPatterns) {

Mark

On Fri, May 12, 2017 at 2:26 PM, Shane Carr <sffc@google.com> wrote:

> I don't understand the question?
>
> If you set the new compactCustomData property to null, you will get the
> default data from the resource bundle, and if you set the property to your
> custom *Map<String,Map<String,String>>*, you will get the data from the
> custom map without going to the resource bundle.
>
> To be clear, this is what the API for this feature looks like:
>
>         CompactDecimalFormat cdf =
>             CompactDecimalFormat.getInstance(
>                 ULocale.ENGLISH,
>                 CompactStyle.SHORT);
>         // Java 7 style:
>         cdf.setProperties(new PropertySetter() {
>             @Override
>             public void set(Properties props) {
>                 props.setCompactCustomData(customData);
>             }
>         });
>         // Java 8 style:
>         cdf.setProperties((Properties props) => {
>             props.setCompactCustomData(customData);
>         });
>
> In other words, the option is not in the constructor.  I can make a
> constructor like it was before, but I feel like we shouldn't clutter the
> @internal API especially if there is only going to be one user.
>
> On Fri, May 12, 2017 at 2:19 PM, Mark Davis ☕️ <mark@macchiato.com> wrote:
>
>>
>> On Fri, May 12, 2017 at 2:17 PM, <sffc@google.com> wrote:
>>
>>> CompactDecimalFingerprint
>>
>>
>> ​So will passing in null work?​
>>
>> Mark
>>
>
>

[mark_macchiato.com, 2017-05-12 21:33:26]

So LGTM on the change.

Mark

On Fri, May 12, 2017 at 2:31 PM, Mark Davis ☕️ <mark@macchiato.com> wrote:

> Sorry, user error. I was looking at the following, but that is not
> relevant. (too much multitasking during UTC)
>
>  538   static CompactDecimalData createDataFromCustom(
>  539       DecimalFormatSymbols symbols,
>  540       CompactDecimalFingerprint fingerprint,
>  541       Map<String, Map<String, String>> powersToPluralsToPatterns) {
>
> Mark
>
> On Fri, May 12, 2017 at 2:26 PM, Shane Carr <sffc@google.com> wrote:
>
>> I don't understand the question?
>>
>> If you set the new compactCustomData property to null, you will get the
>> default data from the resource bundle, and if you set the property to your
>> custom *Map<String,Map<String,String>>*, you will get the data from the
>> custom map without going to the resource bundle.
>>
>> To be clear, this is what the API for this feature looks like:
>>
>>         CompactDecimalFormat cdf =
>>             CompactDecimalFormat.getInstance(
>>                 ULocale.ENGLISH,
>>                 CompactStyle.SHORT);
>>         // Java 7 style:
>>         cdf.setProperties(new PropertySetter() {
>>             @Override
>>             public void set(Properties props) {
>>                 props.setCompactCustomData(customData);
>>             }
>>         });
>>         // Java 8 style:
>>         cdf.setProperties((Properties props) => {
>>             props.setCompactCustomData(customData);
>>         });
>>
>> In other words, the option is not in the constructor.  I can make a
>> constructor like it was before, but I feel like we shouldn't clutter the
>> @internal API especially if there is only going to be one user.
>>
>> On Fri, May 12, 2017 at 2:19 PM, Mark Davis ☕️ <mark@macchiato.com>
>> wrote:
>>
>>>
>>> On Fri, May 12, 2017 at 2:17 PM, <sffc@google.com> wrote:
>>>
>>>> CompactDecimalFingerprint
>>>
>>>
>>> ​So will passing in null work?​
>>>
>>> Mark
>>>
>>
>>
>

[andy.heninger, 2017-05-12 21:46:31]

LGTM

[sffc, 2017-05-12 21:50:54]

On 2017/05/12 21:46:31, andy.heninger wrote:
> LGTM

Committed as r40117.

[mark_macchiato.com, 2017-05-17 01:03:56]

It turns out there is one further thing we need, which is the ability to
set the PluralRules. See http://unicode.org/cldr/trac/ticket/10291.

This is not a blocker for the General Submission, but we should fix it in
the next week. Is that something you could add?

Mark

On Fri, May 12, 2017 at 2:50 PM, <sffc@google.com> wrote:

> On 2017/05/12 21:46:31, andy.heninger wrote:
>
>> LGTM
>>
>
> Committed as r40117.
>
> https://codereview.appspot.com/320570044/
>

[sffc, 2017-05-17 02:25:43]

https://codereview.appspot.com/320590043/

On Tue, May 16, 2017 at 6:03 PM, Mark Davis ☕️ <mark@macchiato.com> wrote:

> It turns out there is one further thing we need, which is the ability to
> set the PluralRules. See http://unicode.org/cldr/trac/ticket/10291.
>
> This is not a blocker for the General Submission, but we should fix it in
> the next week. Is that something you could add?
>
> Mark
>
> On Fri, May 12, 2017 at 2:50 PM, <sffc@google.com> wrote:
>
>> On 2017/05/12 21:46:31, andy.heninger wrote:
>>
>>> LGTM
>>>
>>
>> Committed as r40117.
>>
>> https://codereview.appspot.com/320570044/
>>
>
>