Re: [Qemu-block] [PATCH v2 1/5] option: Make option help nicer to read
On Fri, Oct 19, 2018 at 8:49 PM Max Reitz wrote:
>
> This adds some whitespace into the option help (including indentation)
> and puts angle brackets around the type names. 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.
>
> This patch also restores the description alignment we had before commit
> 9cbef9d68ee1d8d0, just at 24 instead of 16 characters like we used to.
> This increase is because now we have the type and two spaces of
> indentation before the description, and with a usual type name length of
> three chracters, this sums up to eight additional characters -- which
> means that we now need 24 characters to get the same amount of padding
> for most options. Also, 24 is a third of 80, which makes it kind of a
> round number in terminal terms.
>
> Finally, this patch amends the reference output of iotest 082 to match
> the changes (and thus makes it pass again).
>
> Signed-off-by: Max Reitz
Reviewed-by: Marc-André Lureau
> ---
> include/qemu/option.h | 2 +-
> qemu-img.c | 4 +-
> util/qemu-option.c | 32 +-
> tests/qemu-iotests/082.out | 956 ++---
> 4 files changed, 507 insertions(+), 487 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..de42e2a406 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,12 +241,12 @@ 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) {
> +if (str->len < 24) {
> +g_string_append_printf(str, "%*s", 24 - (int)str->len, "");
> +}
> g_string_append_printf(str, " - %s", desc->help);
> }
> g_ptr_array_add(array, g_string_free(str, false));
> @@ -247,6 +254,19 @@ 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[
[Qemu-block] [PATCH v2 1/5] option: Make option help nicer to read
This adds some whitespace into the option help (including indentation)
and puts angle brackets around the type names. 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.
This patch also restores the description alignment we had before commit
9cbef9d68ee1d8d0, just at 24 instead of 16 characters like we used to.
This increase is because now we have the type and two spaces of
indentation before the description, and with a usual type name length of
three chracters, this sums up to eight additional characters -- which
means that we now need 24 characters to get the same amount of padding
for most options. Also, 24 is a third of 80, which makes it kind of a
round number in terminal terms.
Finally, this patch amends the reference output of iotest 082 to match
the changes (and thus makes it pass again).
Signed-off-by: Max Reitz
---
include/qemu/option.h | 2 +-
qemu-img.c | 4 +-
util/qemu-option.c | 32 +-
tests/qemu-iotests/082.out | 956 ++---
4 files changed, 507 insertions(+), 487 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..de42e2a406 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,12 +241,12 @@ 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) {
+if (str->len < 24) {
+g_string_append_printf(str, "%*s", 24 - (int)str->len, "");
+}
g_string_append_printf(str, " - %s", desc->help);
}
g_ptr_array_add(array, g_string_free(str, false));
@@ -247,6 +254,19 @@ 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]);
}
@@ -930,7 +950,7 @@ QemuOpts *qemu_opts_parse_noisily(QemuOptsList *list, const
char *params,
opts = opts_parse(list, params, permit_abbrev, false, &invalidp, &err);
if (err) {
if (invalidp && has_help_option(params)) {
-qemu_opts_print_help(list);
+qemu
