Autoinstallation for localization (l10n) settings #914

mvidner · 2023-12-05T08:43:49Z

Problem

In #881 localization was improved and works in the web UI.
This PR adds CLI and autoinstallation

https://trello.com/c/AvrY5iBP

Solution

Mimic the structure of the other configuration scopes.

TODO:

decide where to use locale and/or localization in the code: mostly "localization" except the D-Bus proxy which is "locale"

Testing

Tested manually
More unit tests for KeymapId

Screenshots

agama config show now works for localization:

7e24bb397c23:/checkout # (cd rust/; cargo run --bin agama -- -f yaml config show)
[... Compiling ...]
     Running `target/debug/agama -f yaml config show`
user:
  fullName: ''
  [... etc ...]
localization:
  language: en_US
  keyboard: us
  timezone: Europe/Berlin

coveralls · 2023-12-05T14:32:30Z

coverage: 74.339% (+0.1%) from 74.237%
when pulling ddd1025 on auto-localization
into f3aadda on master.

Before: > The profile is not valid. Please, check the following errors: > * null is not of type "string" > * null is not of type "string" After: > The profile is not valid. Please, check the following errors: > * null is not of type "string". ValidationError { instance: Null, kind: Type { kind: Single(String) }, instance_path: JSONPointer([Property("root"), Property("sshPublicKey")]), schema_path: JSONPointer([Keyword("properties"), Property("root"), Keyword("properties"), Property("sshPublicKey"), Keyword("type")]) } > * null is not of type "string". ValidationError { instance: Null, kind: Type { kind: Single(String) }, instance_path: JSONPointer([Property("storage"), Property("encryptionPassword")]), schema_path: JSONPointer([Keyword("properties"), Property("storage"), Keyword("properties"), Property("encryptionPassword"), Keyword("type")]) } Which is hard to read but does point to the invalid profile bits: `"sshPublicKey": null`, `"encryptionPassword": null`. BTW that profile is just the output of `agama config show`.

actually localization::settings::LocalizationSettings

Same issue as in #712: zbus-xmlgen generates a proxy for setting a writable property but forgets to mark it as a property, generating a method call instead (I fixed zbus-xmlgen in August but they haven't released a crate since June)

include names of files that were not found

Before, "foo@#$%" was parsed as "foo" Also make the Regex more readable with (?x) verbose syntax

jreidinger · 2024-02-05T12:23:52Z

rust/agama-lib/src/localization/client.rs

