Am 27.09.2024 um 08:11 hat Michael Tokarev geschrieben:
> Add missing long options and --help output.
>
> Signed-off-by: Michael Tokarev <m...@tls.msk.ru>
> ---
> qemu-img.c | 44 ++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 36 insertions(+), 8 deletions(-)
>
> diff --git a/qemu-img.c b/qemu-img.c
> index 9157a6b45d..7a111bce72 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -1048,24 +1048,50 @@ static int img_commit(const img_cmd_t *ccmd, int
> argc, char **argv)
> for(;;) {
> static const struct option long_options[] = {
> {"help", no_argument, 0, 'h'},
> + {"quiet", no_argument, 0, 'q'},
> {"object", required_argument, 0, OPTION_OBJECT},
> + {"format", required_argument, 0, 'f'},
> {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
> + {"cache", required_argument, 0, 't'},
> + {"drop", no_argument, 0, 'd'},
> + {"base", required_argument, 0, 'b'},
> + {"progress", no_argument, 0, 'p'},
> + {"rate", required_argument, 0, 'r'},
"rate-limit"?
> {0, 0, 0, 0}
> };
> - c = getopt_long(argc, argv, ":f:ht:b:dpqr:",
> + c = getopt_long(argc, argv, "f:ht:b:dpqr:",
> long_options, NULL);
Should we try to keep the order in long_options and in the getopt string
consistent? There doesn't seem to be any system behind the order we have
currently. Maybe keep common options (--help, --quiet) first, but then
order things alphabetically?
It doesn't really matter that much here, it would just improve
legibility of the code a bit. But I think in the help text, we should
definitely have a more obvious order so that users can find their option
without having to read everything.
Of course, this is a comment that applies not only to this patch, but to
all subcommands.
> if (c == -1) {
> break;
> }
> switch(c) {
> - case ':':
> - missing_argument(argv[optind - 1]);
> - break;
> - case '?':
> - unrecognized_option(argv[optind - 1]);
> - break;
> case 'h':
> - help();
> + cmd_help(ccmd,
> +"[-f FMT | --image-opts] [-t CACHE_MODE] [-b BASE_IMG] [-d]\n"
> +" [-r RATE] [--object OBJDEF] FILENAME\n"
> +,
> +" -q, --quiet\n"
> +" quiet operations\n"
Same as in the previous patch. After this one, I won't point out things
any more if I already commented on the same thing earlier in the series.
They should apply to all similar instances.
> +" -p, --progress\n"
> +" show operation progress\n"
The man page has "display progress bar" (even though it's not a bar).
Maybe make it "display progress information" in both places?
> +" -f, --format FMT\n"
> +" specify FILENAME image format explicitly\n"
> +" --image-opts\n"
> +" indicates that FILENAME is a complete image specification\n"
> +" instead of a file name (incompatible with --format)\n"
> +" -t, --cache CACHE_MODE image cache mode (" BDRV_DEFAULT_CACHE ")\n"
> +" -d, --drop\n"
> +" skip emptying FILENAME on completion\n"
> +" -b, --base BASE_IMG\n"
> +" image in the backing chain to which to commit changes\n"
> +" instead of the previous one (implies --drop)\n"
"image in the backing chain to commit change to (default: immediate
backing file; implies --drop)"?
> +" -r, --rate RATE\n"
> +" I/O rate limit\n"
in bytes per second
> +" --object OBJDEF\n"
> +" QEMU user-creatable object (eg encryption key)\n"
> +" FILENAME\n"
> +" name of the image file to operate on\n"
> +);
> break;
> case 'f':
> fmt = optarg;
> @@ -1099,6 +1125,8 @@ static int img_commit(const img_cmd_t *ccmd, int argc,
> char **argv)
> case OPTION_IMAGE_OPTS:
> image_opts = true;
> break;
> + default:
> + tryhelp(argv[0]);
> }
> }
Kevin
