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

GUI: Add UI language option #2604

Merged
merged 22 commits into from
May 24, 2021
Merged

Conversation

SunderB
Copy link
Contributor

@SunderB SunderB commented Dec 27, 2020

An implementation of an option to change the language of the Sonic Pi GUI (without CMD args or changing the system language).

Features implemented:

  • Adds UI language option to the Qt GUI
  • Loads specified language on startup
  • When the language fails to load, an error message will be shown
  • When the user selects a new language, the user is asked to confirm the new language, then is given the option to restart Sonic Pi

Potential ideas for the future (not in the scope of this PR):

  • Changes the language instantly when the option is changed (this seemed like it was tricky to get working without potentially changing how the docs window works)

Closes #1506
Based on #2017

Feedback and help in completing this & testing this on different platforms is needed and greatly appreciated :)


Checklist:

  • Add language option

    • Add option in the settings widget
    • Add language menu to toolbar
    • Load and save the option
    • Load the specified language on startup
    • Show error message if the selected language translations fail to load
    • Theme message boxes to match the selected theme
  • Properly detect system language on Windows (to be tested)

  • Pass GitHub Builds

    • Pass Windows build
    • Pass macOS build
    • Pass Linux build
  • Testing

    • Tested on Windows
    • Tested on macOS
    • Tested on Linux (Debian Buster)

@SunderB
Copy link
Contributor Author

SunderB commented Dec 27, 2020

I realise this is a lot of changes in various parts of both the server and the GUI, so if it would be better to split this into multiple different PRs then I'd be happy to try to do so. Also, if it would help debugging, I can try to rebase the commits to separate the changes of the different parts of the code :)

@SunderB SunderB force-pushed the patch-localeOption branch 2 times, most recently from 18d6ce6 to d5a425a Compare January 29, 2021 20:47
@SunderB
Copy link
Contributor Author

SunderB commented Jan 29, 2021

I think this implementation is mostly done. It needs testing to fix any bugs and to make sure it works on all platforms.

In particular the macOS GitHub action build seems to be failing for some reason (it seems like an issue to do with file paths and packaging? but I don't have a Mac to test this on). Help with testing and seeing what's wrong there would be great :)

@samaaron
Copy link
Collaborator

Lovely, I'll look at this soon :-)

Thanks so much for your hard work.

@SunderB SunderB force-pushed the patch-localeOption branch 2 times, most recently from ea74447 to 15feed0 Compare February 4, 2021 17:25
@samaaron samaaron changed the base branch from main to dev February 12, 2021 17:03
@samaaron
Copy link
Collaborator

Hi @SunderB,

I've taken a look at this and in addition to it failing the CI, there's quite a lot of changes in this PR that make it hard to see the precise goals.

Would it be at all possible to either modify this PR or create another which has a clear statement and description of the suggested changes and then just the minimum code changes necessary to achieve this. All changes that do nice refactoring and tidying etc should ideally be separate PRs to keep things easy to follow and review.

@SunderB SunderB changed the title [GUI Feature]: Add GUI language option, improve system lang detection, & improve app/gui/qt/help folder structure GUI: Add UI language option Feb 13, 2021
@SunderB SunderB force-pushed the patch-localeOption branch 5 times, most recently from 5d7b4fa to 47b34e1 Compare February 13, 2021 23:48
@SunderB
Copy link
Contributor Author

SunderB commented Feb 13, 2021

@samaaron The main goal is to add the option to change the UI language within the GUI. I've gone through it, rebased it on top of the new API changes and trimmed it down to code that's only relevant to the GUI feature. Hopefully it makes it clearer to see its purpose and what it does? :)

@samaaron
Copy link
Collaborator

@cmaughan - would you be able to take a very brief look at this?

@cmaughan
Copy link
Contributor

Sure, I'll look at it later today

Copy link
Contributor

@cmaughan cmaughan left a comment

Choose a reason for hiding this comment

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

Minor comments, but looks good to me.
It might be nice to add a startup argument to revert to the system default.

