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

UI locale dbus #725

Merged
merged 21 commits into from
Aug 30, 2023
Merged

UI locale dbus #725

merged 21 commits into from
Aug 30, 2023

Conversation

jreidinger
Copy link
Contributor

@jreidinger jreidinger commented Aug 30, 2023

Problem

There are some string that come from backend and it is displayed to user. But we do not set localization for those strings.

trello: https://trello.com/c/Jp7S5kwh/3398-5-agama-add-support-for-translating-the-messages-from-the-d-bus-interface

Solution

Implement in Locale method to enlist all possible backend localization ( so far only for ruby based ones ) and allow to set localization. In other services have helper that will set initial localization (1) and also catch signal when it is changed (2).
As part of this PR also drop no longer used Language Service.

(1)

class Agama::DBus::FooService
...
      def start
        locale_client = Clients::Locale.new
        @ui_locale = UILocale.new(locale_client)
        export
      end
end

(2) see on_ui_locale_change

Testing

  • Tested manually

Screenshots

Example localization in czech ( in czech is only data comming from dbus ):

localized_actions

Copy link
Contributor

@lslezak lslezak left a comment

Choose a reason for hiding this comment

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

Looks OK to me, but someone with Rust knowledge should check it as well... 😃

Copy link
Contributor

@imobachgs imobachgs left a comment

Choose a reason for hiding this comment

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

It looks good to me. Just a few suggestions.

const DIR : &str = "/usr/share/YaST2/locale/";
let entries = read_dir(DIR).context("Reading YaST2 locale")?;
for entry in entries {
let entry = entry.context("Failed to read entry in YaST2 locale dir")?;
Copy link
Contributor

Choose a reason for hiding this comment

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

So we are returning an error if we cannot read any of the dirs, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, any read failure result in error. I would not expect it unless there is some issue with fs.

Copy link
Contributor

Choose a reason for hiding this comment

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

The /usr/share/YaST2/locale/ directory is owned by the base yast2 package so the error should not happen normally.

rust/agama-dbus-server/src/locale.rs Outdated Show resolved Hide resolved
service/lib/agama/ui_locale.rb Outdated Show resolved Hide resolved
@jreidinger jreidinger force-pushed the ui_locale_dbus branch 2 times, most recently from b5175a7 to 68ac11a Compare August 30, 2023 12:17
@coveralls
Copy link

coveralls commented Aug 30, 2023

Coverage Status

coverage: 74.678% (+0.2%) from 74.458% when pulling ffb9dfc on ui_locale_dbus into 2e2e80a on master.

const DIR : &str = "/usr/share/YaST2/locale/";
let entries = read_dir(DIR).context("Reading YaST2 locale")?;
for entry in entries {
let entry = entry.context("Failed to read entry in YaST2 locale dir")?;
Copy link
Contributor

Choose a reason for hiding this comment

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

The /usr/share/YaST2/locale/ directory is owned by the base yast2 package so the error should not happen normally.

@jreidinger jreidinger merged commit 460444b into master Aug 30, 2023
10 of 11 checks passed
@jreidinger jreidinger deleted the ui_locale_dbus branch August 30, 2023 13:10
@imobachgs imobachgs mentioned this pull request Sep 26, 2023
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