Create IsKnown* function for LanguageTag subtags [chromium/src : main]
Greg Thompson (Gerrit)
Greg Thompson added 7 comments![]( "Open in Gerrit")
source_set("bcp47_subtags") {Please fix this WARNING reported by autoreview issue finding: It looks like this `source_set` is never used. The generator actions (`:bcp47_languages_gen`, etc.) are already included directly in the `public_deps` of the `"internal"` target above. You can safely remove this leftover target.
namespace base::i18n::internal {`i18n_internal` as per comment in other cl
constexpr bool AreSubtagsKnown(const ParsedBcp47Tag& parsed_tag) {BCP 47 language tags are case-insensitive, but `ParseBcp47Tag` preserves the original case and the `IsKnown*` functions perform exact case-sensitive matches (e.g., `language` must be exactly lowercase, `script` titlecase).\n\nIf a user passes an unnormalized tag like `\"EN-US\"` to `ParseBcp47Tag`, this function will unexpectedly return `false` because `\"EN\"` doesn't match the known `\"en\"`.\n\nIs this intentional? If so, consider documenting that this function explicitly expects case-normalized subtags. Otherwise, we might need a case-insensitive lookup.
#include "base/containers/fixed_flat_set.h"unused?
please:
add a .style.yapf to this directory containing:
```
[style]
based_on_style = pep8
```
and then format this file with git cl format --python --full as per https://chromium.googlesource.com/chromium/src/+/main/styleguide/python/python.md#yapf
and add a PRESUBMIT.py with the usual copyright header and something along the lines of:
```
PRESUBMIT_VERSION = '2.0.0'
def CheckPylint(input_api, output_api):
"""Runs pylint on all directory content and subdirectories."""
return input_api.RunTests(input_api.canned_checks.GetPylint(input_api, output_api, version='3.2'))
```
see agents/shared/PRESUBMIT.py for a recent example. maybe just copy that one as-is.
if len(sys.argv) < 4:
print(
"Usage: generate_known_bcp47_subtags.py <output_file> <type> <enable_pseudolocales>"
)
sys.exit(1)
output_file = sys.argv[1]
subtag_type = sys.argv[2]
enable_pseudo = sys.argv[3] == "true"could you use `argparse` to do this nicely?
you can use the `description` trick (e.g., [see here](https://source.chromium.org/chromium/chromium/src/+/main:chrome/test/mini_installer/test_chrome_with_chromedriver.py;l=134)) so that the file-level docstring is the help text emitted by the script.
subtags += _PSEUDO_REGIONSPlease fix this WARNING reported by autoreview issue finding: This `+=` mutates the global `_REGIONS` list because `subtags` points to the same list. Although it doesn't affect correctness here since the script doesn't reuse `_REGIONS` and exits shortly after, it's safer to avoid mutating globals by creating a new list:\n```python\n subtags = _REGIONS + _PSEUDO_REGIONS\n```
Related details
- Danilo Tedeschi
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Danilo Tedeschi (Gerrit)
Danilo Tedeschi added 7 comments![]( "Open in Gerrit")
Please fix this WARNING reported by autoreview issue finding: It looks like this `source_set` is never used. The generator actions (`:bcp47_languages_gen`, etc.) are already included directly in the `public_deps` of the `"internal"` target above. You can safely remove this leftover target.
Done
`i18n_internal` as per comment in other cl
Done
constexpr bool AreSubtagsKnown(const ParsedBcp47Tag& parsed_tag) {BCP 47 language tags are case-insensitive, but `ParseBcp47Tag` preserves the original case and the `IsKnown*` functions perform exact case-sensitive matches (e.g., `language` must be exactly lowercase, `script` titlecase).\n\nIf a user passes an unnormalized tag like `\"EN-US\"` to `ParseBcp47Tag`, this function will unexpectedly return `false` because `\"EN\"` doesn't match the known `\"en\"`.\n\nIs this intentional? If so, consider documenting that this function explicitly expects case-normalized subtags. Otherwise, we might need a case-insensitive lookup.
I will add a comment and maybe add normalization later. I just don't want to end up with GetKnownLanguageTag("EN").tag_string() == "EN"
#include "base/containers/fixed_flat_set.h"Danilo Tedeschiunused?
Done
please:
add a .style.yapf to this directory containing:
```
[style]
based_on_style = pep8
```and then format this file with git cl format --python --full as per https://chromium.googlesource.com/chromium/src/+/main/styleguide/python/python.md#yapf
and add a PRESUBMIT.py with the usual copyright header and something along the lines of:
```
PRESUBMIT_VERSION = '2.0.0'def CheckPylint(input_api, output_api):
"""Runs pylint on all directory content and subdirectories."""
return input_api.RunTests(input_api.canned_checks.GetPylint(input_api, output_api, version='3.2'))
```see agents/shared/PRESUBMIT.py for a recent example. maybe just copy that one as-is.
Done
if len(sys.argv) < 4:
print(
"Usage: generate_known_bcp47_subtags.py <output_file> <type> <enable_pseudolocales>"
)
sys.exit(1)
output_file = sys.argv[1]
subtag_type = sys.argv[2]
enable_pseudo = sys.argv[3] == "true"could you use `argparse` to do this nicely?
you can use the `description` trick (e.g., [see here](https://source.chromium.org/chromium/chromium/src/+/main:chrome/test/mini_installer/test_chrome_with_chromedriver.py;l=134)) so that the file-level docstring is the help text emitted by the script.
Done
Please fix this WARNING reported by autoreview issue finding: This `+=` mutates the global `_REGIONS` list because `subtags` points to the same list. Although it doesn't affect correctness here since the script doesn't reuse `_REGIONS` and exits shortly after, it's safer to avoid mutating globals by creating a new list:\n```python\n subtags = _REGIONS + _PSEUDO_REGIONS\n```
- Greg Thompson
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Greg Thompson (Gerrit)
Greg Thompson voted Code-Review+1![]( "Open in Gerrit")
- Danilo Tedeschi
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |