[PATCH 00/23] qemu-img: refersh options and --help handling

Michael Tokarev posted 23 patches 2 weeks, 1 day ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/cover.1707513011.git.mjt@tls.msk.ru
Maintainers: Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>
docs/tools/qemu-img.rst |   2 +-
qemu-img-cmds.hx        |   4 +-
qemu-img.c              | 843 ++++++++++++++++++++++++++--------------
3 files changed, 558 insertions(+), 291 deletions(-)
[PATCH 00/23] qemu-img: refersh options and --help handling
Posted by Michael Tokarev 2 weeks, 1 day ago
Quite big patchset implementing normal, readable qemu-img --help
(and qemu-img COMMAND --help) output with readable descriptions,
and adding many long options in the process.

In the end I stopped using qemu-img-opts.hx in qemu-img.c, perhaps
this can be avoided, with only list of commands and their desrciptions
kept there, but I don't see big advantage here.  The same list should
be included in docs/tools/qemu-img.rst, - this is not done now.

Also each command syntax isn't reflected in the doc for now, because
I want to give good names for options first, - and there, we've quite
some inconsistences and questions.  For example, measure --output=OFMT
-O OFMT, - this is priceless :)  I've no idea why we have this ugly
--output=json thing, why not have --json? ;)  I gave the desired
format long name --target-format to avoid clash with --output.

For rebase, src vs tgt probably should be renamed in local variables
too, and I'm not even sure I've got the caches right. For caches,
the thing is inconsistent across commands.

For compare, I used --a-format/--b-format (for -f/-F), - this can
be made --souce-format and --target-format, to compare source (file1)
with target (file2).

For bitmap, things are scary, I'm not sure what -b SRC_FILENAME
really means, - for now I gave it --source option, but this does
not make it more clear, suggestions welcome.

There are many other inconsistencies, I can't fix them all in one
go.. :)

Michael Tokarev (23):
  qemu-img: pass current cmd info into command handlers
  qemu-img: refresh options/--help for "create" subcommand
  qemu-img: factor out parse_output_format() and use it in the code
  qemu-img: refresh options/--help for "check" command
  qemu-img: simplify --repair error message
  qemu-img: refresh options/--help for "commit" command
  qemu-img: refresh options/--help for "compare" command
  qemu-img: refresh options/--help for "convert" command
  qemu-img: refresh options/--help for "info" command
  qemu-img: refresh options/--help for "map" command
  qemu-img: allow specifying -f fmt for snapshot subcommand
  qemu-img: make -l (list) the default for "snapshot" subcommand
  qemu-img: refresh options/--help for "snapshot" command
  qemu-img: refresh options/--help for "rebase" command
  qemu-img: resize: do not always eat last argument
  qemu-img: refresh options/--help for "resize" command
  qemu-img: refresh options/--help for "amend" command
  qemu-img: refresh options/--help for "bench" command
  qemu-img: refresh options/--help for "bitmap" command
  qemu-img: refresh options/--help for "dd" command
  qemu-img: refresh options/--help for "measure" command
  qemu-img: implement short --help, remove global help() function
  qemu-img: inline list of supported commands, remove qemu-img-cmds.h
    include

 docs/tools/qemu-img.rst |   2 +-
 qemu-img-cmds.hx        |   4 +-
 qemu-img.c              | 843 ++++++++++++++++++++++++++--------------
 3 files changed, 558 insertions(+), 291 deletions(-)

-- 
2.39.2
Re: [PATCH 00/23] qemu-img: refersh options and --help handling
Posted by Michael Tokarev 2 weeks, 1 day ago
10.02.2024 00:22, Michael Tokarev wrote:
> Quite big patchset implementing normal, readable qemu-img --help
> (and qemu-img COMMAND --help) output with readable descriptions,
> and adding many long options in the process.
...

I forgot to run checkpatch.pl - minor edits, the result is at
https://gitlab.com/mjt0k/qemu/-/commits/qemu-img-options

/mjt
Re: [PATCH 00/23] qemu-img: refersh options and --help handling
Posted by Daniel P. Berrangé 4 days, 4 hours ago
On Sat, Feb 10, 2024 at 12:22:21AM +0300, Michael Tokarev wrote:
> Quite big patchset implementing normal, readable qemu-img --help
> (and qemu-img COMMAND --help) output with readable descriptions,
> and adding many long options in the process.
> 
> In the end I stopped using qemu-img-opts.hx in qemu-img.c, perhaps
> this can be avoided, with only list of commands and their desrciptions
> kept there, but I don't see big advantage here.  The same list should
> be included in docs/tools/qemu-img.rst, - this is not done now.

I think it'd be nice for qemu-img.c to be the canonical source
of truth, so we have the getopt short arg, long args, and
help all in the same place & thus much less likely to get
out of sync.  Thus I like the approach you've taken here
to stop using the .hx file.

As a later work, it wouldn't be too terrible to have a python
script that parses qemu-img.c to look for 'cmd_help(...)' calls
and extra the help text, which could be used to feed into the
qemu-img.rxt man page generation, thus fully eliminating the
.hx file.

> 
> Also each command syntax isn't reflected in the doc for now, because
> I want to give good names for options first, - and there, we've quite
> some inconsistences and questions.  For example, measure --output=OFMT
> -O OFMT, - this is priceless :)  I've no idea why we have this ugly
> --output=json thing, why not have --json? ;)  I gave the desired
> format long name --target-format to avoid clash with --output.
> 
> For rebase, src vs tgt probably should be renamed in local variables
> too, and I'm not even sure I've got the caches right. For caches,
> the thing is inconsistent across commands.
> 
> For compare, I used --a-format/--b-format (for -f/-F), - this can
> be made --souce-format and --target-format, to compare source (file1)
> with target (file2).
> 
> For bitmap, things are scary, I'm not sure what -b SRC_FILENAME
> really means, - for now I gave it --source option, but this does
> not make it more clear, suggestions welcome.
> 
> There are many other inconsistencies, I can't fix them all in one
> go.. :)

This is already a massive improvement on the status quo. My
comments were mostly around whitespace/layout tweaks to the
help output.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|