[pve-devel] [PATCH manager 3/3] Add a proper confirmation message when we detach an used disk

[Dominik Csapak]

Comment inline
On 11/13/2017 12:26 PM, Emmanuel Kasper wrote:
> Signed-off-by: Emmanuel Kasper <e.kasper at proxmox.com>
> ---
>   www/manager6/qemu/HardwareView.js | 13 ++++++++++---
>   1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/www/manager6/qemu/HardwareView.js b/www/manager6/qemu/HardwareView.js
> index 3b0f00f6..605958e1 100644
> --- a/www/manager6/qemu/HardwareView.js
> +++ b/www/manager6/qemu/HardwareView.js
> @@ -402,9 +402,16 @@ Ext.define('PVE.qemu.HardwareView', {
>   	    disabled: true,
>   	    dangerous: true,
>   	    confirmMsg: function(rec) {
> -		var msg = Ext.String.format(gettext('Are you sure you want to remove entry {0}'),
> -					    "'" + me.renderKey(rec.data.key, {}, rec) + "'");
> -		if (rec.data.key.match(/^unused\d+$/)) {
> +		var warn = 'Are you sure you want to remove entry {0}';
> +		if (this.text === this.altText) {
> +		    warn = 'Are you sure you want to detach entry {0}';
> +		}
> +
> +		var entry = rec.data.key;
> +		var msg = Ext.String.format(gettext(warn), "'"
> +		    + me.renderKey(entry, {}, rec) + "'");
> +
> +		if (entry.match(/^unused\d+$/)) {
>   		    msg += " " + gettext('This will permanently erase all data.');
>   		}
>   
> 

i think it would be better to have one big if/else instead of
building it across two smaller ones, for legibility

e.g.,

if (entry.match/^unused\d+$/)) {
// build text for deleting
} else {
// build text for detaching
}