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

updated german translation #1150

Merged
merged 2 commits into from
Jan 26, 2021
Merged

updated german translation #1150

merged 2 commits into from
Jan 26, 2021

Conversation

freddii
Copy link
Contributor

@freddii freddii commented Jan 26, 2021

@kliment
Copy link
Owner

kliment commented Jan 26, 2021

Hmm, wasn't the utils file fixed in #1146 by @DivingDuck?

msgid "Dis&connect"
msgstr "Trennen"
msgstr "Trennen&Verbinden"
Copy link
Owner

Choose a reason for hiding this comment

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

Can you explain what's happening here?

@@ -2321,27 +2320,27 @@ msgstr ""

#: printrun/settings.py:333
msgid "Custom device pattern: for example /dev/3DP_* "
msgstr ""
msgstr "Individuelle Geräte Struktur: zum Beispiel /dev/3DP_* "
Copy link
Owner

Choose a reason for hiding this comment

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

I'd use Muster rather than Struktur for this


#: printrun/settings.py:333
msgid "Device name pattern"
msgstr ""
msgstr "Struktur des Gerätenamen"
Copy link
Owner

Choose a reason for hiding this comment

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

Again, I'd use Muster rather than Struktur here

@kliment kliment merged commit 4f0d7d2 into kliment:master Jan 26, 2021
@freddii
Copy link
Contributor Author

freddii commented Jan 26, 2021

Hmm, wasn't the utils file fixed in #1146 by @DivingDuck?

for me its not working on Debian GNU/Linux bullseye/sid x86_64
It is only working when i use the file https://github.com/DivingDuck/Printrun/blob/3b0ea8224c1913ad13aa3dbf46049f0907dfd992/printrun/utils.py

@kliment
Copy link
Owner

kliment commented Jan 26, 2021

We merged @DivingDuck's version of the file into this repository's master in #1146 so the current utils.py in this repo should be the same now.

@freddii
Copy link
Contributor Author

freddii commented Jan 26, 2021

95e6830

and

DivingDuck@3b0ea82

are not the same.

@kliment
Copy link
Owner

kliment commented Jan 26, 2021

Oh I see! @DivingDuck can you have a look at this when you have a chance?

@DivingDuck
Copy link
Collaborator

DivingDuck commented Jan 26, 2021

Hi @kliment,
just see the this conversation. It is correct that my initial change from Gauges DivingDuck@ 3b0ea82 divers from 1146. There was two changes for this file from @hroncok regarding "Pick files from /usr/(local/)share with sys.prefix" bf9e2375 end of August which I didn't want to destroy.

Side note, my old Gauges branch will change hopefully this week as this was based on RC5. I will soon start to rebuild a new Gauges branch in my repository with the actual master RC7+ branch from here.

So the merged version from yesterday should be the correct one.

@freddii, I saw you did some updates for the German translation. Did you generate a new general .po file from the actual source files and then updated this?

@freddii
Copy link
Contributor Author

freddii commented Jan 26, 2021

I am running pronterface inside the git repo folder with:
python pronterface.py

if i print the variable shared_locale_dir from line 39 in line 40 from file utils.py with:
print("shared_local_dir = ", shared_locale_dir)

i get the output:
shared_local_dir = /usr/share/locale

Of course that path exists, but its not the correct path to pronterface translations that gets checked in line 43

If i print out that translation variable with:
print("translation = ", translation)

i get the output:
translation = <gettext.NullTranslations object at 0x7fe9963216d0>

Expected behaviour in my case would be:
running line 46 and not line 44 for translation value.

Removing lines 43-45 (if...else:) and bringing the line after that in the correct position, fixed it for me.

@DivingDuck
Copy link
Collaborator

@freddii , what is your OS?

@freddii
Copy link
Contributor Author

freddii commented Jan 26, 2021

Debian GNU/Linux bullseye/sid x86_64

@rockstorm101
Copy link
Collaborator

if i print the variable shared_locale_dir from line 39 in line 40 from file utils.py with:
print("shared_local_dir = ", shared_locale_dir)

i get the output:
shared_local_dir = /usr/share/locale

Of course that path exists, but its not the correct path to pronterface translations that gets checked in line 43

Expected behaviour in my case would be:
running line 46 and not line 44 for translation value.

Removing lines 43-45 (if...else:) and bringing the line after that in the correct position, fixed it for me.

I can confirm the described behaviour (also on a Debian Sid machine). However I believe '/usr/share/locale' is the right place where the program should look for translations. When properly installed on a Linux system, that's where the translations should be. Instead of checking whether 'shared_locale_dir' exists I would check for the existence of {pronterface,plater}.mo in 'shared_local_dir'. If not, then search in './locale'.

@freddii
Copy link
Contributor Author

freddii commented Jan 26, 2021

Yes but right now the file utils.py in line 43 is checking if the directory /usr/share/locale exists.
And when it exists it assumes that the translation of pronterface.mo is located there.
But that directory holds all translations of all programs, so its always existing in linux.
It should check if the file /usr/share/locale/de/LC_MESSAGES/pronterface.mo exists, not only the directory /usr/share/locale

Another idea for fix:
Doing it the other way round.
Checking first if ./locale and else ...

@DivingDuck
Copy link
Collaborator

Well, the relative location ./locale is originally for windows installations. The extension of the path .../country code/LC_MESSAGES/... is done by the gettext library.

In the past we had hard coded locations that was proofed for Linus/OSX and Windows:

'/usr/share/locale' (Linux ?)
'/usr/local/share/locale' (OS-X ?)
'./locale' (as fallback for Windows that don't use these kind of organized structures)

What should we do? I can do the hard coding again for the translation, but I'm not happy about this as I like the idea not to have hard coding for different paths.
We maybe need to go back to my old version, if 'DATADIR = os.path.join(sys.prefix, 'share')' won't work for all operating systems,

@hroncok , any thoughts?

Testing for each possible translation isn't a realistic option for me as I want to have translations open for everyone to add w/o the need of compiling pronterface new with each additional language.

I'm open for your suggestions.

@freddii
Copy link
Contributor Author

freddii commented Jan 26, 2021

this fixed it for me. Maybe you can confirm if its still working on windows.

@rockstorm101
Copy link
Collaborator

How about we first try to get the translation from the 'shared_locade_dir', and if that fails, default to './locale'. I haven't checked what error (if any) are thrown by 'gettext'.

def install_locale(domain):
    shared_locale_dir = os.path.join(DATADIR, 'locale')
    translation = None
    lang = locale.getdefaultlocale()

    try: 
        translation = gettext.translation(domain, shared_locale_dir, languages=[lang[0]])
    except FileNotFoundError:
        translation = gettext.translation(domain, './locale', languages=[lang[0]], fallback= True)

    translation.install()

@DivingDuck
Copy link
Collaborator

DivingDuck commented Jan 26, 2021

The fix from @freddii works for windows as the fallback is now the first check. No change for windows users in the end.
@rockstorm101, your version is working too. Tested with and w/o translation file available

A question, that is still open for me, will it work for OS-X and other Linux distributions?

Edit: @rockstorm101, , fallback= True for both because this is the fallback to original file at root of /locale (*.pot) if there is no translation file available (language file selection is based on OS language). I should think first...

@freddii
Copy link
Contributor Author

freddii commented Jan 26, 2021

@rockstorm101 version is working here too and seams to be more professional.

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.

4 participants