From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <m.carrara@proxmox.com> 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 9C29DB8B22 for <pve-devel@lists.proxmox.com>; Wed, 6 Dec 2023 10:59:34 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 76AA037176 for <pve-devel@lists.proxmox.com>; Wed, 6 Dec 2023 10:59:04 +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)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for <pve-devel@lists.proxmox.com>; Wed, 6 Dec 2023 10:59:03 +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 A333A42509 for <pve-devel@lists.proxmox.com>; Wed, 6 Dec 2023 10:59:03 +0100 (CET) Message-ID: <9dc84de5-23c5-4ecb-a25b-41488e5709b3@proxmox.com> Date: Wed, 6 Dec 2023 10:59:02 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: Dominik Csapak <d.csapak@proxmox.com>, Proxmox VE development discussion <pve-devel@lists.proxmox.com> References: <20231205154458.268660-1-m.carrara@proxmox.com> <20231205154458.268660-2-m.carrara@proxmox.com> <f40535fc-38db-495d-85bd-f701783519d1@proxmox.com> Content-Language: en-US From: Max Carrara <m.carrara@proxmox.com> In-Reply-To: <f40535fc-38db-495d-85bd-f701783519d1@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.061 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_SCC_BODY_TEXT_LINE -0.01 - Subject: Re: [pve-devel] [RFC proxmox-widget-toolkit 1/2] input panel: add `raw` parameter to function `getValues` X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com> List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>, <mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe> List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/> List-Post: <mailto:pve-devel@lists.proxmox.com> List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help> List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>, <mailto:pve-devel-request@lists.proxmox.com?subject=subscribe> X-List-Received-Date: Wed, 06 Dec 2023 09:59:34 -0000 On 12/6/23 10:13, Dominik Csapak wrote: > hi, some comment inline > > On 12/5/23 16:44, Max Carrara wrote: >> This parameter may be used to circumvent calls to `onGetValues`. >> >> Also adds a docstring for the function. >> >> Signed-off-by: Max Carrara <m.carrara@proxmox.com> >> --- >> src/panel/InputPanel.js | 15 ++++++++++++++- >> 1 file changed, 14 insertions(+), 1 deletion(-) >> >> diff --git a/src/panel/InputPanel.js b/src/panel/InputPanel.js >> index 34150ef..723be42 100644 >> --- a/src/panel/InputPanel.js >> +++ b/src/panel/InputPanel.js >> @@ -31,7 +31,16 @@ Ext.define('Proxmox.panel.InputPanel', { >> return values; >> }, >> - getValues: function(dirtyOnly) { >> + /** >> + * Returns the submit data from the panel's form fields. >> + * >> + * @param {boolean} dirtyOnly `true` to return only dirty fields >> + * (fields that have been changed from their original value). >> + * @param {boolean} raw `true` to prevent calling >> + * {@link Proxmox.panel.InputPanel#onGetValues onGetValues} and >> + * instead return the original submit data. >> + */ > > nice to see these things documented, not sure about the format, but i guess we could adapt such a standard format (maybe at one point we > could even generate nicer docs for this :) ) That's just JSDoc - I haven't really seen it used anywhere here (and to be honest, also didn't bother to look) so I figured I'd just use it here and see how people respond. ;) Ext JS uses it too, so. > > another thing is the interface, adding a parameter works and can be ok > but wouldn't it be nicer to invent a new 'getRawValues'? > > this would then not change the signature of the original function and > is more in line with what extjs does internally > (getValue/getRawValue/etc.) > > not a hard requirement for me though Absolutely not opposed to this - I think then the code is more exlicit anyway. E.g. `foo.getValues(false, true)` doesn't really say what it does at first glance. I'll definitely incorporate this! > > a slight tangent thought: > > after looking, it seems we never use(d?) the dirtyOnly > parameter... so it might be nice to remove that > > we couldn't even have used it anywhere because we always > set it to false when 'onGetValues' is a function > so we'd need to set it to something different, but > later we call it unconditionally as a function(?!?) > > but that is rather unrelated, just confused me > (it seems it was "always" this way, so maybe > there is some hidden svn history that i'm not seeing > where the code would make more sense) I was wondering about that too, but I didn't really want to turn over too many stones for what's supposed to be just a simple RFC. I wouldn't mind retiring that parameter for the actual patch series then, if you'd like. I'll also double-check all call sites to make sure it's not used anywhere - not that it would make a difference, but just in case the parameter is provided somewhere. > >> + getValues: function(dirtyOnly, raw) { >> let me = this; >> if (Ext.isFunction(me.onGetValues)) { >> @@ -46,6 +55,10 @@ Ext.define('Proxmox.panel.InputPanel', { >> } >> }); >> + if (raw) { >> + return values; >> + } >> + >> return me.onGetValues(values); >> }, >>