Re: imap-send: Use parse options API to determine verbosity

[Junio C Hamano <gitster@pobox.com>]

Bernhard Reiter <ockham@raz.or.at> writes:

> Signed-off-by: Bernhard Reiter <ockham@raz.or.at>
> ---
> As requested per <xmqqzjcewq6p.fsf@gitster.dls.corp.google.com>.

Heh, I didn't quite "request" it, but thanks for a patch anyway.

> 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.

>  __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?

> @@ -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.

>  		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".

> @@ -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.