Re: imap-send: Use parse options API to determine verbosity
From: Junio C Hamano <hidden>
Date: 2016-06-15 23:02:51
Bernhard Reiter [off-list ref] writes:
Signed-off-by: Bernhard Reiter <redacted> --- As requested per [ref].
Heh, I didn't quite "request" it, but thanks for a patch anyway.
quoted hunk
diff --git a/imap-send.c b/imap-send.c index 70bcc7a..7f40960 100644 --- a/imap-send.c +++ b/imap-send.c@@ -26,11 +26,19 @@ #include "credential.h" #include "exec_cmd.h" #include "run-command.h" +#include "parse-options.h" #ifdef NO_OPENSSL typedef void *SSL; #endif -static const char imap_send_usage[] = "git imap-send < <mbox>"; +static int verbosity; + +static const char * const imap_send_usage[] = { "git imap-send [-v] [-q] < <mbox>", NULL }; + +static struct option imap_send_options[] = { + OPT__VERBOSITY(&verbosity), + OPT_END() +}; #undef DRV_OK #define DRV_OK 0@@ -38,8 +46,6 @@ static const char imap_send_usage[] = "git imap-send < <mbox>"; #define DRV_BOX_BAD -2 #define DRV_STORE_BAD -3 -static int Verbose, Quiet;
So it used to be Verbose and Quiet both defaulting to 0 (false). Now we have verbosity that defaults to 0, positive telling us to be verbose and negative telling us to be quiet.
quoted hunk
__attribute__((format (printf, 1, 2))) static void imap_info(const char *, ...); __attribute__((format (printf, 1, 2)))@@ -418,7 +424,7 @@ static int buffer_gets(struct imap_buffer *b, char **s) if (b->buf[b->offset + 1] == '\n') { b->buf[b->offset] = 0; /* terminate the string */ b->offset += 2; /* next line */ - if (Verbose) + if (verbosity >= 0) puts(*s);
... which means this should be "if (0 < verbosity)", i.e. we do not squeak without "-v", no?
quoted hunk
@@ -433,7 +439,7 @@ static void imap_info(const char *msg, ...) { va_list va; - if (!Quiet) { + if (verbosity >= 0) {
This one looks sensible; unless we are told to be "-q"uiet, we used to give this message, and we will continue to do so.
quoted hunk
va_start(va, msg); vprintf(msg, va); va_end(va);@@ -445,7 +451,7 @@ static void imap_warn(const char *msg, ...) { va_list va; - if (Quiet < 2) { + if (verbosity < 2) { va_start(va, msg); vfprintf(stderr, msg, va); va_end(va);
The cut-off value "2" looks strange, but more importantly this conversion looks bogus no matter what the cut-off value is. "As long as the verbosity is smaller than this value, emit this error message????" That sounds backwards that you do not give the message if you see sufficient number of "-v" options, doesn't it? I think if (-2 < verbosity) would be in line with what the original logic seems to have wanted, that is "If the caller did not ask us to be extra quiet (i.e. just the regular Quiet=1 does not count), show this message".
quoted hunk
@@ -522,7 +528,7 @@ static struct imap_cmd *issue_imap_cmd(struct imap_store *ctx, cmd->tag, cmd->cmd, cmd->cb.dlen, CAP(LITERALPLUS) ? "+" : ""); - if (Verbose) { + if (verbosity >= 0) {
Same comment as the one at hunk -418,7 above applies.