preserve language tags #1354

devsnek · 2022-12-29T18:21:32Z

linux-foundation-easycla · 2022-12-29T18:21:38Z

The committers listed above are authorized under a signed CLA.

✅ login: devsnek / name: snek (3511882)

diesieben07 · 2022-12-30T21:45:19Z

This is unfortunately not correct when the language tag contains a private use subtag (-x-). For example en-US-x-twain must turn into en-US-u-nu-hanidec-x-twain when combined with nu-hanidec. Your code would turn it into en-US-x-twain-u-nu-hanidec, which has a different meaning.

icambron · 2022-12-30T22:34:28Z

src/impl/locale.js

@@ -75,22 +75,26 @@ function parseLocaleString(localeStr) {
    return [localeStr];
  } else {
    let options;
+    let selectedStr;
    const smaller = localeStr.substring(0, uIndex);


You can now move this substring into the catch. It was only outside of it because we used it in the return.

icambron · 2022-12-30T22:36:51Z

test/datetime/create.test.js

@@ -695,7 +695,7 @@ test("DateTime.fromObject overrides the locale string with explicit settings", (
    }
  );

-  expect(res.locale).toBe("be");
+  expect(res.locale).toBe("be-u-ca-coptic-nu-mong");
  expect(res.outputCalendar).toBe("islamic");
  expect(res.numberingSystem).toBe("thai");
 });


Can you also add a test here where you set the locale to something with a calendar and an explicit outputCalendar and show that it doesn't blow up (I don't care which calendar "wins", but the test should document the result, i.e. which calendar comes out of the resolved options)

i think this is that test?

oh, ha, you're right. My bad

devsnek · 2022-12-30T23:29:45Z

@diesieben07 do you have any suggestion for what should be done? Does this need to support private use subtags?

diesieben07 · 2022-12-30T23:45:29Z

do you have any suggestion for what should be done?

You could either detect the -x- and insert the -u- at the appropriate position, but I think this would require more extensive changes of the code.
Alternatively you could strip out any -x- before appending (or throw an error), because I don't think the browser cares about any -x- tags. The problem with the current approach is that there won't be any error, the appended -u- will just be ignored by the browser.

Does this need to support private use subtags?

Probably not. But currently this code is a regression, because previously they were just ignored and now they cause the outputCalendar and numberingSystem to stop working.

Edit: I just want to clarify, that I am not a maintainer for this project, so I do not have any decision power here!

icambron · 2023-01-03T14:14:41Z

Looks good. Thanks!

preserve language tags

046998a

devsnek force-pushed the language-tags branch from 3511882 to 046998a Compare December 30, 2022 00:19

devsnek marked this pull request as ready for review December 30, 2022 00:19

note test requirements

a986eb1

icambron reviewed Dec 30, 2022

View reviewed changes

review

619e8b2

icambron merged commit 806467a into moment:master Jan 3, 2023

sync-by-unito bot mentioned this pull request Jan 23, 2023

Bump luxon from 3.1.1 to 3.2.1 SeedCompany/cord-api-v3#2719

Closed

devsnek deleted the language-tags branch February 7, 2023 07:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

preserve language tags #1354

preserve language tags #1354

devsnek commented Dec 29, 2022 •

edited

linux-foundation-easycla bot commented Dec 29, 2022 •

edited

diesieben07 commented Dec 30, 2022

icambron Dec 30, 2022

icambron Dec 30, 2022

devsnek Dec 31, 2022 •

edited

icambron Jan 3, 2023

devsnek commented Dec 30, 2022

diesieben07 commented Dec 30, 2022 •

edited

icambron commented Jan 3, 2023

preserve language tags #1354

preserve language tags #1354

Conversation

devsnek commented Dec 29, 2022 • edited

linux-foundation-easycla bot commented Dec 29, 2022 • edited

diesieben07 commented Dec 30, 2022

icambron Dec 30, 2022

Choose a reason for hiding this comment

icambron Dec 30, 2022

Choose a reason for hiding this comment

devsnek Dec 31, 2022 • edited

Choose a reason for hiding this comment

icambron Jan 3, 2023

Choose a reason for hiding this comment

devsnek commented Dec 30, 2022

diesieben07 commented Dec 30, 2022 • edited

icambron commented Jan 3, 2023

devsnek commented Dec 29, 2022 •

edited

linux-foundation-easycla bot commented Dec 29, 2022 •

edited

devsnek Dec 31, 2022 •

edited

diesieben07 commented Dec 30, 2022 •

edited