search

jake-phy / WindowIconList

GNU General Public License v2.0
75 stars 26 forks source link

Improve the locale management #164

Open ncelerier opened 7 years ago

ncelerier commented 7 years ago

Hello,

While doing a new translation language for this applet, I have had a look at the code related to locale management and I think I can propose improvements. I would like to have feedback before implementing the changes. Maybe I'm wrong for some things since I'm only having a look at applet development since 2 weeks.

Don't commit generated .mo files

It doesn't look like a good idea to commit the .mo files because they are generated from .po files. They can be generated automatically with scripts, so that there seems to be no need to commit them. Also, it is harder to maintain, because you can see that there is a he.po file but no he.mo file so we have to take extra care of them.

I propose to remove the mo folder.

Rely on Cinnamon Install/Uninstall applet process

It looks like that Cinnamon handles localization during the install/uninstall process.

When someone install an applet, Cinnamon check the po folder for localizations and compile them to *.mo file into the ~/.local/share/locale/xx/LC_MESSAGES folder.

When someone uninstall an appplet, Cinnamon also remove these files from LC_MESSAGES folder.

That's what I understand from other applet like the Weather one.

Have a look at the if condition at line 474 of the Cinnamon code that handle this: https://github.com/linuxmint/Cinnamon/blob/f075328c6e983b3cae3c4bf0a0fe5e05f0c8f29c/files/usr/share/cinnamon/cinnamon-settings/bin/Spices.py

The only thing that annoys me is that I don't know how to re-install an installed applet to test that it is working the way I describe it.

I propose to rename locale folder to po folder.

Applet.js, method execInstallLanguage is not called anymore

Since commit ee37b02 (almost 2 years ago), the execInstallLanguage method is not called anymore.

Do we still need this method ? If not, I propose to remove it. If yes, it should be documented why we keep it.

Multiple files to handle locale management

There are multiple files that help us for locale management:

That's really a helpful file. I propose to move it from the po folder to the root folder of this applet so that this folder only contains .po and .pot files.

This file shouldn't be useful anymore since we can rely on the Cinnamon install/uninstall process. Also this is a subset of what localeUpdate.sh is doing.

I propose to remove it.

I'm not sure that we need this file. It does 2 things:

The uninstall procedure is not removing the .mo files from LC_MESSAGES. This is a minor bug.

Can we remove these file, or is it being used for older versions of Cinnamon were the install/uninstall is not present ?

What do you think ?

ncelerier commented 7 years ago

I have just seen this discussion: https://github.com/jake-phy/WindowIconList/issues/46

I have had a further look at the execInstallLanguage method, and sometimes it is commented, sometimes not. As @lestcape said, it might be a good idea to rename the locale folder to po, so that the execInstallLanguage method is only used in development mode and it doesn't slow down.

Maybe would it be nice, if execInstallLanguage is kept, that it compile the .po to .mo, so we don't have to commit them.

But what is the use case for keeping this execInstallLanguage method ? Indeed, if you don't install the applet via Spices, the localization install does not happen. Is it a common use case ? I guess "simple" users use Spices. Maybe not always ? I don't know. Maybe the translators and developers can use the updateLocale.sh script rather than relying on execInstallLanguage ? Is this the role of the applet to manage localization ? Shouldn't we rather rely on scripts for this ?

Maybe it would be nice to have a Gituhb repo that contains scripts for xlets developers and translators. Because I feel like scripts like localeUpdate.sh would be useful for other xlets. And copy-pasting it in every xlets doesn't seem like a good idea (duplication). But that's another problem :smile:

ghost commented 7 years ago

Hi, @ncelerier as i was who implemented this mechanisms "execInstallLanguage" and also who request the implementation of the extension translation in cinnamon (https://github.com/linuxmint/Cinnamon/issues/2245), and who fixed it for work properly with UTF-8 encode (https://github.com/linuxmint/Cinnamon/pull/2855). Also who create the bash script localeUpdate.sh So yes, i know what and why you are talking about of execInstallLanguage and the usage of po files.

The reason of execInstallLanguage: It's because to install an applet or desklet and apply the cinnamon translation mechanism that you perfectly described, you will need an internet connexion or at less know a little about how properly install the applet translation. In others words if you install the applet manually, you will not have translations!!!! I know you probably think this is not a problem because you live in Belgium not? In Africa or Latin America an internet conection is not possible for all people. In Cuba, my country, it's like a dream...

The reason of po folder, it's because this is a open source app, and mo files are not the source code. I know there are softwares to read mo and generate po files from the mo, but po it's the source.

That's my personal reason, but of course, i don't force anyone to think like me...

ncelerier commented 7 years ago

Hello lestcape,

Thank you for your answer. Now I understand why execInstallLanguage is useful. Indeed, in countries like Belgium, good internet connection is common.

It would be nice if Cinnamon was managing the language when you add the applet to the panel. So that there would be no need anymore of language management in the applet itself. This is something we can propose on the Cinnamon project. Maybe it already is, I'll have a look. But in the meanwhile, we will have to live with this, and this is why execInstallLanguage is useful.

I guess configure.py is useful too for the use case where people don't install via Internet and don't know where they have to install the applets. But does configure.py script need to do language management since we can rely on execInstallUpdate (updated to compile .po to .mo) ? I think it is not.

So if I understand properly, there are 3 uses cases:

As it seems to be now, there is not enough documentation in the official documentation website, and we have to go through multiple applets code to check how things have been done, sometimes differently, scratching your head to understand why something has been done that way or for which use case. That's why it is important to discuss and understand the need of each other. Maybe we can propose guidelines for other applets. Having a repository for these kind of utility scripts and methods can be useful too.

Also it would be nice to extract the execInstallLanguage from the applet.js file, having a file that contains the exectInstallLanguage method that would be called by the applet.

About .mo files, I think it would be better to remove them. The execInstallLanguage method would need to be updated to be able to compile the .po to .mo. The cons is that it has a hit on performance, but not that much.

About using the Cinnamon Spices language management, we can rename the locale folder to po folder to use it. Scripts can be updated to check first if the language is already present so that work is not done multiple times.

This is my personal reason too, I'm just having a look at how things are done and how we could improve things :smile:

ghost commented 7 years ago

@ncelerier you have right and you are clear... I just explain why i create this mechanism for my applets. Of course i only help here with the thing that i do and without take on account the configure.py script, because this also force user to run another process for the installation. So, that's why i don't use gsettings and instead y use the cinnamon-settings to save persistent properties.

But again yes, probably have the execInstallLanguage and the configure.py it's to much. I prefer, remove the configure and not use gsettings to save properties. Thats again to simplify the applet installation from users.

About install language when applet will add to the panel, it's not a good idea, because on cinnamon start he will need to do that for all applet at once and this could be pretty slow...

What i considerer that need to be done on cinnamon, it's create a mechanism for install extension locally, that's will resolve all problems at once: https://github.com/linuxmint/Cinnamon/issues/2280