public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pbs-devel] [PATCH proxmox-backup] close #3103: default lang
@ 2022-01-20  9:33 Matthias Heiserer
  2022-01-20 12:04 ` Thomas Lamprecht
  0 siblings, 1 reply; 4+ messages in thread
From: Matthias Heiserer @ 2022-01-20  9:33 UTC (permalink / raw)
  To: pbs-devel

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 
+                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
+const LANG_SCHEMA: Schema = StringSchema::new("Available languages")
+    .format(&ApiStringFormat::Enum(&[
+        EnumEntry::new("ar", "ar"),
+        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 {
-- 
2.30.2





^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [pbs-devel] [PATCH proxmox-backup] close #3103: default lang
  2022-01-20  9:33 [pbs-devel] [PATCH proxmox-backup] close #3103: default lang Matthias Heiserer
@ 2022-01-20 12:04 ` Thomas Lamprecht
  2022-01-24  8:54   ` Matthias Heiserer
  0 siblings, 1 reply; 4+ messages in thread
From: Thomas Lamprecht @ 2022-01-20 12:04 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion, Matthias Heiserer

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 {





^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [pbs-devel] [PATCH proxmox-backup] close #3103: default lang
  2022-01-20 12:04 ` Thomas Lamprecht
@ 2022-01-24  8:54   ` Matthias Heiserer
  2022-01-24  8:59     ` Thomas Lamprecht
  0 siblings, 1 reply; 4+ messages in thread
From: Matthias Heiserer @ 2022-01-24  8:54 UTC (permalink / raw)
  To: Thomas Lamprecht, Proxmox Backup Server development discussion

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 {




^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [pbs-devel] [PATCH proxmox-backup] close #3103: default lang
  2022-01-24  8:54   ` Matthias Heiserer
@ 2022-01-24  8:59     ` Thomas Lamprecht
  0 siblings, 0 replies; 4+ messages in thread
From: Thomas Lamprecht @ 2022-01-24  8:59 UTC (permalink / raw)
  To: Matthias Heiserer, Proxmox Backup Server development discussion

On 24.01.22 09:54, Matthias Heiserer wrote:
> Are the style guidelines documented somewhere, e.g. in a rustfmt.toml file or the wiki? If so, I couldn't find them.

for rust we currently use the plain rustfmt settings, but it's not yet enforced
on build or the like and quite some files are not yet converted.

For new code we'd like to have it already correctly styled though, so using
rustfmt directly or through your editor and disregarding the changes from
unrelated code in the module would be the simplest way to ensure that.

Please do not sent format cleanups for the whole file, those are just big
and icky to handle over the list, that's best done by a maintainer directly.

- Thomas






^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-01-24  8:59 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-20  9:33 [pbs-devel] [PATCH proxmox-backup] close #3103: default lang Matthias Heiserer
2022-01-20 12:04 ` Thomas Lamprecht
2022-01-24  8:54   ` Matthias Heiserer
2022-01-24  8:59     ` Thomas Lamprecht

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal