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

Add multilingual field exceptions #9124

Merged
merged 5 commits into from
May 21, 2022
Merged

Add multilingual field exceptions #9124

merged 5 commits into from
May 21, 2022

Conversation

wcedmisten
Copy link
Contributor

@wcedmisten wcedmisten commented May 21, 2022

Summary

Closes #9043

Adds exceptions for name:left, name:right, name:etymology, name:signed, and name:source when opening the multilingual tag.

Testing

Can be tested by adding these tags to a feature, and verifying that no multilingual field appears.

image

image

Copy link
Collaborator

@1ec5 1ec5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this contribution! I must admit I’m going to miss seeing the etymology displayed so prominently, which hopefully impressed mappers as to our thoroughness, but openstreetmap/id-tagging-schema#256 would be the more correct way to surface that information anyhow.

@@ -116,6 +116,9 @@ export function uiFieldLocalized(field, context) {

// update _multilingual, maintaining the existing order
function calcMultilingual(tags) {
// tags that use the format name:<item>, but
// aren't languages
const nonLanguageTags = ["left", "right", "source", "signed", "etymology"]
Copy link
Collaborator

@1ec5 1ec5 May 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good start, though quite a few common non-language subkeys could be added to this list, either in this PR or in a followup. Off the top of my head, there’s also pronunciation,1 prefix, and genitive.

To avoid a slow trickle of additions, perhaps we could match only the subkeys that follow the BCP 47 pattern. The following regular expression is similar to the one StreetComplete introduced in streetcomplete/StreetComplete@04fd11a, but it also covers country-specific subkeys, such as the Hong Kong and Taiwan variants of Traditional Chinese.

[a-z]{2,3}([A-Z][a-z]{3})?([A-Z]{2})?

Another syntax for transliteration is still in widespread use, for example name:ja_rm for romaji, but it’s deprecated, so we don’t need to go out of our way to support it.

/ref streetcomplete/StreetComplete#2723 (comment)

Footnotes

  1. Technically, name:pronunciation should be name:*-fonipa, but no one is using that syntax.

@wcedmisten
Copy link
Contributor Author

wcedmisten commented May 21, 2022

@1ec5 Thanks for the feedback! This is my first time contributing here, so my initial approach was attempting to be more permissive of nonstandard tags, but I do agree restricting the locale matching would be a better approach.

With your feedback in mind, I've changed the regex to match on 2-3 letters, optionally followed by a hyphen and 4 letters.

This seems to match the streetcomplete commit you referenced.

My main question is if this is too disruptive to non-standard uses of name:<language>, that may not comply with this format. Again, I'm fairly new here, so not sure how to survey the usage of these tags, or what the etiquette is around changing that behavior. Let me know what you think. Thanks!

modules/ui/fields/localized.js Outdated Show resolved Hide resolved
@1ec5 1ec5 added bug A bug - let's fix this! field An issue with a field in the user interface labels May 21, 2022
@1ec5 1ec5 merged commit 67bd692 into openstreetmap:develop May 21, 2022
bhousel added a commit to facebook/Rapid that referenced this pull request Jan 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A bug - let's fix this! field An issue with a field in the user interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Multilingual field exceptions
2 participants