Re: [pbs-devel] [PATCH proxmox-backup] close #3103: default lang

[Matthias Heiserer <m.heiserer@proxmox.com>]

Thank you for the review!
Are the style guidelines documented somewhere, e.g. in a rustfmt.toml 
file or the wiki? If so, I couldn't find them.

On 20.01.2022 13:04, Thomas Lamprecht wrote:
> Looks as it'd work out in general, which is nice, a few style and other comments still inline
> though.
>
> The commit subject is not really telling, maybe use something like:
>
> "fix #3103: node config: allow to configure default language for ui"
>
> (don't care much for close vs. fix, latter is just shorter and a bit more
> commonly used)
>
> On 20.01.22 10:33, Matthias Heiserer wrote:
>> A default language can now be set in node.cfg.
>> This language is only used if none is set in the cookies.
>>
>> Signed-off-by: Matthias Heiserer <m.heiserer@proxmox.com>
>> ---
>>   src/bin/proxmox-backup-proxy.rs |  8 ++++++
>>   src/config/node.rs              | 45 ++++++++++++++++++++++++++++++---
>>   2 files changed, 50 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs
>> index 523966cf..c197b512 100644
>> --- a/src/bin/proxmox-backup-proxy.rs
>> +++ b/src/bin/proxmox-backup-proxy.rs
>> @@ -168,6 +168,14 @@ async fn get_index_future(
>>               lang = language;
>>           }
>>       }
>> +    if lang.is_empty() {
>> +        if let Ok((config, _)) = proxmox_backup::config::node::config() {
>> +            if let Some(default_lang) = config.default_lang {
>> +                // don't need to check for existance for language file because node::config() only returns valid results
> trailing whitespace and the line is to long in any way, use 100 character columns
> as maximum in our rust code base.
>
>> +                lang = default_lang;
>> +            }
>> +        }
>> +    }
>>   
>>       let data = json!({
>>           "NodeName": nodename,
>> diff --git a/src/config/node.rs b/src/config/node.rs
>> index 40d7b220..d78dfec6 100644
>> --- a/src/config/node.rs
>> +++ b/src/config/node.rs
>> @@ -4,7 +4,7 @@ use openssl::ssl::{SslAcceptor, SslMethod};
>>   use anyhow::{bail, Error};
>>   use serde::{Deserialize, Serialize};
>>   
>> -use proxmox_schema::{api, ApiStringFormat, ApiType, Updater};
>> +use proxmox_schema::{api, ApiStringFormat, ApiType, EnumEntry, Schema, StringSchema, Updater};
>>   
>>   use proxmox_http::ProxyConfig;
>>   
>> @@ -57,6 +57,37 @@ pub struct AcmeConfig {
>>       account: AcmeAccountName,
>>   }
>>   
>> +/// All available languages in Proxmox. Taken from proxmox-i18n repository
> In the long run we probably want to (and/or):
> a) auto-generate this from a central source of truth
> b) add it as API type in the schema and so that the gui can get it from there,
>     avoiding the hardcoded list also in the widget-toolkit and allows adding a
>     new language without touching less repos.
>
> But both are more involved and it's fine for me to get this now in with a hardcoded list,
> maybe throw in some `// TODO: auto-generate` comment though as better reminder.
>
>> +const LANG_SCHEMA: Schema = StringSchema::new("Available languages")
>> +    .format(&ApiStringFormat::Enum(&[
>> +        EnumEntry::new("ar", "ar"),
> IMO the enum description should be the actual language, similar to how we display
> it in the GUI. With the api macro you should be able to actually use a normal enum
> which use the doc-comments for the description.
>
>
> #[api]
> #[derive(Serialize, Deserialize)]
> #[serde(rename_all="lowercase")]
> /// All available languages in Proxmox. Taken from proxmox-i18n repository
> pub enum UILanguages {
>      /// Arabic
>      Ar,
>      /// Catalan
>      Ca,
>      /// Danish
>      Da,
> }
>
> ...
>
>         "default-lang" : {
>             type: UILanguages,
>             optional: true,
>         }
>
> ...
>
>       /// Default language used in the GUI
>       #[serde(skip_serializing_if = "Option::is_none")]
>       pub default_lang: Option<UILanguages>,
>
>
> Note that I used UILanguage just as placeholder, there's hopefully a better name for
> that type ;)
>
>> +        EnumEntry::new("ca", "ca"),
>> +        EnumEntry::new("da", "da"),
>> +        EnumEntry::new("de", "de"),
>> +        EnumEntry::new("es", "es"),
>> +        EnumEntry::new("eu", "eu"),
>> +        EnumEntry::new("fa", "fa"),
>> +        EnumEntry::new("fr", "fr"),
>> +        EnumEntry::new("gl", "gl"),
>> +        EnumEntry::new("he", "he"),
>> +        EnumEntry::new("hu", "hu"),
>> +        EnumEntry::new("it", "it"),
>> +        EnumEntry::new("ja", "ja"),
>> +        EnumEntry::new("kr", "kr"),
>> +        EnumEntry::new("nb", "nb"),
>> +        EnumEntry::new("nl", "nl"),
>> +        EnumEntry::new("nn", "nn"),
>> +        EnumEntry::new("pl", "pl"),
>> +        EnumEntry::new("pt_BR", "pt_BR"),
>> +        EnumEntry::new("ru", "ru"),
>> +        EnumEntry::new("sl", "sl"),
>> +        EnumEntry::new("sv", "sv"),
>> +        EnumEntry::new("tr", "tr"),
>> +        EnumEntry::new("zh_CN", "zh_CN"),
>> +        EnumEntry::new("zh_TW", "zh_TW"),
>> +    ]))
>> +    .schema();
>> +
>>   #[api(
>>       properties: {
>>           acme: {
>> @@ -100,6 +131,10 @@ pub struct AcmeConfig {
>>               schema: OPENSSL_CIPHERS_TLS_1_2_SCHEMA,
>>               optional: true,
>>           },
>> +        "default-lang" : {
>> +            schema: LANG_SCHEMA,
>> +            optional: true,
>> +        }
>>       },
>>   )]
>>   #[derive(Deserialize, Serialize, Updater)]
>> @@ -127,17 +162,21 @@ pub struct NodeConfig {
>>   
>>       #[serde(skip_serializing_if = "Option::is_none")]
>>       pub http_proxy: Option<String>,
>> -
>> +
>>       #[serde(skip_serializing_if = "Option::is_none")]
>>       pub email_from: Option<String>,
>>   
>>       /// List of TLS ciphers for TLS 1.3 that will be used by the proxy. (Proxy has to be restarted for changes to take effect)
>>       #[serde(skip_serializing_if = "Option::is_none", rename="ciphers-tls-1.3")]
>>       pub ciphers_tls_1_3: Option<String>,
>> -
>> +
>>       /// List of TLS ciphers for TLS <= 1.2 that will be used by the proxy. (Proxy has to be restarted for changes to take effect)
>>       #[serde(skip_serializing_if = "Option::is_none", rename="ciphers-tls-1.2")]
>>       pub ciphers_tls_1_2: Option<String>,
>> +
>> +    /// Default language used in the GUI
>> +    #[serde(skip_serializing_if = "Option::is_none")]
>> +    pub default_lang: Option<String>,
>>   }
>>   
>>   impl NodeConfig {