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 Latin hyphenation #6910

Merged
merged 3 commits into from Nov 26, 2020
Merged

Add Latin hyphenation #6910

merged 3 commits into from Nov 26, 2020

Conversation

jperon
Copy link
Contributor

@jperon jperon commented Nov 24, 2020

This PR is also an issue: some months ago (I don't record exactly when), it was possible to choose hyphenation from a big list, an latin hyphenation thanks to this file, generated from this project was possible.

Since the only option depends on "typographical rules":

  • I tried to add the modification proposed in this PR;

  • I edited data/hyph/languages.json, adding following lines:

    {
        "name": "Latin",
        "filename": "Latin.pattern",
        "language": "la",
        "left_hyphen_min": 2,
        "right_hyphen_min": 2,
        "aliases": ["lat"]
    },

I'd have expected Latin.pattern to be used as hyphen dictionary; but the only option I have is English-US, although typographical rules are correctly set to la.


This change is Reviewable

@NiLuJe
Copy link
Member

NiLuJe commented Nov 24, 2020

This is because things now require a bit more metadata on CRe's side (c.f., koreader/crengine#383, koreader/crengine#378, koreader/crengine#376, koreader/crengine#372) ;).

@poire-z
Copy link
Contributor

poire-z commented Nov 24, 2020

Yes, since a few months ago, some stuff is now hardcoded into crengine.
What you did is just the frontend part, which just makes stuff available in the menu.
You'll need to do a little PR in crengine (see https://github.com/koreader/crengine/commits/master/cr3gui/data/hyph for the little it takes to add each language, some can have a little bit more tweaks if needed).

@jperon
Copy link
Contributor Author

jperon commented Nov 24, 2020

Just submitted as koreader/crengine#393

@poire-z
Copy link
Contributor

poire-z commented Nov 26, 2020

koreader-base updated with the crengine bits, you'll have to update koreader-base in here (you'll be bringing 5 other little commits with it, no worry).

@poire-z poire-z merged commit 16b37b5 into koreader:master Nov 26, 2020
@Frenzie Frenzie added this to the 2020.12 milestone Nov 26, 2020
@@ -66,6 +66,8 @@ local LANGUAGES = {
{ "it", {"ita"}, "H ", _("Italian"), "Italian.pattern" },
{ "ja", {}, " ", _("Japanese") },
{ "ko", {}, " ", _("Korean") },
{ "la-lit", {"lat-lit"}, "H ", _("Latin (liturgical)"), "Latin.pattern" },
Copy link
Member

Choose a reason for hiding this comment

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

Looks like I'm too late, but that should of course be Latin_liturgical.pattern. ;-)

Copy link
Contributor

Choose a reason for hiding this comment

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

Going to fix that (no real issue, just a display one, we'll show the hyph filename instead of this pretty language name).
While I'm at it, should these be re-ordered, so the shorter/plain name is first, before the next longer/specific ones?

     { "ko",                    {},   "    ",   _("Korean") },
-    { "la-lit",       {"lat-lit"},   "H   ",   _("Latin (liturgical)"),
     { "la",               {"lat"},   "H   ",   _("Latin"),
+    { "la-lit",       {"lat-lit"},   "H   ",   _("Latin (liturgical)"),
     { "lv",               {"lav"},   "H   ",   _("Latvian"),
@@ -77,9 +77,9 @@ local LANGUAGES = {
     { "pms",                   {},   "H   ",   _("Piedmontese"),
-    { "pt-BR",                 {},   "HB  ",   _("Portuguese (BR)"),
     { "pt",               {"por"},   "HB  ",   _("Portuguese"),
+    { "pt-BR",                 {},   "HB  ",   _("Portuguese (BR)"),
     { "rm",               {"roh"},   "H   ",   _("Romansh"),
     { "ro",               {"ron"},   "H   ",   _("Romanian"),
+    { "ru",               {"rus"},   "Hb  ",   _("Russian"),
     { "ru-GB",                 {},   "Hb  ",   _("Russian + English (UK)"),
     { "ru-US",                 {},   "Hb  ",   _("Russian + English (US)"),
-    { "ru",               {"rus"},   "Hb  ",   _("Russian"),
     { "sr",               {"srp"},   "HB  ",   _("Serbian"),

Copy link
Contributor

Choose a reason for hiding this comment

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

(It needs to be in the current order in crengine to match longer ones first - but in frontend, this order does not matter.)

Copy link
Member

Choose a reason for hiding this comment

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

Definitely makes more sense to me.

Copy link
Contributor

Choose a reason for hiding this comment

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

Done with #6913.

Frenzie pushed a commit that referenced this pull request Nov 26, 2020
Also fix just added "Latin (liturgical)" hyph filename.
See #6910 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants