public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
	Dominik Csapak <d.csapak@proxmox.com>
Subject: Re: [pve-devel] [PATCH manager] ui: lxc options: fix rendering default and '0' value for tty count
Date: Mon, 17 Nov 2025 11:41:53 +0100	[thread overview]
Message-ID: <eabf04eb-56af-490a-9c5a-3d741b7427f6@proxmox.com> (raw)
In-Reply-To: <20251117092110.628324-1-d.csapak@proxmox.com>

Am 17.11.25 um 10:21 schrieb Dominik Csapak:
> if '0' was entered, the options grid would show an empty line. Fix that
> by adding a renderer that checks if the value is a number.

I was a bit confused by this, as it sounded like setting it to '0' in the
frontend caused some non-numeric value in the backend which then was handled
wrong here.

But it's really just the default renderer in PendingObjectGrid's renderValue
using a truthy fallback to empty string for the value, i.e. `value || ''`

Nowadays we could replace the boolean or `||` with a nullish coalescing
operator `??`, which is limited to fallback only if the value is exactly
one of `null` or `undefined`, that should also fix these issues in a more
generic fashion.

> 
> If the default is set, the edit window would always pre-fill the value
> '2' instead of leaving the field empty. Fix that by removing the
> manually set 'value' property.
> 
> Reported-by: Dietmar Maurer <dietmar@proxmox.com>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  www/manager6/lxc/Options.js | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/www/manager6/lxc/Options.js b/www/manager6/lxc/Options.js
> index 6cc2e2b4..0acb88f4 100644
> --- a/www/manager6/lxc/Options.js
> +++ b/www/manager6/lxc/Options.js
> @@ -81,6 +81,7 @@ Ext.define('PVE.lxc.Options', {
>              tty: {
>                  header: gettext('TTY count'),
>                  defaultValue: 2,
> +                renderer: (value) => (Ext.isNumber(value) ? value : '2'),

But this would mask any other non-numeric value with "2" too, and then be
potentially misleading. Initially thought this was more theoretic, but
Ext.isNumeric basically comes down to `typeof value === "number"', so if
anything in the perl backend just "looks" at the returned value the wrong
way and it gets stringified along the way, we would mask the underlying
value here.

In Yew based UIs being fully typed is OK as there deserialization fails
explicitly and one has to chose the behavior for how one wants to handle
this, but in JS we (sadly) cannot really relay on this.


>                  editor: caps.vms['VM.Config.Options']
>                      ? {
>                            xtype: 'proxmoxWindowEdit',
> @@ -90,7 +91,6 @@ Ext.define('PVE.lxc.Options', {
>                                name: 'tty',
>                                minValue: 0,
>                                maxValue: 6,
> -                              value: 2,
>                                fieldLabel: gettext('TTY count'),
>                                emptyText: gettext('Default'),
>                                deleteEmpty: true,



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


  reply	other threads:[~2025-11-17 10:42 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-17  9:21 Dominik Csapak
2025-11-17 10:41 ` Thomas Lamprecht [this message]
2025-11-17 11:09   ` Dominik Csapak
2025-11-17 11:24 ` [pve-devel] superseded: " Dominik Csapak

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=eabf04eb-56af-490a-9c5a-3d741b7427f6@proxmox.com \
    --to=t.lamprecht@proxmox.com \
    --cc=d.csapak@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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