.gitignore Outdated
@@ -61,7 +61,8 @@ app/gui/qt/moc_mainwindow.cpp
app/gui/qt/moc_sonicpiudpserver.cpp
app/gui/qt/qrc_SonicPi.cpp
app/gui/qt/utils/ruby_help.h
app/gui/qt/help/*.html
app/gui/qt/utils/lang_list.h
app/gui/qt/help
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we never update the help as a checkin?

app/gui/qt/mainwindow.cpp Outdated Show resolved Hide resolved
@samaaron
Copy link
Collaborator

@cmaughan, thanks!

@SunderB SunderB marked this pull request as ready for review February 14, 2021 13:32
@samaaron
Copy link
Collaborator

@SunderB - let me know when you think it's ready to be merged.

@SunderB
Copy link
Contributor Author

SunderB commented Feb 14, 2021

The only main issue is that some languages' translations fail to load (I believe because no strings are translated?):

  • bn - Bengali
  • ca@valencia - Valencian
  • en_AU - English (Australian)
  • eo - Esperanto

Apart from that if you can't find anything else that needs tweaking, it should be good to go I think :)

@SunderB
Copy link
Contributor Author

SunderB commented Feb 14, 2021

Also some of the native language names may need double checking and correcting - I've done my best but some may not be 100% correct.
zh and zh_Hans currently have the same name (简体中文 - Chinese (Simplified)) as I didn't know what specific language zh is here, and weblate redirects zh to zh_Hans.

SunderB and others added 12 commits April 15, 2021 22:03
Co-authored-by: ethancrawford <ethan_jc@hotmail.com>
Load the language after program output has been redirected to gui.log
… and add a warning about stopping runs & recordings (due to restart) when changing language.
…ts own tab in the SettingsWidget; fix bugs from renaming source files

Also, when the user selects 'System language', we now show them the list of
detected system languages, and show them which one is in use,
…n changing the language

Also clean up/refactor SonicPii18n class
@SunderB
Copy link
Contributor Author

SunderB commented Apr 15, 2021

I've updated it to allow the user to decide whether to restart Sonic Pi or not after selecting a new language. If the user doesn't restart, then the language will be applied when next running Sonic Pi.

@SunderB
Copy link
Contributor Author

SunderB commented Apr 15, 2021

@samaaron @ethancrawford Are there any things that need tweaking or fixing? Are you happy with the structure and how the implementation works?

@samaaron
Copy link
Collaborator

@SunderB This is looking really nice, thank-you so much for all your really hard work.

I guess my only major question is how people can easily reset to the system default. If a user accidentally changes to a different locale that they can't understand, what's the plan for us supporting them to easily reset things?

@samaaron
Copy link
Collaborator

Hi @SunderB - looks like my recent patch removing the old api code has cause some conflicts here - apologies!

Any chance you could fix this in the PR? Also, am I right in thinking that you store the current locale choice in the QSettings file? If so, I've recently moved it to the standard config dir, so just nuking that should be a simple way of reverting locale to system default?

* Fixes an issue where the settings wouldn't be saved when restarting
after selecting a new language.
* Also, remove the unnecessary delay in restartApp(). Cleanup should be
complete when onExitCleanup() returns.
@SunderB
Copy link
Contributor Author

SunderB commented Apr 27, 2021

Hi @SunderB - looks like my recent patch removing the old api code has cause some conflicts here - apologies!

Any chance you could fix this in the PR? Also, am I right in thinking that you store the current locale choice in the QSettings file? If so, I've recently moved it to the standard config dir, so just nuking that should be a simple way of reverting locale to system default?

@samaaron I've managed to fix the conflicts, and after fixing a problem with settings not saving when restarting, it still seems to work. :) And yes, deleting the gui-settings.ini file seems to reset the UI language to the system language on my system.

* Add Basque and generic English to the list of language names
* Add a message in the language options panel when the translations fail
to load
* Set fallback language code to en_GB instead of en (so it's clear which
English translation is being used)
* Tweak some log messages from SonicPii18n
@samaaron samaaron merged commit ef24bc7 into sonic-pi-net:dev May 24, 2021
@samaaron
Copy link
Collaborator

Thanks so much for this work - much appreciated.

@rbnpi
Copy link
Contributor

rbnpi commented May 24, 2021

Tested on my Mac and Raspberry Pi. Works nicely. Nice addition.

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.

Change GUI Language
5 participants