Use `no` locale as synonym for `nb` by shihaohong · Pull Request #53880 · flutter/flutter

shihaohong · 2020-04-02T23:14:13Z

Description

Uses the no locale as a synonym for the nb locale. This is done because some systems like Android use nb to identify Norwegian Bokmål. However, in other systems, like internally at Google, no is used instead. This adds some logic to the Material and Cupertino localizations generation code to have nb be synonymous with no in the event that an _nb.arb file is not provided.

I switched _nb.arb files back to to _no.arb files, since that is how it is named and pulled down from the Google translation console internally. The name was manually updated to _nb.arb ever since #17788, but keeping it as _no.arb keeps things consistent with how these files are pulled down.

Related Issues

Fixes #53036

Tests

I added the following tests:

A test to ensure that nb is currently being generated as a synonym for no. However, this test wouldn't hold up in the event that an _nb.arb file is introduced with different message strings than no. Any feedback here would be appreciated on how to better test this behavior.

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I signed the CLA.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I updated/added relevant documentation (doc comments with ///).
All existing and new tests are passing.
The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
I am willing to follow-up on review comments in a timely manner.

Breaking Change

Did any tests fail when you ran them? Please read Handling breaking changes.

No, no existing tests failed, so this is not a breaking change.
Yes, this is a breaking change. If not, delete the remainder of this section.
- I wrote a design doc: https://flutter.dev/go/template Replace this with a link to your design doc's short link
- I got input from the developer relations team, specifically from: Replace with the names of who gave advice
- I wrote a migration guide: Replace with a link to your migration guide

shihaohong · 2020-04-03T00:00:16Z

dev/tools/localization/bin/gen_localizations.dart

  assert(supportedLanguagesConstant.isNotEmpty);
  assert(supportedLanguagesDocMacro.isNotEmpty);

+  bool isNbSynonymOfNo = false;


Maybe I can come up with a better variable name. It reads oddly when I say it out loud

The name seems OK (we're deep in a corner here) but a comment that referred to #53036 would be helpful

HansMuller

This change, and the analysis in #53036, looks good. Only concern is about the changes in material_nb.arb when it was renamed (?) material_no.arb.

HansMuller · 2020-04-03T14:18:03Z

dev/tools/localization/bin/gen_localizations.dart

  assert(supportedLanguagesConstant.isNotEmpty);
  assert(supportedLanguagesDocMacro.isNotEmpty);

+  bool isNbSynonymOfNo = false;


The name seems OK (we're deep in a corner here) but a comment that referred to #53036 would be helpful

HansMuller · 2020-04-03T14:18:14Z

dev/tools/localization/bin/gen_localizations.dart


  // If scriptCodes for a language are defined, we expect a scriptCode to be
  // defined for locales that contain a countryCode. The superclass becomes
-  // the script sublcass (e.g. `MaterialLocalizationZhHant`) and the generated


HansMuller · 2020-04-03T14:19:51Z

packages/flutter_localizations/lib/src/l10n/material_no.arb

@@ -48,9 +49,7 @@
  "reorderItemRight": "Flytt til høyre",
  "expandedIconTapHint": "Skjul",
  "collapsedIconTapHint": "Vis",
-  "remainingTextFieldCharacterCountZero": "TBD",


If this file was just renamed, why did the messages it contains change?

My description was misleading, I actually ran the translation console script to regenerate the _no.arb files in case anything had changed. I'm not sure why this would be removed instead of saying 'TBD'. Do we want to keep it as-is, or re-introduce the original arb file setup?

I see, didn't realize we were updating translations. Removing this line is OK; just means that there's no special case for zero.

Turns out there was a manual step to introduce the 'TBD' for any message that does not have a translation in google's translation console. I re-introduced the removed 'TBD' string.

With that, none of the strings are actually updated, the order is somehow different now though (alphabetical, which seems more correct).

HansMuller · 2020-04-03T14:20:59Z

packages/flutter_localizations/test/cupertino/translations_test.dart

    expect(localizations.timerPickerMinute(10), '10');
  });
+
+  // See https://github.com/flutter/flutter/issues/53036 for context on why


Please add the standard "Regression test for ..." comment as well.

HansMuller · 2020-04-03T14:21:56Z

packages/flutter_localizations/test/material/date_time_test.dart

      }, throwsAssertionError);
    });

+    test('`nb` uses `no` as a synonym when `nb` arb file is not present', () async {


Please add the standard "Regression test for ..." comment here.

HansMuller · 2020-04-03T14:22:13Z

packages/flutter_localizations/test/material/translations_test.dart

+
+  // See https://github.com/flutter/flutter/issues/53036 for context on why
+  // `no` is being used as a synonym for `nb`.
+  testWidgets('`nb` uses `no` as its synonym when `nb` arb file is not present', (WidgetTester tester) async {


Please add the standard "Regression test for ..." comment as well.

HansMuller

LGTM

HansMuller · 2020-04-03T16:59:35Z

packages/flutter_localizations/lib/src/l10n/material_no.arb

@@ -48,9 +49,7 @@
  "reorderItemRight": "Flytt til høyre",
  "expandedIconTapHint": "Skjul",
  "collapsedIconTapHint": "Vis",
-  "remainingTextFieldCharacterCountZero": "TBD",


I see, didn't realize we were updating translations. Removing this line is OK; just means that there's no special case for zero.

Shi Hao Hong and others added 8 commits April 2, 2020 15:04

Add _no.arb files

c977ed5

Generated files

33045ef

Add no <-> nb synonym usage in Material libraries

c63a51b

Add supportedLocales comment for "nb" and regenerated code

32ecf33

Reorder supportedLocales generation

56445a9

Regenerate code

6188715

Add test for Norwegian locale synonyms

af95b48

Fix supportedLocales for synonymous nb usage

4edc793

googlebot added the cla: yes label Apr 2, 2020

Add date localization generation, fix bugs

2c1118e

shihaohong changed the title ~~[WIP] Use no locale as synonym for nb~~ Use no locale as synonym for nb Apr 2, 2020

shihaohong added customer: dream (g3) severe: customer critical labels Apr 2, 2020

Run gen_localizations

639eb16

shihaohong commented Apr 3, 2020

View reviewed changes

HansMuller reviewed Apr 3, 2020

View reviewed changes

Code review feedback

d2ee61b

HansMuller approved these changes Apr 3, 2020

View reviewed changes

Reintroduce missing TBD in _no.arb

41ac3aa

shihaohong mentioned this pull request Apr 3, 2020

Discrepancy in updating translations in regenerating Material and Cupertino translations from scratch #53934

Closed

fluttergithubbot merged commit 365528a into flutter:master Apr 3, 2020

shihaohong deleted the norwegian-translations branch April 3, 2020 19:03

chunhtai mentioned this pull request Apr 8, 2020

[Web] broken gestures for mobile Safari #54269

Closed

This was referenced Aug 5, 2020

Update to the latest translations #62896

Closed

Remove Special Handling of Norwegian Locales #63034

Merged

github-actions bot locked as resolved and limited conversation to collaborators Jul 31, 2021

Conversation

shihaohong commented Apr 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issues

Tests

Checklist

Breaking Change

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HansMuller left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shihaohong Apr 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HansMuller left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

shihaohong commented Apr 2, 2020 •

edited

Loading

shihaohong Apr 3, 2020 •

edited

Loading