From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 674F462FAA for ; Mon, 24 Jan 2022 09:55:36 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 586511F5E3 for ; Mon, 24 Jan 2022 09:55:06 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 7DEFB1F5D4 for ; Mon, 24 Jan 2022 09:55:04 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 454E446139 for ; Mon, 24 Jan 2022 09:54:58 +0100 (CET) Message-ID: Date: Mon, 24 Jan 2022 09:54:56 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.5.0 Content-Language: en-US To: Thomas Lamprecht , Proxmox Backup Server development discussion References: <20220120093325.1120218-1-m.heiserer@proxmox.com> From: Matthias Heiserer In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.001 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -0.001 Looks like a legit reply (A) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [proxmox-backup-proxy.rs, node.rs] Subject: Re: [pbs-devel] [PATCH proxmox-backup] close #3103: default lang X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 24 Jan 2022 08:55:36 -0000 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 >> --- >> 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, > > > 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, >> - >> + >> #[serde(skip_serializing_if = "Option::is_none")] >> pub email_from: Option, >> >> /// 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, >> - >> + >> /// 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, >> + >> + /// Default language used in the GUI >> + #[serde(skip_serializing_if = "Option::is_none")] >> + pub default_lang: Option, >> } >> >> impl NodeConfig {