Add initial version of Latin locale #966

tukusejssirs · 2020-07-23T05:44:47Z

Fixes #961

codecov · 2020-07-23T05:46:29Z

Codecov Report

Merging #966 into dev will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##               dev      #966   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          172       175    +3     
  Lines         1547      1641   +94     
  Branches       328       359   +31     
=========================================
+ Hits          1547      1641   +94

Impacted Files	Coverage Δ
src/locale/la.js	`100.00% <100.00%> (ø)`
src/locale/br.js	`100.00% <0.00%> (ø)`
src/locale/ca.js	`100.00% <0.00%> (ø)`
src/locale/de.js	`100.00% <0.00%> (ø)`
src/locale/sr.js	`100.00% <0.00%> (ø)`
src/locale/en-nz.js	`100.00% <0.00%> (ø)`
src/locale/sr-cyrl.js	`100.00% <0.00%> (ø)`
src/plugin/duration/index.js	`100.00% <0.00%> (ø)`
src/plugin/localeData/index.js	`100.00% <0.00%> (ø)`
src/plugin/objectSupport/index.js	`100.00% <0.00%> (ø)`
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2e1e426...40d4d4e. Read the comment docs.

iamkun · 2020-07-29T02:36:33Z

There's no need to create a unit test compared with moment.js's result. Just compare with normal string, please.

- fix outputing month names in nominative and genitive cases; - add full stop (`.`) after `monthsShort` values; - move `isNaN()` test at the beginning of `romanise()` function; - revert romanising years as it does not work ATM; - add tests of: - names of months in nominative and genitive; - relatime time (based on `cs.test.js`).

tukusejssirs · 2020-10-01T19:22:40Z

I have updated this PR with additional tests. I tried to create tests for everything in la.js, but the coverage output still says it is not 100 %. Could you explain to me why is it so? Thanks.

node node_modules/jest/bin/jest.js test/locale/la.test.js --coverage=true

 PASS  test/locale/la.test.js
  ✓ Latin: Test relative time (106ms)
  ✓ Latin: Test ordinal numbers (3ms)
  ✓ minimal weekday names (2ms)
  Latin: Test output of month names:
    ✓ in genitive case if day of month is output (11ms)
    ✓ in nominative if day of month is not output (3ms)
    ✓ also as a shortcut (2ms)
  Latin: Test weeday names:
    ✓ full weekday names (2ms)
    ✓ short weekday names (1ms)
    ✓ minimal weekday names (2ms)

----------------------------|----------|----------|----------|----------|-------------------|
File                        |  % Stmts | % Branch |  % Funcs |  % Lines | Uncovered Line #s |
----------------------------|----------|----------|----------|----------|-------------------|
All files                   |    85.54 |    59.39 |    75.71 |    86.84 |                   |
 src                        |    88.36 |    62.73 |    78.43 |    89.35 |                   |
  constant.js               |      100 |      100 |      100 |      100 |                   |
  index.js                  |    85.79 |    61.87 |    77.78 |    86.55 |... 57,364,368,399 |
  utils.js                  |    96.15 |    68.18 |    83.33 |      100 |       14,24,28,43 |
 src/locale                 |    95.24 |    83.33 |      100 |    95.24 |                   |
  en.js                     |      100 |      100 |      100 |      100 |                   |
  la.js                     |    95.24 |    83.33 |      100 |    95.24 |                20 |
 src/plugin/advancedFormat  |    45.83 |     8.33 |       75 |    45.83 |                   |
  index.js                  |    45.83 |     8.33 |       75 |    45.83 |... 33,35,37,39,41 |
 src/plugin/localizedFormat |    84.62 |       40 |       60 |      100 |                   |
  index.js                  |    84.62 |       40 |       60 |      100 |       15,16,17,20 |
 src/plugin/relativeTime    |    88.57 |    85.71 |    57.14 |    90.63 |                   |
  index.js                  |    88.57 |    85.71 |    57.14 |    90.63 |          57,75,78 |
