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

Added more Translation for DE #83

Open
wants to merge 15 commits into
base: feature/extend-i18n
Choose a base branch
from

Conversation

StoiaCode
Copy link
Contributor

Also cleaned up de_DE to be more in line with en_US
Checkcheck?

Copy link
Member

@Fayti1703 Fayti1703 left a comment

Choose a reason for hiding this comment

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

Please leave the tab-indentation intact, rather than replacing it with a space indentation.

In addition, formatting changes should always be in their own, separate commit to help with conflict-resolution when merging or cherry-picking.

Some additional notes that I couldn't place inline:

@@ -4,7 +4,7 @@
     "selfName": "Deutsch",
     "maintainers": [
       "@Fayti1703",
-                       "@EstoyMejor",
+      "@StoiaCode",
       "@chirpcodes",
       null
     ]

I see no need to drop them from the maintainers list.

In config.input.keybind.purpose.generic:

            "release": "Loslassen",

I intentionally did not translate that literally, since the functionality that this key is for doesn't really match up with the literal meaning.

In config.references:

      "explaination": "Zeige ein Refernezbild auf deinem Bildschirm, um dabei zu helfen die Pose exakt zu treffen.",

That's supposed to be Referenz, correct?

@@ -140,7 +149,9 @@
         "dialog": {
           "title": "Referenzbild hinzufügen",
           "filter": "Bild-Dateien"
-                               }
         },
-                       "addText": "Neues Referenzbild hinzufügen"
+        "addText": "Neues Referenzbild hinzufügen",
+        "add": "Add reference file",
+        "supportedTypes": ""
+      }
     },

add and supportedTypes are not used. There's no need to add them here.

In config.language:

      "noteWIP": "Achtung! Diese Einstellungen sind derzeit nur für das, sich in arbeit befindende, Übersetzungs system.",
      "contNoteWIP": "Die meisten Oberflächen sind bis jetzt nicht Übersetzt!",

Grammar-wise, this should be:

			"noteWIP": "Achtung! Diese Einstellungen sind derzeit nur für das sich in Arbeit befindende Übersetzungssystem.",
			"contNoteWIP": "Die meisten Oberflächen sind bis jetzt nicht übersetzt!",

In config.offset, all strings are still in English. I'd rather not have them present at all if we currently do not have a translation for them.

"The plus (+) button will insert a copied row");
Locale.GetString("config.offset.tipsTwo") +
Locale.GetString("config.offset.tipsThree") +
Locale.GetString("config.offset.tipsFour"));
Copy link
Member

Choose a reason for hiding this comment

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

Incorrect indentation.

Copy link
Member

Choose a reason for hiding this comment

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

Also, this should be a single string (including the initial Tips:\n) -- there's no reason to have this as three separate keys.

@StoiaCode
Copy link
Contributor Author

StoiaCode commented Dec 12, 2022

Tried to implement them.

will it correctly parse the \n from the locals? I implemented it in three parts since that was how it was in the original.

As for Estoy, thats me. Hi. I changed my name. :D Sorry

@Fayti1703
Copy link
Member

will it correctly parse the \n from the locals?

JSON has support for \n, so yes. It will parse those as a newline.

As for Estoy, thats me. Hi. I changed my name. :D Sorry

Oh, I see. My bad.

@Fayti1703 Fayti1703 force-pushed the feature/extend-i18n branch 2 times, most recently from 86ff3a2 to bb7d405 Compare January 1, 2023 15:44
@Fayti1703
Copy link
Member

Going to reserve this PR for a larger rebase (mainly bringing it up to date and splitting the changes into their own commits where appropriate).

You don't need to do anything, though if you could refrain from pushing additional changes, that'd be appreciated (though I suspect you didn't have plans for that anyway).

I'll keep the resulting combined diff pretty close to what is currently there, though I might add some comments where appropriate afterwards.

@StoiaCode
Copy link
Contributor Author

I've been waiting for the rework to continue translation :) so go ahead, I'll be picking up once you guys let me know it's 'ready' again :D

@Fayti1703 Fayti1703 closed this Jan 6, 2023
@Fayti1703 Fayti1703 force-pushed the feature/extend-i18n branch from 44cbeaa to 4a2b572 Compare January 6, 2023 21:40
@Fayti1703
Copy link
Member

Fayti1703 commented Jan 6, 2023

Urgh, I messed up a force-push and GitHub auto-closed the PR because there's no changes, which also prevents me from pushing the correct changeset up. (And it doesn't let me re-open the PR because no changes)

Could you add me as a collaborator on your fork, @StoiaCode?

@StoiaCode
Copy link
Contributor Author

Done!

@Fayti1703 Fayti1703 reopened this Jan 6, 2023
Copy link
Member

@Fayti1703 Fayti1703 left a comment

Choose a reason for hiding this comment

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

Sorry about that mess there... overall, this looks good; I have just one complaint:

"tips": "Tips:\nClick on a row to copy it into clipboard.\nCtrl + Shift + Right click to remove a row.\nThe plus (+) button will insert a copied row",
"addTooltip": "Add a line from clipboard.",
"dropTooltip": "Hold Ctrl and Shift to drop ALL bone offsets."
},
Copy link
Member

Choose a reason for hiding this comment

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

imo if there's no translation for a certain key yet (as is the case here), instead of copying from another language, those keys should just be omitted.

We can always implement a fallback system in code.

@Fayti1703 Fayti1703 self-assigned this Jan 18, 2023
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.

2 participants