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

kdecapplet@joejoetv: Update translation files and cleanup unnecessary file #6478

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rcalixte
Copy link
Member

@rcalixte rcalixte commented Oct 9, 2024

@JoeJoeTV
Copy link
Contributor

JoeJoeTV commented Oct 9, 2024

@rcalixte You added gettext translation calls (_(...)) to all the display names in modules.js. But these calls are not missing, they are simply delegated to the Settings Widget (see settingsWidget.py, line 223), where the text in the columns is translated, so that the settings file doesn't contain any already translated strings, which would make switching the language afterwards more difficult.

Also a note: Since the applet is GPL3, shouldn't the .pot file represent this instead of saying it's public domain?

The rest reems fine to me, although i'm not sure why the colons were replaced with dots.

@rcalixte
Copy link
Member Author

rcalixte commented Oct 9, 2024

But these calls are not missing, they are simply delegated to the Settings Widget (see settingsWidget.py, line 223)

So this function translates them but the strings weren't being detected due to the formatting. That is what the purpose of this pull request is. Sorry that I wasn't more clear.

so that the settings file doesn't contain any already translated strings, which would make switching the language afterwards more difficult.

Can you explain this a little bit more? I'm not sure I understand.

Also a note: Since the applet is GPL3, shouldn't the .pot file represent this instead of saying it's public domain?

This is just a built-in of xgettext and how it phrases things. As I understand, it should mean the same thing?

although i'm not sure why the colons were replaced with dots

This assists in translations in the case of some graphical tools like Poedit. The proper format means that the strings now show in the additional context for translators.

Feel free to let me know what else you need to feel comfortable here.

@rcalixte
Copy link
Member Author

rcalixte commented Oct 9, 2024

But these calls are not missing, they are simply delegated to the Settings Widget (see settingsWidget.py, line 223), where the text in the columns is translated, so that the settings file doesn't contain any already translated strings, which would make switching the language afterwards more difficult.

If it's easier, we can make a dummy.js file or something like that. That would get picked up by the translation scripts in the repository just the same too. It's just that the current text file does not get automatically picked up and the manual addition is something we can implement much better.

@JoeJoeTV
Copy link
Contributor

JoeJoeTV commented Oct 9, 2024

Can you explain this a little bit more? I'm not sure I understand.

Well, I'm using the custom settings widget so one can manage the modules that KDEConnect provides in the settings.
This data is more or less dynamic, as the version of KDE Connect and the modules it provides, aswell as the devices that can also be managed there, can change. Thus the widget uses those dynamic values and puts some of them (the display names) through the gettext translate function, as they would have translations.
This works fine with multiple languages, as the value in the config file is not translated yet. If I now were to translate it in the applet and save that to the config file, the translation in the settings widget wouldn't work anymore, if the language is different, as the string is not the original english "id".

This is just a built-in of xgettext and how it phrases things. As I understand, it should mean the same thing?

Well, previously, the file contained the following:

# Copyright (C) YEAR THE PACKAGE'S COPYRIGHT HOLDER
# This file is distributed under the same license as the PACKAGE package.

(Yes it is missing some values that I didn't fill in, but it still has the text that mentiones that it has the same license)
But pulbic domain and having a license is still very different.

This assists in translations in the case of some graphical tools like Poedit. The proper format means that the strings now show in the additional context for translators.

Interesting, as I did edit this file in PoEdit and didn't notice that

If it's easier, we can make a dummy.js file or something like that. That would get picked up by the translation scripts in the repository just the same too. It's just that the current text file does not get automatically picked up and the manual addition is something we can implement much better.

That could be an option to replace the txt file.

@rcalixte
Copy link
Member Author

Well, previously, the file contained the following:

Right this change was a different flag in xgettext and that's where the string in the header comes from. Take a look at the two consecutive flags here for clarification. Let us know if you think this should be changed.

That could be an option to replace the txt file.

I'll push this now for your review.

@rcalixte rcalixte force-pushed the kdec_cleanup branch 2 times, most recently from 0223966 to a8c684e Compare October 10, 2024 01:16
@JoeJoeTV
Copy link
Contributor

Right this change was a different flag in xgettext and that's where the string in the header comes from. Take a look at the two consecutive flags here for clarification. Let us know if you think this should be changed.

Yeah it was a failure on my part to fill out the fields for the pot file properly before, but I would like it to be filled with the copyright and the mention that the file is under the same license as the package like it was before, which I'm assuming is added when specifiying the string as the option parameter. Would that be fine this way?

I'll push this now for your review.

I would maybe make it more clear that the entire file is just there such that the strings get picked up for translation.
Something like:

// This dummy file is used to make sure that some strings get picked up for translation and doesn't have any other usecase in the project currently.

@rcalixte
Copy link
Member Author

but I would like it to be filled with the copyright and the mention that the file is under the same license as the package like it was before, which I'm assuming is added when specifiying the string as the option parameter.

What should the xgettext command look like in this case?

I would maybe make it more clear that the entire file is just there such that the strings get picked up for translation.

Done.

@JoeJoeTV
Copy link
Contributor

JoeJoeTV commented Oct 17, 2024

What should the xgettext command look like in this case?

I just used cinnamon-xlet-makepot to generate the .pot files originally. Do you already have a command you're using?

@rcalixte
Copy link
Member Author

I just used cinnamon-xlet-makepot to generate the .pot files originally. Do you already have a command you're using?

The cinnamon-spices-makepot in the root of each project. The cinnamon-xlet-makepot script has strong desire to be rewritten but is problematic because it ships with Cinnamon and therefore all users won't be guaranteed to have the same version. The script in the root of the repository should be the one that is run. When this is automated in the future, that will be the script that is run.

@JoeJoeTV
Copy link
Contributor

@rcalixte Ok, but to suggest a xgettext command, I would like to know if you already have one that I could modify, or if I should create one from scratch as I haven't used xgettext before, so modifying would be straight forward.

@rcalixte
Copy link
Member Author

@rcalixte Ok, but to suggest a xgettext command, I would like to know if you already have one that I could modify, or if I should create one from scratch as I haven't used xgettext before, so modifying would be straight forward.

I'd mentioned before that it is what is used in the cinnamon-spices-makepot script in the root of this repository.

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