----------------------------|----------|----------|----------|----------|-------------------|
Handlebars: Access has been denied to resolve the property "statements" because it is not an "own property" of its parent.
You can add a runtime option to disable the check or this warning:
See https://handlebarsjs.com/api-reference/runtime-options.html#options-to-control-prototype-access for details
Handlebars: Access has been denied to resolve the property "branches" because it is not an "own property" of its parent.
You can add a runtime option to disable the check or this warning:
See https://handlebarsjs.com/api-reference/runtime-options.html#options-to-control-prototype-access for details
Handlebars: Access has been denied to resolve the property "functions" because it is not an "own property" of its parent.
You can add a runtime option to disable the check or this warning:
See https://handlebarsjs.com/api-reference/runtime-options.html#options-to-control-prototype-access for details
Handlebars: Access has been denied to resolve the property "lines" because it is not an "own property" of its parent.
You can add a runtime option to disable the check or this warning:
See https://handlebarsjs.com/api-reference/runtime-options.html#options-to-control-prototype-access for details
Test Suites: 1 passed, 1 total
Tests:       9 passed, 9 total
Snapshots:   0 total
Time:        2.716s
Ran all test suites matching /test\/locale\/la.test.js/i.

tukusejssirs · 2020-10-02T08:47:30Z

@iamkun, okay, I’ve found the cause of the non-100-% issue: NaN in romanise() function was the culprit, which was ignored by eslint (as it is disabled for the function), but it was still caught by Codecov.

Anyway, now the code is working, passes all tests (npm test + Travis + Codecov), therefore from my point of view this PR is mergable.

As a sidenote/question: could I open a new issue for dom/month/year romanisation that would be done within dayjs? I personally don’t plan implementing it, but it might be a nice feature that is not found in any other date/time implementation, but it is still used by real humans (e.g. in Latin and Hungarian). Only one of dom/month/year could be romanised at a time and it could be implemented as a dayjs flag/option, like romanise = 'year' (possible options: false, year, month, dom; default: false).

tukusejssirs · 2020-10-05T12:46:10Z

