[PATCH] cli: change argument parsing convention for subcommands

[PATCH] cli: change argument parsing convention for subcommands

[david]

From: David Bremner <bremner@debian.org>

previously we deleted the subcommand name from argv before passing to
the subcommand. In this version, the deletion is done in the actual
subcommands. Although this causes some duplication of code, it allows
us to be more flexible about how we parse command line arguments in
the subcommand, including possibly using off-the-shelf routines like
getopt_long that expect the name of the command in argv[0].
---

What to people think about this? It is a bit intrusive, but I think
everyone is sick of writing ad-hoc argument parsing code.

 notmuch-config.c  |    2 ++
 notmuch-count.c   |    2 ++
 notmuch-dump.c    |    2 ++
 notmuch-new.c     |    2 ++
 notmuch-reply.c   |    2 ++
 notmuch-restore.c |    2 ++
 notmuch-search.c  |    2 ++
 notmuch-show.c    |    2 ++
 notmuch-tag.c     |    2 ++
 notmuch.c         |    2 +-
 10 files changed, 19 insertions(+), 1 deletions(-)

diff --git a/notmuch-config.c b/notmuch-config.c
index 485fa72..1a7ed58 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -735,6 +735,8 @@ notmuch_config_command_set (void *ctx, char *item, int argc, char *argv[])
 int
 notmuch_config_command (void *ctx, int argc, char *argv[])
 {
+    argc--; argv++; /* skip subcommand argument */
+
     if (argc < 2) {
 	fprintf (stderr, "Error: notmuch config requires at least two arguments.\n");
 	return 1;
diff --git a/notmuch-count.c b/notmuch-count.c
index 39f08c6..0d700a9 100644
--- a/notmuch-count.c
+++ b/notmuch-count.c
@@ -35,6 +35,8 @@ notmuch_count_command (void *ctx, int argc, char *argv[])
     notmuch_sort_t sort = NOTMUCH_SORT_NEWEST_FIRST;
 #endif
 
+    argc--; argv++; /* skip subcommand argument */
+
     for (i = 0; i < argc && argv[i][0] == '-'; i++) {
 	if (strcmp (argv[i], "--") == 0) {
 	    i++;
diff --git a/notmuch-dump.c b/notmuch-dump.c
index 409c86a..126593d 100644
--- a/notmuch-dump.c
+++ b/notmuch-dump.c
@@ -41,6 +41,8 @@ notmuch_dump_command (unused (void *ctx), int argc, char *argv[])
     if (notmuch == NULL)
 	return 1;
 
+    argc--; argv++; /* skip subcommand argument */
+
     if (argc && strcmp (argv[0], "--") != 0) {
 	fprintf (stderr, "Warning: the output file argument of dump is deprecated.\n");
 	output = fopen (argv[0], "w");
diff --git a/notmuch-new.c b/notmuch-new.c
index 96a1e31..81a9350 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -815,6 +815,8 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
     add_files_state.verbose = 0;
     add_files_state.output_is_a_tty = isatty (fileno (stdout));
 
+    argc--; argv++; /* skip subcommand argument */
+
     for (i = 0; i < argc && argv[i][0] == '-'; i++) {
 	if (STRNCMP_LITERAL (argv[i], "--verbose") == 0) {
 	    add_files_state.verbose = 1;
diff --git a/notmuch-reply.c b/notmuch-reply.c
index 8f23cea..7ac879f 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -628,6 +628,8 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
     params.part = -1;
     params.cryptoctx = NULL;
 
+    argc--; argv++; /* skip subcommand argument */
+
     for (i = 0; i < argc && argv[i][0] == '-'; i++) {
 	if (strcmp (argv[i], "--") == 0) {
 	    i++;
diff --git a/notmuch-restore.c b/notmuch-restore.c
index e4a5355..f530bb4 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -44,6 +44,8 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[])
 
     synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config);
 
+    argc--; argv++; /* skip subcommand argument */
+
     if (argc) {
 	input = fopen (argv[0], "r");
 	if (input == NULL) {
diff --git a/notmuch-search.c b/notmuch-search.c
index faccaf7..6f04d9a 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -395,6 +395,8 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
     int i, ret;
     output_t output = OUTPUT_SUMMARY;
 
+    argc--; argv++; /* skip subcommand argument */
+
     for (i = 0; i < argc && argv[i][0] == '-'; i++) {
 	if (strcmp (argv[i], "--") == 0) {
 	    i++;
diff --git a/notmuch-show.c b/notmuch-show.c
index 912999e..603992a 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -936,6 +936,8 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[]))
     params.cryptoctx = NULL;
     params.decrypt = 0;
 
+    argc--; argv++; /* skip subcommand argument */
+
     for (i = 0; i < argc && argv[i][0] == '-'; i++) {
 	if (strcmp (argv[i], "--") == 0) {
 	    i++;
diff --git a/notmuch-tag.c b/notmuch-tag.c
index 6204ae3..dded39e 100644
--- a/notmuch-tag.c
+++ b/notmuch-tag.c
@@ -65,6 +65,8 @@ notmuch_tag_command (void *ctx, unused (int argc), unused (char *argv[]))
 	return 1;
     }
 
+    argc--; argv++; /* skip subcommand argument */
+
     for (i = 0; i < argc; i++) {
 	if (strcmp (argv[i], "--") == 0) {
 	    i++;
diff --git a/notmuch.c b/notmuch.c
index 640ad8d..9c77349 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -639,7 +639,7 @@ main (int argc, char *argv[])
 	command = &commands[i];
 
 	if (strcmp (argv[1], command->name) == 0)
-	    return (command->function) (local, argc - 2, &argv[2]);
+	    return (command->function) (local, argc - 1, &argv[1]);
     }
 
     fprintf (stderr, "Error: Unknown command '%s' (see \"notmuch help\")\n",
-- 
1.7.5.4

Re: [PATCH] cli: change argument parsing convention for subcommands

[Jameson Graef Rollins]

On Fri, 21 Oct 2011 09:19:17 -0300, david@tethera.net wrote:
> What to people think about this? It is a bit intrusive, but I think
> everyone is sick of writing ad-hoc argument parsing code.

The larger goal seems like a good one to me, and if this helps on that
front I certainly don't see any issues.

jamie.

Re: [PATCH] cli: change argument parsing convention for subcommands

[Thomas Schwinge]

[-- Attachment #1: Type: text/plain, Size: 918 bytes --]

Hi!

On Fri, 21 Oct 2011 09:19:17 -0300, david@tethera.net wrote:
> previously we deleted the subcommand name from argv before passing to
> the subcommand. In this version, the deletion is done in the actual
> subcommands. Although this causes some duplication of code, it allows
> us to be more flexible about how we parse command line arguments in
> the subcommand, including possibly using off-the-shelf routines like
> getopt_long that expect the name of the command in argv[0].

Ack.  Like when the C library startup passes control to the main
function, where argv[0] is the invoked executable.

It seems that notmuch.c:notmuch_help_command also needs to be adapted?

notmuch-setup.c:notmuch_setup_command does not need to be adapted (and
hasn't been) for it doesn't look at its argv.  (It should bail out if
there are any arguments passed, but that's for another patch.)


Grüße,
 Thomas

[-- Attachment #2: Type: application/pgp-signature, Size: 489 bytes --]

Re: [PATCH] cli: change argument parsing convention for subcommands

[David Bremner]

On Sat, 22 Oct 2011 19:08:50 +0200, Thomas Schwinge <thomas@schwinge.name> wrote:
> 
> It seems that notmuch.c:notmuch_help_command also needs to be adapted?
> 

Thanks Thomas.

I pushed a version which doesn't break the help command completely ;).

d