Add missing keys to CollatorPreferences

[jedel1043]

Adds support for kn and kf on CollatorPreferences.

[hsivonen]

Looks OK to me, but I let @zbraniecki review the macro usage.

[zbraniecki]

Thank you for the PR! The only thing I'd like to ponder over is that you're introducing an example where our sepration of Preferences and Options gets blurry - the developer sets an option, and user may set another. We have to decide who takes precedence.

[jedel1043]

@zbraniecki FWIW, on ECMA402 explicit configs take priority over unicode keys, so I expressed that here. If we want to add more flexibility for it, I think we can add an option to explicitly prioritise either one of them.

[zbraniecki]

q: Should we panic here or debug_assert and in release select [default]?

[zbraniecki]

Why you don't need a catch case despite CaseFirst being non-exhaustive?

Should we switch Options to use CollactionCaseFirst from Preferences then?

[jedel1043]

#[non_exhaustive] only applies for external crates that use the enum.

I preserved the old enum because I wanted to avoid unnecessary breaking changes, but we can switch to that if preferred.

[zbraniecki]

We have a shot at doing it in 2.0, I think i'd prefer that. @sffc @Manishearth @robertbastian - you ok with that?

[sffc]

I have a slight preference for removing the settings from options and moving them to preferences in 2.0

[Manishearth]

Same.

[jedel1043]

Just to be clear – we would move the relevant options to CollatorPreferences, using the structs on icu_locale_core, and the rest of the options would still be in CollatorOptions, is that right?

[zbraniecki]

correct!

[jedel1043]

Ok, switched it to use preference keyword structs and removed the old config structs. Had to slightly adjust the documentation to mention CollatorPreferences, but it should be feature-complete.

[zbraniecki]

Oh, interesting. Am I correct to read your PR as - you removed the two preferences from Options, and allow for setting it by the developer by overriding Preferences?

In other words if the developer wants to set one of those two, they interact with Preferences bag, not Options bag anymore?

[jedel1043]

@zbraniecki Yes. That way, the problem we had about priority is solved in an arguably cleaner way, since users can either extend the implicit locale preferences with explicit preferences to give priority to the latter, or do the opposite to give priority to the former.

[zbraniecki]

Yes. That way, the problem we had about priority is solved in an arguably cleaner way, since users can either extend the implicit locale preferences with explicit preferences to give priority to the latter, or do the opposite to give priority to the former.

Ok, so you think this is even a cleaner approach! I value your perspective as an implementer. I think I have a hard time deciding on the tradeoffs around it.
My main concern would be that Preferences are intended to be out of control for developers - they accept it (from Locale and from Regional Prefs of their system) and pass it on. The Options on the other hand are meant to be set by the developer. I thought this is a nice clean design but of course reality check is that there are cases where Prefs and Options duplicate and you brought up this here.

With your change we're telling developers to touch Preferences manually. The value of it is that, as you rightly pointed out, it is clean and allows developers to control which direction they want to merge - options->prefs, or prefs->options. ECMA-402 specifies one, but that doesn't mean everyone has to repeat it.

The downsides are:

1. we make the distinction between Prefs and Options murky. "Prefs are for customers, but if you want to set X, modify them, as you do with Options"
2. we're now explicitly requiring implementers to think of this nuance and risk getting it wrong or not at all.

I think I'm fine with that as the benefit outweights the cost for me but wanted to have this explicit paper trail of this decision to revisit in the future.

[zbraniecki]

lgtm! thank you!