Added a commit with small update: now la.js is fully complying the ESLint rules (there is no eslint-disable.

tukusejssirs · 2020-10-15T11:51:29Z

Is there anything that blocks this PR from being merged? Is there anything else I need to do before this PR could be merged?

iamkun · 2020-10-15T13:05:45Z

Cool, thx. Waiting for native speaker review ( 0 / 1 )

tukusejssirs · 2020-10-15T15:55:37Z

@iamkun, I see.

@igneus, could you review this PR from the linguistic point of view? We (the romcal dev/users) need this merged, because otherwise dayjs with default to English locale.

Some notes to guide to in the review to do it real quick:

When I started the work on this PR, I asked some people at LatinDiscussion.com to proof the Latin used in the code. You might want to read it.
The la.js file contains the following:
- on line 4: month names in genitive (used in dates, where there is at least a day of month and month, while month is written out);
- on line 5: month names in nominative (used otherwise);
- on lines 17-31:
  - there is function romanise() that converts Arabic numerals to Roman numeral;
  - note that currently, it is only used in ordinal numbers (line 42; see below: on line 42), because a romanisation module won’t get into the dayjs codebase (for more info, see this comment or even the whole conversation in Add Latin locale #961);
- on line 35: full weekday names as used in the Missale Romanum (ed. typ. tertia);
- on line 36: shorter weekday names (Saturday and Sunday are not shortened, for weekdays I used feria + number);
- on line 37: short/abbreviated weekday names;
- on line 39: abbreviated month names;
- on line 42:
  - the format of ordinal numbers;
  - it is only used when the day of month (D) is followed by o (lowercase letter o); this is how dayjs uses it (I can’t do anything about it);
  - I’ve read somewhere that it is followed by ᵒ (Modifier Letter Small O; U+1D52), however it is not obligatory;
- on lines 45-57: relative time:
  - as currently, I don’t know of a way to output all forms in all grammer cases, therefore I decided to include only the forms in nominative;
  - the strings are quite easy; in case you need there English translation, see e.g. en-gb.js (L13-26).

PS—@igneus, sorry if I the above description is too detailed, however, I wish you’d find some time to review this PR ASAP. However, I don’t want to hurry you nor push you into anything. 😃

igneus

I looked through the code and tests and so far haven't found any obvious errors.

But it has to be clearly said that the authority of my approval is quite low. I'm just a theologian, no Latin expert, and without any training in "modern Latinity".

test/locale/la.test.js

igneus · 2020-10-15T20:49:27Z

src/locale/la.js

+  ordinal: n => [romanise(n), 'ᵒ'].join(''),
+  // The relative time variables are in nominative case only
+  relativeTime: {
+    future: 'ad %s',


(Yes, I'm lazy to install the library, understand it and try myself.)
@tukusejssirs would you please post one (preferably non-trivial) example for both relative time in future and in the past?

You can see the output in the locale test (L64-80), in the T constant (it is a multi-dimensional array consisting of [number, 'English word', 'number Latin word']; I think it is quite readable 😉).

No, it isn't readable. It's code, DRY as a desert.

Okay, here’s a simple ‘conversion‘ table:

English Latin

a few seconds paucæ secundæ

a minute minuta

2-∞ minutes 2-∞ minutæ

an hour hora

2-∞ hours 2-∞ horæ

a day dies

2-∞ days 2-∞ dies

a month mensis

2-∞ months 2-∞ menses

a year annus

2-∞ years 2-∞ anni

Update: I have updated the table to better reflect that the plural forms are used for any number from 2-∞ interval.

Just pinging you, @igneus. When you find some time, please check the table above if it is sufficiently readable for you. All other issues you raised were fixed in the latest commit.

I'm not sure I'll manage to communicate what I really wanted to see, so I cloned your repo and checked out the latin branch with hope that I will be able to play with the code myself, but it turns out that my NodeJS skills don't suffice to load the package from development sources.

I ran npm run-script build, managed to load dayjs.min.js in the interactive console (var dayjs = require('dayjs.min.js');), but attempts to load locales fail, probably due to my lack of understanding of some basic NodeJS concepts.

Since I personally don't care much about dayjs' Latin locale, I probably just give up and 🙈

I'm not sure I'll manage to communicate what I really wanted to see

I’m sorry I didn’t understand you. Anyway, I still think that you proofed everything in the Latin locale except for those string in the ‘conversion’ table.

Anyway, in the Czech locale, they use different inflection of those strings when they are used in the past (e.g. a few seconds ago) or future tense (in a few seconds). If any of the strings from relativeTime should be in different grammar case in the past and future tenses, we would need to update it accordingly.

I cloned your repo and checked out the latin branch with hope that I will be able to play with the code myself, but it turns out that my NodeJS skills don't suffice to load the package from development sources.

All you should need to do after cloning the repo is as follows:

# Install the dependencies npm install # Build `dayjs` # Note: This is same as running `npm run-script build` npm run build # Run the `la.test.js` test # Note: Run the following command from repo root or modify both relative paths node node_modules/jest/bin/jest.js test/locale/la.test.js --coverage=false

I think the easiest way to test things without any deeper knowledge of Node.js, it to modify the test/locale/la.test.js to include console.log() command(s), where you put the output of whatever you want to see.

Since I personally don't care much about dayjs' Latin locale, I probably just give up and see_no_evil

Pinging @igneus. Are you determined to give up on this review? As I understand it, you have not reviewed only one part, the relativeTime.

Ping, @igneus. No answer from you for more than a month. I presume you are occupied with some other stuff, however, I’d be very happy if you at least say something. 😉 Thanks in advance with all your help! 😃

tukusejssirs · 2020-10-15T21:14:40Z

I looked through the code and tests and so far haven't found any obvious errors.

Thanks, @igneus, for such a prompt review! 👍

But it has to be clearly said that the authority of my approval is quite low. I'm just a theologian, no Latin expert, and without any training in "modern Latinity".

IMHO, still better a review than no review. 😉

Anyway, I aim this PR to add Ecclesiastical Latin, i.e. one used in the current (port Vatican II) liturgical books.

- remove `ᵒ` from ordinal numbers; - use lowercase first letter in `dominica` and `sabbato`.

tukusejssirs mentioned this pull request Aug 4, 2020

Add Latin locale romcal/romcal#207

Merged

tukusejssirs mentioned this pull request Sep 17, 2020

fix: fix DST bug in utc plugin #1053

Merged

tukusejssirs force-pushed the latin branch from cdcaf52 to 312af8b Compare September 17, 2020 09:33

tukusejssirs marked this pull request as draft September 17, 2020 14:55

tukusejssirs mentioned this pull request Sep 18, 2020

Date romanisation (day of month, month number, year) romcal/romcal#196

Closed

tukusejssirs added 3 commits October 1, 2020 17:46

Add initial version of Latin locale

12a2f14

Use Arabic numerals for days of month

2ecfece

tukusejssirs force-pushed the latin branch from 7b4dcd4 to 3c98c0f Compare October 1, 2020 15:48

Fix some issues and add more tests

cf85d6a

Remove a number test from the romanise() function (it contained NaN)

2e26255

tukusejssirs marked this pull request as ready for review October 2, 2020 08:39

tukusejssirs mentioned this pull request Oct 4, 2020

Add a commit hook to generate a list of calendar propers romcal/romcal#234

Open

15 tasks

Update la.js to fully comply with .eslintrc.json config

049bf1d

iamkun added the Need Review label Oct 15, 2020

igneus reviewed Oct 15, 2020

View reviewed changes

Fix some issues raised by @igneus

40d4d4e

- remove `ᵒ` from ordinal numbers; - use lowercase first letter in `dominica` and `sabbato`.

tukusejssirs mentioned this pull request Oct 20, 2020

Fix Czech locale issues romcal/romcal#125

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add initial version of Latin locale #966

Add initial version of Latin locale #966

tukusejssirs commented Jul 23, 2020 •

edited

Loading

codecov bot commented Jul 23, 2020 •

edited

Loading

iamkun commented Jul 29, 2020

tukusejssirs commented Oct 1, 2020

tukusejssirs commented Oct 2, 2020

tukusejssirs commented Oct 5, 2020

tukusejssirs commented Oct 15, 2020

iamkun commented Oct 15, 2020

tukusejssirs commented Oct 15, 2020

igneus left a comment

igneus Oct 15, 2020

tukusejssirs Oct 15, 2020

igneus Oct 15, 2020

tukusejssirs Oct 15, 2020 •

edited

Loading

tukusejssirs Oct 19, 2020 •

edited

Loading

igneus Oct 21, 2020

tukusejssirs Oct 23, 2020

tukusejssirs Nov 5, 2020

tukusejssirs Dec 7, 2020

tukusejssirs commented Oct 15, 2020 •

edited

Loading

English	Latin
a few seconds	paucæ secundæ
a minute	minuta
2-∞ minutes	2-∞ minutæ
an hour	hora
2-∞ hours	2-∞ horæ
a day	dies
2-∞ days	2-∞ dies
a month	mensis
2-∞ months	2-∞ menses
a year	annus
2-∞ years	2-∞ anni

Add initial version of Latin locale #966

Are you sure you want to change the base?

Add initial version of Latin locale #966

Conversation

tukusejssirs commented Jul 23, 2020 • edited Loading

codecov bot commented Jul 23, 2020 • edited Loading

Codecov Report

iamkun commented Jul 29, 2020

tukusejssirs commented Oct 1, 2020

tukusejssirs commented Oct 2, 2020

tukusejssirs commented Oct 5, 2020

tukusejssirs commented Oct 15, 2020

iamkun commented Oct 15, 2020

tukusejssirs commented Oct 15, 2020

igneus left a comment

Choose a reason for hiding this comment

igneus Oct 15, 2020

Choose a reason for hiding this comment

tukusejssirs Oct 15, 2020

Choose a reason for hiding this comment

igneus Oct 15, 2020

Choose a reason for hiding this comment

tukusejssirs Oct 15, 2020 • edited Loading

Choose a reason for hiding this comment

tukusejssirs Oct 19, 2020 • edited Loading

Choose a reason for hiding this comment

igneus Oct 21, 2020

Choose a reason for hiding this comment

tukusejssirs Oct 23, 2020

Choose a reason for hiding this comment

tukusejssirs Nov 5, 2020

Choose a reason for hiding this comment

tukusejssirs Dec 7, 2020

Choose a reason for hiding this comment

tukusejssirs commented Oct 15, 2020 • edited Loading

tukusejssirs commented Jul 23, 2020 •

edited

Loading

codecov bot commented Jul 23, 2020 •

edited

Loading

tukusejssirs Oct 15, 2020 •

edited

Loading

tukusejssirs Oct 19, 2020 •

edited

Loading

tukusejssirs commented Oct 15, 2020 •

edited

Loading