+    pub async fn language(&self) -> Result<String, ServiceError> {
+        let locales = self.localization_proxy.locales().await?;
+        if locales.is_empty() {
+            // FIXME is this right?


well, maybe more correct way is to use Result<Option<String>, ServiceError> and return Ok(None) here.

jreidinger · 2024-02-05T12:28:14Z

rust/agama-lib/src/localization/client.rs

+        } else {
+            // without clone:
+            // error[E0507]: cannot move out of index of `Vec<std::string::String>`
+            // but why, it's a Vec<String> I should be able to move it?!


I think reason is that you want to return just part of Vector that will be destructed. I think that if you return whole locales, then it should work as you move whole ownership.

vec.into_iter().next() does what I want, yay

jreidinger · 2024-02-05T12:42:47Z

rust/agama-lib/src/localization/store.rs

+    pub async fn load(&self) -> Result<LocalizationSettings, ServiceError> {
+        // TODO: we should use a single D-Bus call with Properties.GetAll
+        // but LocaleProxy does not have it, only get_property for individual methods
+        // and properties_proxy is private


Just use Property proxy instead of LocaleProxy for given object.
https://docs.rs/zbus/latest/zbus/fdo/struct.PropertiesProxy.html

so basically you can construct proxy yourself...but it is usually not necessary as it is cached and zbus checking PropertiesChange signal for you, so in the end it is very efficient if property does not change too much.

rust/agama-lib/src/proxies.rs

jreidinger · 2024-02-05T12:58:52Z

rust/agama-lib/src/store.rs

        // TODO: use try_join here
        Ok(settings)
    }

    /// Stores the given installation settings in the D-Bus service
    pub async fn store(&self, settings: &InstallSettings) -> Result<(), ServiceError> {
+        // ordering: localization before product


why it needs to be done before product? It is target system product and support for given localization can change between products, not?

Prepare for releasing Agama 8. It includes the following pull requests: * #884 * #886 * #914 * #918 * #956 * #957 * #958 * #959 * #960 * #961 * #962 * #963 * #964 * #965 * #966 * #969 * #970 * #976 * #977 * #978 * #979 * #980 * #981 * #983 * #984 * #985 * #986 * #988 * #991 * #992 * #995 * #996 * #997 * #999 * #1003 * #1004 * #1006 * #1007 * #1008 * #1009 * #1010 * #1011 * #1012 * #1014 * #1015 * #1016 * #1017 * #1020 * #1022 * #1023 * #1024 * #1025 * #1027 * #1028 * #1029 * #1030 * #1031 * #1032 * #1033 * #1034 * #1035 * #1036 * #1038 * #1039 * #1041 * #1042 * #1043 * #1045 * #1046 * #1047 * #1048 * #1052 * #1054 * #1056 * #1057 * #1060 * #1061 * #1062 * #1063 * #1064 * #1066 * #1067 * #1068 * #1069 * #1071 * #1072 * #1073 * #1074 * #1075 * #1079 * #1080 * #1081 * #1082 * #1085 * #1086 * #1087 * #1088 * #1089 * #1090 * #1091 * #1092 * #1093 * #1094 * #1095 * #1096 * #1097 * #1098 * #1099 * #1100 * #1102 * #1103 * #1104 * #1105 * #1106 * #1109 * #1110 * #1111 * #1112 * #1114 * #1116 * #1117 * #1118 * #1119 * #1120 * #1121 * #1122 * #1123 * #1125 * #1126 * #1127 * #1128 * #1129 * #1130 * #1131 * #1132 * #1133 * #1134 * #1135 * #1136 * #1138 * #1139 * #1140 * #1141 * #1142 * #1143 * #1144 * #1145 * #1146 * #1147 * #1148 * #1149 * #1151 * #1152 * #1153 * #1154 * #1155 * #1156 * #1157 * #1158 * #1160 * #1161 * #1162 * #1163 * #1164 * #1165 * #1166 * #1167 * #1168 * #1169 * #1170 * #1171 * #1172 * #1173 * #1174 * #1175 * #1177 * #1178 * #1180 * #1181 * #1182 * #1183 * #1184 * #1185 * #1187 * #1188 * #1189 * #1190 * #1191 * #1192 * #1193 * #1194 * #1195 * #1196 * #1198 * #1199 * #1200 * #1201 * #1203 * #1204 * #1205 * #1206 * #1207 * #1208 * #1209 * #1210 * #1211 * #1212 * #1213 * #1214 * #1215 * #1216 * #1217 * #1219 * #1220 * #1221 * #1222 * #1223 * #1224 * #1225 * #1226 * #1227 * #1229

mvidner force-pushed the auto-localization branch from 7889153 to da3c5e2 Compare December 5, 2023 14:28

mvidner force-pushed the auto-localization branch from da3c5e2 to 6bfc692 Compare January 19, 2024 20:25

mvidner added 7 commits February 1, 2024 16:33

Add localization::LocalizationSettings

15a3d4c

actually localization::settings::LocalizationSettings

LocalizationStore using LocalizationClient

fb8945b

Fix LocaleProxy writable properties

037618b

Same issue as in #712: zbus-xmlgen generates a proxy for setting a writable property but forgets to mark it as a property, generating a method call instead (I fixed zbus-xmlgen in August but they haven't released a crate since June)

More better errors for agama profile validate

491ea9e

include names of files that were not found

Localization part of agama config load

be31da6

Add localization/timezone to schema

8bd270a

Parsing KeymapID: don't allow traling garbage

8c26e4c

Before, "foo@#$%" was parsed as "foo" Also make the Regex more readable with (?x) verbose syntax

mvidner force-pushed the auto-localization branch from 6bfc692 to 8c26e4c Compare February 1, 2024 15:45

mvidner marked this pull request as ready for review February 2, 2024 10:06

jreidinger reviewed Feb 5, 2024

View reviewed changes

rust/agama-lib/src/proxies.rs Outdated Show resolved Hide resolved

jreidinger reviewed Feb 5, 2024

View reviewed changes

mvidner added 3 commits February 7, 2024 12:50

Simplify LocalizationClient::language

b815325

Move LocaleProxy to its own file like the others.

c5ca5ab

Store localization after product

ddd1025

jreidinger approved these changes Feb 7, 2024

View reviewed changes

mvidner merged commit d8b9f08 into master Feb 7, 2024
2 checks passed

mvidner deleted the auto-localization branch February 7, 2024 13:29

imobachgs mentioned this pull request Feb 12, 2024

Update to release 8 #1037

Closed

imobachgs mentioned this pull request May 17, 2024

Update to release 8 #1232

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Autoinstallation for localization (l10n) settings #914

Autoinstallation for localization (l10n) settings #914

mvidner commented Dec 5, 2023 •

edited

Loading

coveralls commented Dec 5, 2023 •

edited

Loading

jreidinger Feb 5, 2024

jreidinger Feb 5, 2024

mvidner Feb 7, 2024

jreidinger Feb 5, 2024

jreidinger Feb 5, 2024

jreidinger Feb 5, 2024

Autoinstallation for localization (l10n) settings #914

Autoinstallation for localization (l10n) settings #914

Conversation

mvidner commented Dec 5, 2023 • edited Loading

Problem

Solution

Testing

Screenshots

coveralls commented Dec 5, 2023 • edited Loading

jreidinger Feb 5, 2024

Choose a reason for hiding this comment

jreidinger Feb 5, 2024

Choose a reason for hiding this comment

mvidner Feb 7, 2024

Choose a reason for hiding this comment

jreidinger Feb 5, 2024

Choose a reason for hiding this comment

jreidinger Feb 5, 2024

Choose a reason for hiding this comment

jreidinger Feb 5, 2024

Choose a reason for hiding this comment

mvidner commented Dec 5, 2023 •

edited

Loading

coveralls commented Dec 5, 2023 •

edited

Loading