-
Notifications
You must be signed in to change notification settings - Fork 85
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
fixes for language filtering to support private-use subtags #330
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@litvinovg please check my comments before we merge your contribution
String[] subtags = lang.split(SEPARATOR); | ||
int length = subtags.length; | ||
for (int i = 1; i < length; i++) { | ||
int lastIndex = length - i; | ||
if (PRIVATE_USE_SUBTAG.equals(subtags[lastIndex - 1])) { | ||
continue; | ||
} | ||
String baseLang = String.join(SEPARATOR, Arrays.copyOfRange(subtags, 0, lastIndex)); | ||
if (!lang.equals(baseLang) && !this.contains(baseLang)) { | ||
this.add(baseLang); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe I need this code for this PR. However, I am wondering can we centralized working with languages/locales, at the moment we are struggling with acceptable languages and format of a language tag at a couple of places:
public List<Locale> getCandidateLocales(String baseName, Locale locale) { - and
Vitro/api/src/main/java/edu/cornell/mannlib/vitro/webapp/utils/LocaleUtility.java
Line 15 in 9230bb3
localeString.matches("^[a-z]{1,3}_[A-Za-z]{2}_x_[a-z]{1,}") ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also, please notice that if we define this language tag in the runtime.properties
fr_CA_x_uqam
once when it is loaded in a Locale object, the code
locale.toLanguageTag().toString()
will produce
fr-CA-x-uqam
while the code
locale.toString()
will produce
fr_CA_#x-uqam
I believe this might introduce some issues, for instance here - https://github.com/vivo-project/Vitro/blob/9230bb3f23ffbfd58347ae61bb7ce55bd0d006d3/api/src/main/java/edu/cornell/mannlib/vitro/webapp/utils/searchengine/SearchQueryUtils.java#L211:L217
Added my approval. @chenejac is the directive not to merge from 27 Sep still valid or stale at this point? |
I would suggest to postpone merging of this PR. I am hoping we might have much wider picture and complete solution for usage of private tags in the next sprint. |
What does this pull request do?
Fixes created list of tags for cases with private use sub-tags.
Example
For requested language fr-CA-x-uqam creates list of acceptable [ fr-CA-x-uqam, fr-CA , fr ]
How should this be tested?
Unit tests are included in this PR
Interested parties
@chenejac
@brianjlowe