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

Fix for issue 348 #648

Open
wants to merge 11 commits into
base: 1.19.x
Choose a base branch
from
Open

Fix for issue 348 #648

wants to merge 11 commits into from

Conversation

WVam
Copy link

@WVam WVam commented Mar 3, 2023

This is meant to fix issue #348. Please revise this and make sure it actually works, because I have no idea how to do that.

However, here how it's supposed to work.

In all translation files, there should be the following strings:

"patchouli.gui.lexicon.edition_str.ord.uu"
"patchouli.gui.lexicon.edition_str.ord.00"
"patchouli.gui.lexicon.edition_str.ord.10"
"patchouli.gui.lexicon.edition_str.ord.20"
"patchouli.gui.lexicon.edition_str.ord.30"
"patchouli.gui.lexicon.edition_str.ord.40"
"patchouli.gui.lexicon.edition_str.ord.50"
"patchouli.gui.lexicon.edition_str.ord.60"
"patchouli.gui.lexicon.edition_str.ord.70"
"patchouli.gui.lexicon.edition_str.ord.80"
"patchouli.gui.lexicon.edition_str.ord.90"
"patchouli.gui.lexicon.edition_str.ord.100"

These strings define the behavior for certain categories of numbers:

  • The .uu string, for numbers between 1 and 9 inclusive.
  • The .00 string, for numbers > 10 whose tens digit and hundreds digit are 0.
  • Strings .10 to .90, for numbers whose tens digit is 1 to 9.
  • The .100 string, for numbers whose tens digit is 0 and whose hundreds digit is not 0.

These strings should be translated as one of the following:

  • "%1$s<suffix>": All numbers in that category will use the suffix. (Of course, it could also be a prefix if you write it before the variable. Same goes in all other cases.)
  • "%2$s": All numbers in that category will use a generic suffix based on the number's units digit. These can be defined by translating the strings "patchouli.gui.lexicon.edition_str.ord.0" through "patchouli.gui.lexicon.edition_str.ord.9" as "%1$s<suffix>".
  • "%3$s": All numbers in that category will use a suffix based on the number's units AND tens digits. These can be defined by translating the strings "patchouli.gui.lexicon.edition_str.ord.<tens>.0" through "patchouli.gui.lexicon.edition_str.ord.<tens>.9", where <tens> should be replaced with uu, 10, 20, ecc… depending on the category of numbers that it applies to. Those strings can be translated as "%1$s<suffix>" (if you want to use a special suffix) or as "%2$s" (if you want to use the units suffix, i.e. "patchouli.gui.lexicon.edition_str.ord.<units>").

Since negative ordinal numbers don't exist, I've also made it so that using negative numbers returns the "Writer's Edition".

Also regarding the Writer's Edition, since in some languages "Writer's" might be positioned differently from the ordinal, "Writer's Edition" is now its own string, completely independent from "%s Edition" which is only for ordinals.

@TheRealWormbo
Copy link
Contributor

Let me know whether I understood this implementation correctly, please.

  • Book version 0 continues to be replaced by the book's subtitle.

  • Negative or non-numerical versions only use the patchouli.gui.lexicon.dev_edition translation now, which is no longer piped through the patchouli.gui.lexicon.edition_str translation (makes sense, as it would e.g. be "Autorenausgabe" in German, not something like "Autoren Ausgabe")

  • For all positive numbers this examines the number to put it into one of twelve categories (the "tens"):

    • "uu" for the single-digit numbers 1-9
    • "00" for numbers ending in 00-09, except those ending in 000-009.
    • "10", "20", …, and "90" for numbers ending in 10-19, 20-29, …, and 90-99, respectively
    • "100" for numbers ending in 000-009

    Then the translation key patchouli.gui.lexicon.edition_str.ord.<tens> can use an indexed format string to select between three formatting options:

    1. "%1$s" – Just the number itself, i.e. the selected "tens" category is the only source of formatting. In English this would cover all numbers ending in 10-19, which all get the suffix "th". (Many languages will probably use this option for all categories if they denote ordinals just with a period as the suffix.)
    2. "%2$s" – A value formatted according to the translation key patchouli.gui.lexicon.edition_str.ord.<ones> (where "ones" is the least significant digit of the number). This is where the translation logic for any English numbers lives that don't have a 1 as their second least significant digit (e.g. "1st", "22nd", "53rd", etc.)
    3. "%3$s" – A value formatted according to the translation key patchouli.gui.lexicon.edition_str.ord.<tens>.<ones>. This won't be used in English, but could cover any language where ordinals are specific to the last two digits somehow.

Assuming the above interpretation corresponds to your intention, this implementation looks good in my opinion. Having rebased it onto the 1.20.x branch locally, I gave it a test run with a couple different book version numbers and it seemed to work as I described above.

There are just two little things left to do:

  • Remove the now unused ORDINAL_SUFFIXES array definition at the top of the class.
  • Run the spotlessApply Gradle task to fix any formatting inconsistencies.

(I don't know if it makes sense to fix the now incorrect values of the patchouli.gui.lexicon.dev_edition entry in other languages by running "Writer's edition" through Google's translator to get passable translations. Those languages would need updates to their ordinal formatting anyway.)

@WVam
Copy link
Author

WVam commented Feb 19, 2024

Yes, your understanding of my implementation is correct.

As for the other languages, I suggest that you put in "Writer's Edition" as it would currently appear and then let whoever takes care of that language's translation fix it if needed. Of course, if you know any of those langauges, then it would be better if you fixed it yourself.

Is there anything else that I need to do or can you take care of those two remaining things you mentioned? I'd rather avoid touching any more code since I know next to nothing about Java and even less about GitHub workflows, tests and the like.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants