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

Legacy-free Locale Service, D-Bus API proposal #529

Merged
merged 9 commits into from
Apr 27, 2023

Conversation

mvidner
Copy link
Member

@mvidner mvidner commented Apr 11, 2023

Problem

D-Installer currently have a separate Language service, although it's rather
simplistic. It just allows to set the language of the installed system using
Yast::Language.Set. And it's quite memory demanding for such an unimpressive
task.

That service would be a nice candidate to be rewritten from scratch with no
dependencies on YaST or Ruby. It's small enough and could give us a good
overview on how much can we save.

Solution

Design document for the new service API

Testing

  • ?

Screenshots

  • ?

@coveralls
Copy link

coveralls commented Apr 11, 2023

Pull Request Test Coverage Report for Build 4787610171

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 73.786%

Totals Coverage Status
Change from base Build 4755725623: 0.0%
Covered Lines: 4759
Relevant Lines: 6218

💛 - Coveralls

doc/locale_api.md Outdated Show resolved Hide resolved

#### Design of Proposal Layer (uppper)

None needed?
Copy link
Member

Choose a reason for hiding this comment

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

well, if we allow change of RTC that we should get here proposal, not?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think you are confusing the caller and callee of the proposal?

Agama.locale.SetLocale takes a locale and proposes software, keyboard, timezone

Agama.locale.SetLocalRTC does nothing on top of systemd. Something else calls it depending on what disks are there. It is not needed.

Copy link
Member Author

Choose a reason for hiding this comment

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

local_rtc: Priority<bool> should solve this, right?


#### Design of Elementary Layer (lower)

Just use the systemd API, don't add any API of our own. We will use
Copy link
Member

Choose a reason for hiding this comment

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

just to make it clear, by using systemd API you mean ahving own service, but use org.freedesktop.timedate1 interface, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, I wanted to omit it completely and use systemd directly, but maybe it is worth it to add a forwarder because it uses a different bus (Agama vs system)

Agama.Locale1 service -> Agama/Locale1 object -> freedesktop.timedate1 interface, forwards to freedesktop/timedate1

Copy link
Member

Choose a reason for hiding this comment

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

Well, systemd cannot be used directly as we do not want to modify insts-sys ( or live system ) and only target ( aka installed ) one.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, good point 🤔

interface ...Agama.Locale1 {
methods:
# FIXME: the method part is unclear to me. anyone better at proposals?
Commit();
Copy link
Member

Choose a reason for hiding this comment

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

I think we also need proposal method to 1. do initial proposal for given product 2. allow to reset values.
Commit method is fine for me.

simply set what we consider a good default for a newly set locale, because:

1. it is just one setting (as opposed to whole partitioning layout)
2. the change will be visible in the UI, I assume
Copy link
Member

Choose a reason for hiding this comment

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

not so true for CLI

Copy link
Member Author

Choose a reason for hiding this comment

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

For CLI, I see two UI modes:
a. if you have a "dumb" CLI, it does not need to give feedback, it does exactly as it is told, except it may throw errors
b. for a "smart" CLI like our proposal logic, it needs to output the resulting state/changes, so that the user does not confuse it with a dumb one

You may know a [similar settings in libzypp][resstatus] where it has 4 levels.

*: maybe this is a crazy optimization? I am not too opposed to use strings for
this on the bus.
Copy link
Member

@jreidinger jreidinger Apr 12, 2023

Choose a reason for hiding this comment

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

  1. or maybe other way. Just use priority like it is on libzypp repos?
  2. Another thing to consider here is that beside user/machine it can be also controlled by product, so e.g. some locale is not available on ALP Micro ( now only en is there ). So it can remove invalid value with high priority.

Copy link
Member Author

Choose a reason for hiding this comment

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

re (2) higher priority than Human?? IMHO a bad idea

Copy link
Member Author

@mvidner mvidner Apr 13, 2023

Choose a reason for hiding this comment

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

re (1): priority is a confusing concept, but I have found nothing better ☹️

The basic problem is:

  1. in words, everyone agrees that when lower priority and higher priority meet, the higher priority wins
  2. if you express them in numbers, you have conflicting interpretations:
    a. bigger number means bigger=higher priority
    b. lower number means higher priority, because a lower number is higher on the page in an ordered list.

Anyway, both my numeric values (23 and 42) and libzypp use the "ordered list" semantic.
So, I don't understand what you mean, actually?

Copy link
Member Author

Choose a reason for hiding this comment

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

man zypper:

-p, --priority positive-integer

Set priority of the repository. Priority of 1 is the highest, the higher the number the lower the priority. Default priority is 99. Packages from repositories with higher priority will be preferred even in case there is a higher installable version available in the repository with a lower priority.

Copy link
Member

Choose a reason for hiding this comment

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

well, question is if you use words human and machine, what will win? and if we introduce more like machine proposal, product specific, etc. what then will win?

Copy link
Member Author

Choose a reason for hiding this comment

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

Human always wins. (Haven't you watched Terminator? Haven't you known Asimov's laws since you've been manufactured born??)

Product specific? Then specify how it should work and I will name it.

@mvidner mvidner changed the title Legacy-free Locale Service, API proposal Legacy-free Locale Service, D-Bus API proposal Apr 17, 2023

```
ListLocales(
in s display_locale # "en_US.UTF-8"
Copy link
Member

Choose a reason for hiding this comment

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

Do we need this param or should we use what @shundhammer mention on call, so always return label in form english - native e.g. Czech - Čeština

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, YaST does it the english+native way already


```
ListX11Keyboards(
in s display_locale # "en_US.UTF-8"
Copy link
Member

Choose a reason for hiding this comment

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

langtable does not provide localization for keyboard. It contain only id and english only description like

<keyboardId>se(rus_nodeadkeys)</keyboardId>
<description>Russian (Sweden, phonetic, eliminate dead keys)</description>

in s locale_id
out s tz_id
)
```
Copy link
Member

Choose a reason for hiding this comment

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

There will be probably one Commit method to write all of them down. I also miss here some method to get locale/keyboard/timezone as UI should display what user already selected.


```
ListLocales(
out a(sss) id_english_native
Copy link
Member

Choose a reason for hiding this comment

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

what langtable has is not locales specific, but language oriented. So here it makes more sense to have something like out a(sssas) id_english_native_variants where values will be like `(en,English,English,[en_US.UTF-8,en_GB.UTF-8,en_IN.UTF-8,...]). So in UI user can select language and then variant which is not translated. Similar data we can get for territories where is territory like Zambia and locales and languages speaked there.

Copy link
Member

Choose a reason for hiding this comment

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

and when I try to implement it I also found that for some less common languages there are no translations in langtable itself like Central Aymara. I expect it will solve when we also get data for filtering to only supported languages


SetX11Keyboard(in s kb_id)

X11KeyboardForLocale(
Copy link
Member

Choose a reason for hiding this comment

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

and same here. Keyboard is linked to language or territory, but not to locale. So we do not have such data.

SetTimezone(in s tz_id)

TimezoneForLocale(
in s locale_id
Copy link
Member

Choose a reason for hiding this comment

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

and same here. There are languages or territories for timezone suggestion, but not for locale.

considering langtable,
de-emphasizing systemd but not throwing it away just yet
  because systemd:locale != langtable:(territory+language)
  well yes but the `+` is a nontrivial operation
@mvidner mvidner changed the base branch from master to rust_locale April 27, 2023 08:52
@mvidner
Copy link
Member Author

mvidner commented Apr 27, 2023

@jreidinger
I have

  • changed the PR target to rust_locale
  • also rebased this on master so you will be merging master too

@mvidner mvidner merged commit 9ec96f5 into rust_locale Apr 27, 2023
14 checks passed
@mvidner mvidner deleted the redesign-locale-api branch April 27, 2023 08:57
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.

None yet

3 participants