[pve-devel] [PATCH v6 manager 5/6] add cmd parameter & rework query string creation

[Dominik Csapak]

looks good, nits inline


On 2/27/19 3:01 PM, Tim Marx wrote:
> Signed-off-by: Tim Marx <t.marx at proxmox.com>
> ---
>   www/manager6/VNCConsole.js | 32 +++++++++++++++++++++++++-------
>   1 file changed, 25 insertions(+), 7 deletions(-)
> 
> diff --git a/www/manager6/VNCConsole.js b/www/manager6/VNCConsole.js
> index 1438dd0c..e38fd0c8 100644
> --- a/www/manager6/VNCConsole.js
> +++ b/www/manager6/VNCConsole.js
> @@ -6,7 +6,9 @@ Ext.define('PVE.noVncConsole', {
>   
>       vmid: undefined,
>   
> -    consoleType: undefined, // lxc or kvm
> +    cmd: undefined,
> +
> +    consoleType: undefined, // lxc, kvm, shell, cmd
>   
>       layout: 'fit',
>   
> @@ -14,6 +16,17 @@ Ext.define('PVE.noVncConsole', {
>   
>       border: false,
>   
> +    clean: function (obj) {
> +	var propName;
> +	for (propName in obj) {
> +	    if (obj.hasOwnProperty(propName)) {
> +		if (obj[propName] === null || obj[propName] === undefined) {
> +		    delete obj[propName];
> +		}
> +	    }
> +	}
> +    },
> +

would it make sense to put the 'clean' into Utils, so that we
could reuse that if we need it? looks generic enough

>       initComponent : function() {
>   	var me = this;
>   
> @@ -25,7 +38,7 @@ Ext.define('PVE.noVncConsole', {
>   	    throw "no console type specified";
>   	}
>   
> -	if (!me.vmid && me.consoleType !== 'shell') {
> +	if (!me.vmid && (me.consoleType !== 'shell' && me.consoleType !== 'cmd')) {

nit: the () around the consoletype checks are not necessary
a && b && c === a && (b && c)

>   	    throw "no VM ID specified";
>   	}
>   
> @@ -34,15 +47,20 @@ Ext.define('PVE.noVncConsole', {
>   	var box = Ext.create('Ext.ux.IFrame', { itemid : "vncconsole" });
>   
>   	var type = me.xtermjs ? 'xtermjs' : 'novnc';
> -
>   	Ext.apply(me, {
>   	    items: box,
>   	    listeners: {
>   		activate: function() {
> -		    var url = '/?console=' + me.consoleType + '&' + type + '=1&node=' + me.nodename + '&resize=scale';
> -		    if (me.vmid) {
> -			url += '&vmid='+ me.vmid;
> -		    }
> +		    var queryDict = {
> +			console: me.consoleType, // kvm, lxc, upgrade or shell
> +			vmid: me.vmid,
> +			node: me.nodename,
> +			cmd: me.cmd,
> +			resize: 'scale'
> +		    };
> +		    queryDict[type] = 1;
> +		    me.clean(queryDict);
> +		    var url = '/?' + Ext.Object.toQueryString(queryDict);
>   		    box.load(url);
>   		}
>   	    }
>