imap-send: Use parse options API to determine verbosity

PATCH In review Bernhard Reiter 2 participants 3 messages Jun 15, 2016 Documentation

lore

Share & tools

lore

Bernhard Reiter

author

2016-06-15 23:02 UTC #0 lore raw

PATCH v2 1/1 imap-send: Use parse options API to determine verbosity

Signed-off-by Bernhard Reiter <[email protected]>

---
In reply to <[email protected]>.

Thanks for bearing with me. I should've given the corresponding verbosity values more thought myself in the first place.

 Documentation/git-imap-send.txt | 14 +++++++++++++-
 imap-send.c                     | 25 +++++++++++++++----------
 2 files changed, 28 insertions(+), 11 deletions(-)

Documentation/git-imap-send.txt +13 -1

index c7c0d21..0897131 100644
@@ -9,7 +9,7 @@ git-imap-send - Send a collection of patches from stdin to an IMAP folder
 SYNOPSIS
 --------
 [verse]
-'git imap-send'
+'git imap-send' [-v] [-q]
 
 
 DESCRIPTION

@@ -26,6 +26,18 @@ Typical usage is something like:
 git format-patch --signoff --stdout --attach origin | git imap-send
 
 
+OPTIONS
+-------
+
+-v::
+--verbose::
+	Be verbose.
+
+-q::
+--quiet::
+	Be quiet.
+
+
 CONFIGURATION
 -------------

imap-send.c +15 ~~-10~~

index 70bcc7a..6efaae8 100644
@@ -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;
-
 __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 (0 < verbosity)
 					puts(*s);
 				return 0;
 			}

@@ -433,7 +439,7 @@ static void imap_info(const char *msg, ...)
 {
 	va_list va;
 
-	if (!Quiet) {
+	if (verbosity >= 0) {
 		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 (-2 < verbosity) {
 		va_start(va, msg);
 		vfprintf(stderr, msg, va);
 		va_end(va);

@@ -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 (0 < verbosity) {
 		if (imap->num_in_progress)
 			printf("(%d in progress) ", imap->num_in_progress);
 		if (!starts_with(cmd->cmd, "LOGIN"))

@@ -1352,12 +1358,11 @@ int main(int argc, char **argv)
 
 	git_setup_gettext();
 
-	if (argc != 1)
-		usage(imap_send_usage);
-
 	setup_git_directory_gently(&nongit_ok);
 	git_imap_config();
 
+	argc = parse_options(argc, (const char **)argv, "", imap_send_options, imap_send_usage, 0);
+
 	if (!server.port)
 		server.port = server.use_ssl ? 993 : 143;

Signature · 1 line

2.1.2.556.g44ebd84

Junio C Hamano

2016-06-15 23:02 UTC #1 lore raw

Quoted 6 lines · Bernhard Reiter

Signed-off-by Bernhard Reiter <[email protected]>
---
In reply to <[email protected]>.
Thanks for bearing with me. I should've given the corresponding verbosity values more thought myself in the first place.

The original defined and even used these two variables, but as far as I can tell they were always set to 0 and nobody modified them. Hence, we would be OK as long as the end result will behave the same as before without any -v/-q from the command line ;-)

Thanks. This version looks more sensible.

imap-send.c int main(int argc, char **argv) · line 1365

Quoted 8 lines

imap-send.c +2 -3

-	if (argc != 1)
-		usage(imap_send_usage);
-
 	setup_git_directory_gently(&nongit_ok);
 	git_imap_config();
 
+	argc = parse_options(argc, (const char **)argv, "", imap_send_options, imap_send_usage, 0);
+

... except we might want to check argc here and say something about missing or excess parameters, which was lost in the change in this hunk. I think (without giving it a real thought, though ;-) you would expect that nothing remains on the command line after parse_options() has done its thing, no?

Quoted 2 lines

imap-send.c +0 -0

 	if (!server.port)
 		server.port = server.use_ssl ? 993 : 143;

Junio C Hamano

2016-06-15 23:02 UTC #2 lore raw

Quoted 8 lines · Junio C Hamano

Thanks. This version looks more sensible.
Quoted 6 lines
(quoted diff) +0 -3
-	if (argc != 1)
-		usage(imap_send_usage);
-
 	setup_git_directory_gently(&nongit_ok);
 	git_imap_config();

Quoted 13 lines

Quoted 3 lines
+ argc = parse_options(argc, (const char **)argv, "", imap_send_options, imap_send_usage, 0);
+
... except we might want to check argc here and say something about missing or excess parameters, which was lost in the change in this hunk. I think (without giving it a real thought, though ;-) you would expect that nothing remains on the command line after parse_options() has done its thing, no?
Quoted 2 lines
(quoted diff) +0 -0
 	if (!server.port)
 		server.port = server.use_ssl ? 993 : 143;

So here is what I ended up queuing for now. I think the new check on argc is correct but I wouldn't be surprised if I had off-by-one error or something silly like that ;-).

-- >8 --
From: Bernhard Reiter <[email protected]>
Date: Wed, 5 Nov 2014 15:29:21 +0100
Subject: [PATCH] imap-send: use parse options API to determine verbosity

The -v/-q options were sort-of supported but without using the parse-options API, and were not documented.

Signed-off-by Bernhard Reiter <[email protected]>

Signed-off-by Junio C Hamano <[email protected]>

---
 Documentation/git-imap-send.txt | 14 +++++++++++++-
 imap-send.c                     | 28 ++++++++++++++++++----------
 2 files changed, 31 insertions(+), 11 deletions(-)

Documentation/git-imap-send.txt +13 -1

index c7c0d21..0897131 100644
@@ -9,7 +9,7 @@ git-imap-send - Send a collection of patches from stdin to an IMAP folder
 SYNOPSIS
 --------
 [verse]
-'git imap-send'
+'git imap-send' [-v] [-q]
 
 
 DESCRIPTION

@@ -26,6 +26,18 @@ Typical usage is something like:
 git format-patch --signoff --stdout --attach origin | git imap-send
 
 
+OPTIONS
+-------
+
+-v::
+--verbose::
+	Be verbose.
+
+-q::
+--quiet::
+	Be quiet.
+
+
 CONFIGURATION
 -------------

imap-send.c +18 ~~-10~~

index 70bcc7a..7f9d30e 100644
@@ -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;
-
 __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 (0 < verbosity)
 					puts(*s);
 				return 0;
 			}

@@ -433,7 +439,7 @@ static void imap_info(const char *msg, ...)
 {
 	va_list va;
 
-	if (!Quiet) {
+	if (0 <= verbosity) {
 		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 (-2 < verbosity) {
 		va_start(va, msg);
 		vfprintf(stderr, msg, va);
 		va_end(va);

@@ -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 (0 < verbosity) {
 		if (imap->num_in_progress)
 			printf("(%d in progress) ", imap->num_in_progress);
 		if (!starts_with(cmd->cmd, "LOGIN"))

@@ -1352,12 +1358,14 @@ int main(int argc, char **argv)
 
 	git_setup_gettext();
 
-	if (argc != 1)
-		usage(imap_send_usage);
-
 	setup_git_directory_gently(&nongit_ok);
 	git_imap_config();
 
+	argc = parse_options(argc, (const char **)argv, "", imap_send_options, imap_send_usage, 0);
+
+	if (argc)
+		usage_with_options(imap_send_usage, imap_send_options);
+
 	if (!server.port)
 		server.port = server.use_ssl ? 993 : 143;

Signature · 1 line

2.2.0-rc0-55-gac00a8d