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 plugin credit and check level minimums #3513

Merged
merged 2 commits into from
Nov 20, 2024
Merged

Fix plugin credit and check level minimums #3513

merged 2 commits into from
Nov 20, 2024

Conversation

Digitalone1
Copy link
Contributor

@Digitalone1 Digitalone1 commented Nov 20, 2024

I tried to implement ftm::format to fix plugin credits translations, then discovered QString can make formatting with arg() method. Passing the result of i18n I got a i18n_argument_missing.

Later I made a search and went to this:

In which I discovered that we can do the same in QML. So it was more easy than we though, but it's weird there's no documentation on that system and I had to find this solution on a bugtracker.

Besides I saw you initialized plugin peak levels to the minimum linear value, but this was weird to me because those class properties are in dB units (we pass them to QML to show levels). So I changed them to the minimum db level (defined in util.hpp), but this led to show -100 on all levels when the app is started (which is reasonable since nothing is playing, but maybe this was not your purpose).

So in order to show 0 at the start when nothing is playing (the same happens on libadwaita), I initialized them to 0.0F.

At last, I added a check for -99 db like we did in libadwaita for the output levels.

Update. I forgot to mention the following: Is there a way to translate new QML strings? I'd like to test those plugin credits in my local language. I suppose I won't find them in weblate.

Copy link

github-actions bot commented Nov 20, 2024

Workflow failed, but the following artifacts are still available for this pull request:

@wwmm
Copy link
Owner

wwmm commented Nov 20, 2024

which is reasonable since nothing is playing, but maybe this was not your purpose

I do not remember. At some point Idecided to change the place where the conversion to decibel was done when compared to the gtk branch. I probably forgot that the class member were still initialized like the old way.

but it's weird there's no documentation on that system and I had to find this solution on a bugtracker.

There is but somehow I did not notice the comment about argument substitution https://api.kde.org/frameworks/ki18n/html/prg_guide.html. Or maybe the link I found before is another one.

Update. I forgot to mention the following: Is there a way to translate new QML strings? I'd like to test those plugin credits in my local language. I suppose I won't find them in weblate.

Translations are handled in a very different way in KDE's land. It took me a while to figure out how to adapt KDe's example to our needs. But long story short do

cd util
./extract-translation-messages.sh

This will update the translation template and the language files. It may be needed to install it somewhere to make the new translation strings active. In case you need to do this the procedure I have been doing is

cd easyeffects
mkdir build
cd build
cmake -DCMAKE_INSTALL_PREFIX=/tmp/ee -G Ninja ..
ninja install

@wwmm wwmm merged commit 579820e into wwmm:eeqt Nov 20, 2024
7 of 10 checks passed
@wwmm
Copy link
Owner

wwmm commented Nov 20, 2024

I decided to do some tests after uninstalling the gtk based easyeffects package and the strings that were translated before are now in English. This is a sign that the translation framework is searching for strings in the system path and not the custom installation folder. Hum... I will see if I can make it replicate the old behavior somehow.

@wwmm
Copy link
Owner

wwmm commented Nov 20, 2024

This is a sign that the translation framework is searching for strings in the system path and not the custom installation folder.

For testing purposes I installed the qt branch in the system path /usr and translations worked as expected. So it is just a matter of forcing Qt/KDE to search in a different place. Having to install in the system path to test translations is far from ideal. The gtk branch only manages that because Meson is forcing during the compilation a custom path directly in the source code. I wonder if the same gettext calls we forced on gtk can be used on KDE or if something specific has to be used.

@wwmm
Copy link
Owner

wwmm commented Nov 20, 2024

I decided to use /usr/local for testing and this also works. Honestly it may be better to just test through a ninja install to /usr/local. The chances of this causing problems are almost zero. And I am not sure if it is a good idea to mess with the gettext configuration Qt and KDE are doing just to make it a little easier to test the translations. Once the qt branch becomes the master we will just use our Arch Linux pkgbuild scripts to test this kind of thing anyway.

By the way the code you implemented for the Using credits works
image

@Digitalone1
Copy link
Contributor Author

So, If I understood correctly, we have to install the development build under /usr/ to test the translations. There's no other way we can do the same from build/src/ inside the git folder?

@wwmm
Copy link
Owner

wwmm commented Nov 21, 2024

So, If I understood correctly, we have to install the development build under /usr/ to test the translations.

No. Use /usr/local as install path. It may not seem at first but there is a huge difference. Historically this is a place reserved for local installations done by the users through means like make install. Nowadays most Linux users do not think about it. But 20 years ago we used to do this more often 😄 .

@Digitalone1
Copy link
Contributor Author

Digitalone1 commented Nov 21, 2024

No. Use /usr/local as install path. It may not seem at first but there is a huge difference. Historically this is a place reserved for local installations done by the users through means like make install. Nowadays most Linux users do not think about it. But 20 years ago we used to do this more often 😄 .

Okay, but if I make the build normally, I won't see new translations, right?

@wwmm
Copy link
Owner

wwmm commented Nov 21, 2024

Okay, but if I make the build normally, I won't see new translations, right?

I saw them. I had to update the brazilian translation for Using %1 using poedit for it to take effect. You just have to make sure you uninstall your easyeffects package first. Gettext probably gives higher priority to /usr paths when compared to /usr/local.

@wwmm
Copy link
Owner

wwmm commented Nov 21, 2024

Okay, but if I make the build normally, I won't see new translations, right?

Just building and running from the build folder does not work even in the gtk branch. It also needs that we install the binary somewhere.

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