Re: [Qemu-block] [PATCH 1/5] option: Make option help nicer to read
On 16.10.18 14:12, Kevin Wolf wrote: > Am 15.10.2018 um 19:28 hat Max Reitz geschrieben: >> This adds some whitespace into the option help (including indentation) >> and replaces '=' by ': ' (not least because '=' should be used for >> values, not types). Furthermore, the list name is no longer printed as >> part of every line, but only once in advance, and only if the caller did >> not print a caption already. >> >> Signed-off-by: Max Reitz > > If this isn't a bike shedding series, then what is? So... Sure it is. >> --- a/tests/qemu-iotests/082.out >> +++ b/tests/qemu-iotests/082.out >> @@ -44,171 +44,171 @@ cluster_size: 8192 >> >> Testing: create -f qcow2 -o help TEST_DIR/t.qcow2 128M >> Supported options: >> -size Virtual disk size >> -compat Compatibility level (0.10 or 1.1) >> -backing_file File name of a base image >> -backing_fmt Image format of the base image >> -encryption Encrypt the image with format 'aes'. (Deprecated in favor >> of encrypt.format=aes) >> -encrypt.format Encrypt the image, format choices: 'aes', 'luks' >> -encrypt.key-secret ID of secret providing qcow AES key or LUKS passphrase >> -encrypt.cipher-alg Name of encryption cipher algorithm >> -encrypt.cipher-mode Name of encryption cipher mode >> -encrypt.ivgen-alg Name of IV generator algorithm >> -encrypt.ivgen-hash-alg Name of IV generator hash algorithm >> -encrypt.hash-alg Name of encryption hash algorithm >> -encrypt.iter-time Time to spend in PBKDF in milliseconds >> -cluster_size qcow2 cluster size >> -preallocationPreallocation mode (allowed values: off, metadata, falloc, >> full) >> -lazy_refcounts Postpone refcount updates >> -refcount_bitsWidth of a reference count entry in bits >> -nocowTurn off copy-on-write (valid only on btrfs) >> + backing_file: str - File name of a base image >> + backing_fmt: str - Image format of the base image >> + cluster_size: size - qcow2 cluster size >> + compat: str - Compatibility level (0.10 or 1.1) >> + encrypt.cipher-alg: str - Name of encryption cipher algorithm >> + encrypt.cipher-mode: str - Name of encryption cipher mode >> + encrypt.format: str - Encrypt the image, format choices: 'aes', 'luks' >> + encrypt.hash-alg: str - Name of encryption hash algorithm >> + encrypt.iter-time: num - Time to spend in PBKDF in milliseconds >> + encrypt.ivgen-alg: str - Name of IV generator algorithm >> + encrypt.ivgen-hash-alg: str - Name of IV generator hash algorithm >> + encrypt.key-secret: str - ID of secret providing qcow AES key or LUKS >> passphrase >> + encryption: bool (on/off) - Encrypt the image with format 'aes'. >> (Deprecated in favor of encrypt.format=aes) >> + lazy_refcounts: bool (on/off) - Postpone refcount updates >> + nocow: bool (on/off) - Turn off copy-on-write (valid only on btrfs) >> + preallocation: str - Preallocation mode (allowed values: off, metadata, >> falloc, full) >> + refcount_bits: num - Width of a reference count entry in bits >> + size: size - Virtual disk size > > I like the result of this series better than what we have in current > master. Looking at this patch, however, I must also say that I like the > original state better than both. I don't disagree. But having the types is indeed a worthy improvement. > How about aligning the description to the same column again to combine > the best of both worlds? That sounds good to me. Max signature.asc Description: OpenPGP digital signature
Re: [Qemu-block] [PATCH 1/5] option: Make option help nicer to read
Am 15.10.2018 um 19:28 hat Max Reitz geschrieben: > This adds some whitespace into the option help (including indentation) > and replaces '=' by ': ' (not least because '=' should be used for > values, not types). Furthermore, the list name is no longer printed as > part of every line, but only once in advance, and only if the caller did > not print a caption already. > > Signed-off-by: Max Reitz If this isn't a bike shedding series, then what is? So... > --- a/tests/qemu-iotests/082.out > +++ b/tests/qemu-iotests/082.out > @@ -44,171 +44,171 @@ cluster_size: 8192 > > Testing: create -f qcow2 -o help TEST_DIR/t.qcow2 128M > Supported options: > -size Virtual disk size > -compat Compatibility level (0.10 or 1.1) > -backing_file File name of a base image > -backing_fmt Image format of the base image > -encryption Encrypt the image with format 'aes'. (Deprecated in favor > of encrypt.format=aes) > -encrypt.format Encrypt the image, format choices: 'aes', 'luks' > -encrypt.key-secret ID of secret providing qcow AES key or LUKS passphrase > -encrypt.cipher-alg Name of encryption cipher algorithm > -encrypt.cipher-mode Name of encryption cipher mode > -encrypt.ivgen-alg Name of IV generator algorithm > -encrypt.ivgen-hash-alg Name of IV generator hash algorithm > -encrypt.hash-alg Name of encryption hash algorithm > -encrypt.iter-time Time to spend in PBKDF in milliseconds > -cluster_size qcow2 cluster size > -preallocationPreallocation mode (allowed values: off, metadata, falloc, > full) > -lazy_refcounts Postpone refcount updates > -refcount_bitsWidth of a reference count entry in bits > -nocowTurn off copy-on-write (valid only on btrfs) > + backing_file: str - File name of a base image > + backing_fmt: str - Image format of the base image > + cluster_size: size - qcow2 cluster size > + compat: str - Compatibility level (0.10 or 1.1) > + encrypt.cipher-alg: str - Name of encryption cipher algorithm > + encrypt.cipher-mode: str - Name of encryption cipher mode > + encrypt.format: str - Encrypt the image, format choices: 'aes', 'luks' > + encrypt.hash-alg: str - Name of encryption hash algorithm > + encrypt.iter-time: num - Time to spend in PBKDF in milliseconds > + encrypt.ivgen-alg: str - Name of IV generator algorithm > + encrypt.ivgen-hash-alg: str - Name of IV generator hash algorithm > + encrypt.key-secret: str - ID of secret providing qcow AES key or LUKS > passphrase > + encryption: bool (on/off) - Encrypt the image with format 'aes'. > (Deprecated in favor of encrypt.format=aes) > + lazy_refcounts: bool (on/off) - Postpone refcount updates > + nocow: bool (on/off) - Turn off copy-on-write (valid only on btrfs) > + preallocation: str - Preallocation mode (allowed values: off, metadata, > falloc, full) > + refcount_bits: num - Width of a reference count entry in bits > + size: size - Virtual disk size I like the result of this series better than what we have in current master. Looking at this patch, however, I must also say that I like the original state better than both. How about aligning the description to the same column again to combine the best of both worlds? Kevin
[Qemu-block] [PATCH 1/5] option: Make option help nicer to read
This adds some whitespace into the option help (including indentation)
and replaces '=' by ': ' (not least because '=' should be used for
values, not types). Furthermore, the list name is no longer printed as
part of every line, but only once in advance, and only if the caller did
not print a caption already.
Signed-off-by: Max Reitz
---
include/qemu/option.h | 2 +-
qemu-img.c | 4 +-
util/qemu-option.c | 31 +-
tests/qemu-iotests/082.out | 956 ++---
4 files changed, 505 insertions(+), 488 deletions(-)
diff --git a/include/qemu/option.h b/include/qemu/option.h
index 3dfb4493cc..844587cab3 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -132,7 +132,7 @@ typedef int (*qemu_opts_loopfunc)(void *opaque, QemuOpts
*opts, Error **errp);
int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func,
void *opaque, Error **errp);
void qemu_opts_print(QemuOpts *opts, const char *sep);
-void qemu_opts_print_help(QemuOptsList *list);
+void qemu_opts_print_help(QemuOptsList *list, bool print_caption);
void qemu_opts_free(QemuOptsList *list);
QemuOptsList *qemu_opts_append(QemuOptsList *dst, QemuOptsList *list);
diff --git a/qemu-img.c b/qemu-img.c
index b12f4cd19b..4c96db7ba4 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -269,7 +269,7 @@ static int print_block_option_help(const char *filename,
const char *fmt)
}
printf("Supported options:\n");
-qemu_opts_print_help(create_opts);
+qemu_opts_print_help(create_opts, false);
qemu_opts_free(create_opts);
return 0;
}
@@ -3773,7 +3773,7 @@ static int print_amend_option_help(const char *format)
assert(drv->create_opts);
printf("Creation options for '%s':\n", format);
-qemu_opts_print_help(drv->create_opts);
+qemu_opts_print_help(drv->create_opts, false);
printf("\nNote that not all of these options may be amendable.\n");
return 0;
}
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 9a5f263294..ce79332912 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -224,7 +224,14 @@ static const char *opt_type_to_string(enum QemuOptType
type)
g_assert_not_reached();
}
-void qemu_opts_print_help(QemuOptsList *list)
+/**
+ * Print the list of options available in the given list. If
+ * @print_caption is true, a caption (including the list name, if it
+ * exists) is printed. The options itself will be indented, so
+ * @print_caption should only be set to false if the caller prints its
+ * own custom caption (so that the indentation makes sense).
+ */
+void qemu_opts_print_help(QemuOptsList *list, bool print_caption)
{
QemuOptDesc *desc;
int i;
@@ -234,10 +241,7 @@ void qemu_opts_print_help(QemuOptsList *list)
desc = list->desc;
while (desc && desc->name) {
GString *str = g_string_new(NULL);
-if (list->name) {
-g_string_append_printf(str, "%s.", list->name);
-}
-g_string_append_printf(str, "%s=%s", desc->name,
+g_string_append_printf(str, "%s: %s", desc->name,
opt_type_to_string(desc->type));
if (desc->help) {
g_string_append_printf(str, " - %s", desc->help);
@@ -247,8 +251,21 @@ void qemu_opts_print_help(QemuOptsList *list)
}
g_ptr_array_sort(array, (GCompareFunc)qemu_pstrcmp0);
+if (print_caption && array->len > 0) {
+if (list->name) {
+printf("%s options:\n", list->name);
+} else {
+printf("Options:\n");
+}
+} else if (array->len == 0) {
+if (list->name) {
+printf("There are no options for %s.\n", list->name);
+} else {
+printf("No options available.\n");
+}
+}
for (i = 0; i < array->len; i++) {
-printf("%s\n", (char *)array->pdata[i]);
+printf(" %s\n", (char *)array->pdata[i]);
}
g_ptr_array_set_free_func(array, g_free);
g_ptr_array_free(array, true);
@@ -930,7 +947,7 @@ QemuOpts *qemu_opts_parse_noisily(QemuOptsList *list, const
char *params,
opts = opts_parse(list, params, permit_abbrev, false, , );
if (err) {
if (invalidp && has_help_option(params)) {
-qemu_opts_print_help(list);
+qemu_opts_print_help(list, true);
error_free(err);
} else {
error_report_err(err);
diff --git a/tests/qemu-iotests/082.out b/tests/qemu-iotests/082.out
index 19e9fb13ff..115745f474 100644
--- a/tests/qemu-iotests/082.out
+++ b/tests/qemu-iotests/082.out
@@ -44,171 +44,171 @@ cluster_size: 8192
Testing: create -f qcow2 -o help TEST_DIR/t.qcow2 128M
Supported options:
-size Virtual disk size
-compat Compatibility level (0.10 or 1.1)
-backing_file File name of a base image
-backing_fmt Image format of the base image
-encryption Encrypt the image with format 'aes'. (Deprecated in favor of
