[PATCH 1/2] cli: convert "notmuch show" to use the new argument parser
Quoth Jani Nikula on Feb 06 at 7:54 am: > On Sun, 5 Feb 2012 23:12:05 -0500, Austin Clements > wrote: > > Yikes. I don't envy you this patch. Two minor nits, otherwise this > > and the next patch LGTM. > > Thanks for the review. I'll roll another version, I think it will be > better with your suggestions incorporated. > > > Quoth Jani Nikula on Feb 04 at 12:41 am: > > > Use the new notmuch argument parser to handle arguments in "notmuch > > > show". There are two corner case functional changes: > > > > > > 1) Also set params.raw = 1 when defaulting to raw format when part is > > >requested but format is not specified. > > > > Huh. So "notmuch show --part=0 " was broken before. > > Hmm, yes, it seems so. Do you think I should make this a separate fix? I wouldn't bother. Since this usage would throw --format=raw into "useless mode", clearly no one cared. No sense throwing good code after bad.
[PATCH 1/2] cli: convert "notmuch show" to use the new argument parser
On Sun, 5 Feb 2012 23:12:05 -0500, Austin Clements wrote: > Yikes. I don't envy you this patch. Two minor nits, otherwise this > and the next patch LGTM. Thanks for the review. I'll roll another version, I think it will be better with your suggestions incorporated. > Quoth Jani Nikula on Feb 04 at 12:41 am: > > Use the new notmuch argument parser to handle arguments in "notmuch > > show". There are two corner case functional changes: > > > > 1) Also set params.raw = 1 when defaulting to raw format when part is > >requested but format is not specified. > > Huh. So "notmuch show --part=0 " was broken before. Hmm, yes, it seems so. Do you think I should make this a separate fix? BTW an alternative would be to require setting --format if --part is specified, and not adapt the default format depending on --part. > > 2) Do not set params.decrypt if crypto context creation fails. > > Technically this also behaves differently if given multiple --format > arguments, but I'll let that slide. Ugh, right, the old parsing was broken also that way. Luckily that falls in the corner case department too. > > > > Signed-off-by: Jani Nikula > > --- > > notmuch-show.c | 153 > > +--- > > 1 files changed, 79 insertions(+), 74 deletions(-) > > > > diff --git a/notmuch-show.c b/notmuch-show.c > > index dec799c..f93e121 100644 > > --- a/notmuch-show.c > > +++ b/notmuch-show.c > > @@ -1049,6 +1049,14 @@ do_show (void *ctx, > > return 0; > > } > > > > +enum { > > +NOTMUCH_FORMAT_NOT_SPECIFIED, > > +NOTMUCH_FORMAT_JSON, > > +NOTMUCH_FORMAT_TEXT, > > +NOTMUCH_FORMAT_MBOX, > > +NOTMUCH_FORMAT_RAW > > +}; > > + > > int > > notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[])) > > { > > @@ -1056,92 +1064,98 @@ notmuch_show_command (void *ctx, unused (int argc), > > unused (char *argv[])) > > notmuch_database_t *notmuch; > > notmuch_query_t *query; > > char *query_string; > > -char *opt; > > +int opt_index; > > const notmuch_show_format_t *format = _text; > > -notmuch_show_params_t params; > > -int mbox = 0; > > -int format_specified = 0; > > -int i; > > +notmuch_show_params_t params = { .part = -1 }; > > +int format_sel = NOTMUCH_FORMAT_NOT_SPECIFIED; > > +notmuch_bool_t decrypt = FALSE; > > +notmuch_bool_t verify = FALSE; > > +notmuch_bool_t entire_thread = FALSE; > > + > > +notmuch_opt_desc_t options[] = { > > + { NOTMUCH_OPT_KEYWORD, _sel, "format", 'f', > > + (notmuch_keyword_t []){ { "json", NOTMUCH_FORMAT_JSON }, > > + { "text", NOTMUCH_FORMAT_TEXT }, > > + { "mbox", NOTMUCH_FORMAT_MBOX }, > > + { "raw", NOTMUCH_FORMAT_RAW }, > > + { 0, 0 } } }, > > + { NOTMUCH_OPT_INT, , "part", 'p', 0 }, > > + { NOTMUCH_OPT_BOOLEAN, _thread, "entire-thread", 't', 0 }, > > + { NOTMUCH_OPT_BOOLEAN, , "decrypt", 'd', 0 }, > > + { NOTMUCH_OPT_BOOLEAN, , "verify", 'v', 0 }, > > + { 0, 0, 0, 0, 0 } > > +}; > > + > > +opt_index = parse_arguments (argc, argv, options, 1); > > +if (opt_index < 0) { > > + /* diagnostics already printed */ > > + return 1; > > +} > > > > -params.entire_thread = 0; > > -params.raw = 0; > > -params.part = -1; > > -params.cryptoctx = NULL; > > -params.decrypt = 0; > > +params.entire_thread = entire_thread; > > If you make params.entire_thread a notmuch_bool_t (instead of an int), > you could pass _thread in the notmuch_opt_desc_t and get > rid of the local variable. You're right; I was a bit lazy and didn't consider changing notmuch_show_params_t. I'll change both this and params.decrypt to notmuch_bool_t. > > > > > -argc--; argv++; /* skip subcommand argument */ > > +if (format_sel == NOTMUCH_FORMAT_NOT_SPECIFIED) { > > + /* if part was requested and format was not specified, use format=raw */ > > + if (params.part >= 0) > > + format_sel = NOTMUCH_FORMAT_RAW; > > + else > > + format_sel = NOTMUCH_FORMAT_TEXT; > > +} > > > > -for (i = 0; i < argc && argv[i][0] == '-'; i++) { > > - if (strcmp (argv[i], "--") == 0) { > > - i++; > > - break; > > +switch (format_sel) { > > +case NOTMUCH_FORMAT_JSON: > > + format = _json; > > + params.entire_thread = 1; > > + break; > > +case NOTMUCH_FORMAT_TEXT: > > + format = _text; > > + break; > > +case NOTMUCH_FORMAT_MBOX: > > + if (params.part > 0) { > > + fprintf (stderr, "Error: specifying parts is incompatible with mbox > > output format.\n"); > > + return 1; > > } > > - if (STRNCMP_LITERAL (argv[i], "--format=") == 0) { > > - opt = argv[i] + sizeof ("--format=") - 1; > > - if (strcmp (opt, "text") == 0) { > > - format = _text; > > - } else if (strcmp (opt, "json") == 0) { > > - format
Re: [PATCH 1/2] cli: convert notmuch show to use the new argument parser
Quoth Jani Nikula on Feb 06 at 7:54 am: On Sun, 5 Feb 2012 23:12:05 -0500, Austin Clements amdra...@mit.edu wrote: Yikes. I don't envy you this patch. Two minor nits, otherwise this and the next patch LGTM. Thanks for the review. I'll roll another version, I think it will be better with your suggestions incorporated. Quoth Jani Nikula on Feb 04 at 12:41 am: Use the new notmuch argument parser to handle arguments in notmuch show. There are two corner case functional changes: 1) Also set params.raw = 1 when defaulting to raw format when part is requested but format is not specified. Huh. So notmuch show --part=0 query was broken before. Hmm, yes, it seems so. Do you think I should make this a separate fix? I wouldn't bother. Since this usage would throw --format=raw into useless mode, clearly no one cared. No sense throwing good code after bad. ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 1/2] cli: convert "notmuch show" to use the new argument parser
Yikes. I don't envy you this patch. Two minor nits, otherwise this and the next patch LGTM. Quoth Jani Nikula on Feb 04 at 12:41 am: > Use the new notmuch argument parser to handle arguments in "notmuch > show". There are two corner case functional changes: > > 1) Also set params.raw = 1 when defaulting to raw format when part is >requested but format is not specified. Huh. So "notmuch show --part=0 " was broken before. > 2) Do not set params.decrypt if crypto context creation fails. Technically this also behaves differently if given multiple --format arguments, but I'll let that slide. > > Signed-off-by: Jani Nikula > --- > notmuch-show.c | 153 > +--- > 1 files changed, 79 insertions(+), 74 deletions(-) > > diff --git a/notmuch-show.c b/notmuch-show.c > index dec799c..f93e121 100644 > --- a/notmuch-show.c > +++ b/notmuch-show.c > @@ -1049,6 +1049,14 @@ do_show (void *ctx, > return 0; > } > > +enum { > +NOTMUCH_FORMAT_NOT_SPECIFIED, > +NOTMUCH_FORMAT_JSON, > +NOTMUCH_FORMAT_TEXT, > +NOTMUCH_FORMAT_MBOX, > +NOTMUCH_FORMAT_RAW > +}; > + > int > notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[])) > { > @@ -1056,92 +1064,98 @@ notmuch_show_command (void *ctx, unused (int argc), > unused (char *argv[])) > notmuch_database_t *notmuch; > notmuch_query_t *query; > char *query_string; > -char *opt; > +int opt_index; > const notmuch_show_format_t *format = _text; > -notmuch_show_params_t params; > -int mbox = 0; > -int format_specified = 0; > -int i; > +notmuch_show_params_t params = { .part = -1 }; > +int format_sel = NOTMUCH_FORMAT_NOT_SPECIFIED; > +notmuch_bool_t decrypt = FALSE; > +notmuch_bool_t verify = FALSE; > +notmuch_bool_t entire_thread = FALSE; > + > +notmuch_opt_desc_t options[] = { > + { NOTMUCH_OPT_KEYWORD, _sel, "format", 'f', > + (notmuch_keyword_t []){ { "json", NOTMUCH_FORMAT_JSON }, > + { "text", NOTMUCH_FORMAT_TEXT }, > + { "mbox", NOTMUCH_FORMAT_MBOX }, > + { "raw", NOTMUCH_FORMAT_RAW }, > + { 0, 0 } } }, > + { NOTMUCH_OPT_INT, , "part", 'p', 0 }, > + { NOTMUCH_OPT_BOOLEAN, _thread, "entire-thread", 't', 0 }, > + { NOTMUCH_OPT_BOOLEAN, , "decrypt", 'd', 0 }, > + { NOTMUCH_OPT_BOOLEAN, , "verify", 'v', 0 }, > + { 0, 0, 0, 0, 0 } > +}; > + > +opt_index = parse_arguments (argc, argv, options, 1); > +if (opt_index < 0) { > + /* diagnostics already printed */ > + return 1; > +} > > -params.entire_thread = 0; > -params.raw = 0; > -params.part = -1; > -params.cryptoctx = NULL; > -params.decrypt = 0; > +params.entire_thread = entire_thread; If you make params.entire_thread a notmuch_bool_t (instead of an int), you could pass _thread in the notmuch_opt_desc_t and get rid of the local variable. > > -argc--; argv++; /* skip subcommand argument */ > +if (format_sel == NOTMUCH_FORMAT_NOT_SPECIFIED) { > + /* if part was requested and format was not specified, use format=raw */ > + if (params.part >= 0) > + format_sel = NOTMUCH_FORMAT_RAW; > + else > + format_sel = NOTMUCH_FORMAT_TEXT; > +} > > -for (i = 0; i < argc && argv[i][0] == '-'; i++) { > - if (strcmp (argv[i], "--") == 0) { > - i++; > - break; > +switch (format_sel) { > +case NOTMUCH_FORMAT_JSON: > + format = _json; > + params.entire_thread = 1; > + break; > +case NOTMUCH_FORMAT_TEXT: > + format = _text; > + break; > +case NOTMUCH_FORMAT_MBOX: > + if (params.part > 0) { > + fprintf (stderr, "Error: specifying parts is incompatible with mbox > output format.\n"); > + return 1; > } > - if (STRNCMP_LITERAL (argv[i], "--format=") == 0) { > - opt = argv[i] + sizeof ("--format=") - 1; > - if (strcmp (opt, "text") == 0) { > - format = _text; > - } else if (strcmp (opt, "json") == 0) { > - format = _json; > - params.entire_thread = 1; > - } else if (strcmp (opt, "mbox") == 0) { > - format = _mbox; > - mbox = 1; > - } else if (strcmp (opt, "raw") == 0) { > - format = _raw; > - params.raw = 1; > - } else { > - fprintf (stderr, "Invalid value for --format: %s\n", opt); > - return 1; > - } > - format_specified = 1; > - } else if (STRNCMP_LITERAL (argv[i], "--part=") == 0) { > - params.part = atoi(argv[i] + sizeof ("--part=") - 1); > - } else if (STRNCMP_LITERAL (argv[i], "--entire-thread") == 0) { > - params.entire_thread = 1; > - } else if ((STRNCMP_LITERAL (argv[i], "--verify") == 0) || > -(STRNCMP_LITERAL (argv[i],
Re: [PATCH 1/2] cli: convert notmuch show to use the new argument parser
Yikes. I don't envy you this patch. Two minor nits, otherwise this and the next patch LGTM. Quoth Jani Nikula on Feb 04 at 12:41 am: Use the new notmuch argument parser to handle arguments in notmuch show. There are two corner case functional changes: 1) Also set params.raw = 1 when defaulting to raw format when part is requested but format is not specified. Huh. So notmuch show --part=0 query was broken before. 2) Do not set params.decrypt if crypto context creation fails. Technically this also behaves differently if given multiple --format arguments, but I'll let that slide. Signed-off-by: Jani Nikula j...@nikula.org --- notmuch-show.c | 153 +--- 1 files changed, 79 insertions(+), 74 deletions(-) diff --git a/notmuch-show.c b/notmuch-show.c index dec799c..f93e121 100644 --- a/notmuch-show.c +++ b/notmuch-show.c @@ -1049,6 +1049,14 @@ do_show (void *ctx, return 0; } +enum { +NOTMUCH_FORMAT_NOT_SPECIFIED, +NOTMUCH_FORMAT_JSON, +NOTMUCH_FORMAT_TEXT, +NOTMUCH_FORMAT_MBOX, +NOTMUCH_FORMAT_RAW +}; + int notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[])) { @@ -1056,92 +1064,98 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[])) notmuch_database_t *notmuch; notmuch_query_t *query; char *query_string; -char *opt; +int opt_index; const notmuch_show_format_t *format = format_text; -notmuch_show_params_t params; -int mbox = 0; -int format_specified = 0; -int i; +notmuch_show_params_t params = { .part = -1 }; +int format_sel = NOTMUCH_FORMAT_NOT_SPECIFIED; +notmuch_bool_t decrypt = FALSE; +notmuch_bool_t verify = FALSE; +notmuch_bool_t entire_thread = FALSE; + +notmuch_opt_desc_t options[] = { + { NOTMUCH_OPT_KEYWORD, format_sel, format, 'f', + (notmuch_keyword_t []){ { json, NOTMUCH_FORMAT_JSON }, + { text, NOTMUCH_FORMAT_TEXT }, + { mbox, NOTMUCH_FORMAT_MBOX }, + { raw, NOTMUCH_FORMAT_RAW }, + { 0, 0 } } }, + { NOTMUCH_OPT_INT, params.part, part, 'p', 0 }, + { NOTMUCH_OPT_BOOLEAN, entire_thread, entire-thread, 't', 0 }, + { NOTMUCH_OPT_BOOLEAN, decrypt, decrypt, 'd', 0 }, + { NOTMUCH_OPT_BOOLEAN, verify, verify, 'v', 0 }, + { 0, 0, 0, 0, 0 } +}; + +opt_index = parse_arguments (argc, argv, options, 1); +if (opt_index 0) { + /* diagnostics already printed */ + return 1; +} -params.entire_thread = 0; -params.raw = 0; -params.part = -1; -params.cryptoctx = NULL; -params.decrypt = 0; +params.entire_thread = entire_thread; If you make params.entire_thread a notmuch_bool_t (instead of an int), you could pass params.entire_thread in the notmuch_opt_desc_t and get rid of the local variable. -argc--; argv++; /* skip subcommand argument */ +if (format_sel == NOTMUCH_FORMAT_NOT_SPECIFIED) { + /* if part was requested and format was not specified, use format=raw */ + if (params.part = 0) + format_sel = NOTMUCH_FORMAT_RAW; + else + format_sel = NOTMUCH_FORMAT_TEXT; +} -for (i = 0; i argc argv[i][0] == '-'; i++) { - if (strcmp (argv[i], --) == 0) { - i++; - break; +switch (format_sel) { +case NOTMUCH_FORMAT_JSON: + format = format_json; + params.entire_thread = 1; + break; +case NOTMUCH_FORMAT_TEXT: + format = format_text; + break; +case NOTMUCH_FORMAT_MBOX: + if (params.part 0) { + fprintf (stderr, Error: specifying parts is incompatible with mbox output format.\n); + return 1; } - if (STRNCMP_LITERAL (argv[i], --format=) == 0) { - opt = argv[i] + sizeof (--format=) - 1; - if (strcmp (opt, text) == 0) { - format = format_text; - } else if (strcmp (opt, json) == 0) { - format = format_json; - params.entire_thread = 1; - } else if (strcmp (opt, mbox) == 0) { - format = format_mbox; - mbox = 1; - } else if (strcmp (opt, raw) == 0) { - format = format_raw; - params.raw = 1; - } else { - fprintf (stderr, Invalid value for --format: %s\n, opt); - return 1; - } - format_specified = 1; - } else if (STRNCMP_LITERAL (argv[i], --part=) == 0) { - params.part = atoi(argv[i] + sizeof (--part=) - 1); - } else if (STRNCMP_LITERAL (argv[i], --entire-thread) == 0) { - params.entire_thread = 1; - } else if ((STRNCMP_LITERAL (argv[i], --verify) == 0) || -(STRNCMP_LITERAL (argv[i], --decrypt) == 0)) { - if (params.cryptoctx == NULL) { + format =
Re: [PATCH 1/2] cli: convert notmuch show to use the new argument parser
On Sun, 5 Feb 2012 23:12:05 -0500, Austin Clements amdra...@mit.edu wrote: Yikes. I don't envy you this patch. Two minor nits, otherwise this and the next patch LGTM. Thanks for the review. I'll roll another version, I think it will be better with your suggestions incorporated. Quoth Jani Nikula on Feb 04 at 12:41 am: Use the new notmuch argument parser to handle arguments in notmuch show. There are two corner case functional changes: 1) Also set params.raw = 1 when defaulting to raw format when part is requested but format is not specified. Huh. So notmuch show --part=0 query was broken before. Hmm, yes, it seems so. Do you think I should make this a separate fix? BTW an alternative would be to require setting --format if --part is specified, and not adapt the default format depending on --part. 2) Do not set params.decrypt if crypto context creation fails. Technically this also behaves differently if given multiple --format arguments, but I'll let that slide. Ugh, right, the old parsing was broken also that way. Luckily that falls in the corner case department too. Signed-off-by: Jani Nikula j...@nikula.org --- notmuch-show.c | 153 +--- 1 files changed, 79 insertions(+), 74 deletions(-) diff --git a/notmuch-show.c b/notmuch-show.c index dec799c..f93e121 100644 --- a/notmuch-show.c +++ b/notmuch-show.c @@ -1049,6 +1049,14 @@ do_show (void *ctx, return 0; } +enum { +NOTMUCH_FORMAT_NOT_SPECIFIED, +NOTMUCH_FORMAT_JSON, +NOTMUCH_FORMAT_TEXT, +NOTMUCH_FORMAT_MBOX, +NOTMUCH_FORMAT_RAW +}; + int notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[])) { @@ -1056,92 +1064,98 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[])) notmuch_database_t *notmuch; notmuch_query_t *query; char *query_string; -char *opt; +int opt_index; const notmuch_show_format_t *format = format_text; -notmuch_show_params_t params; -int mbox = 0; -int format_specified = 0; -int i; +notmuch_show_params_t params = { .part = -1 }; +int format_sel = NOTMUCH_FORMAT_NOT_SPECIFIED; +notmuch_bool_t decrypt = FALSE; +notmuch_bool_t verify = FALSE; +notmuch_bool_t entire_thread = FALSE; + +notmuch_opt_desc_t options[] = { + { NOTMUCH_OPT_KEYWORD, format_sel, format, 'f', + (notmuch_keyword_t []){ { json, NOTMUCH_FORMAT_JSON }, + { text, NOTMUCH_FORMAT_TEXT }, + { mbox, NOTMUCH_FORMAT_MBOX }, + { raw, NOTMUCH_FORMAT_RAW }, + { 0, 0 } } }, + { NOTMUCH_OPT_INT, params.part, part, 'p', 0 }, + { NOTMUCH_OPT_BOOLEAN, entire_thread, entire-thread, 't', 0 }, + { NOTMUCH_OPT_BOOLEAN, decrypt, decrypt, 'd', 0 }, + { NOTMUCH_OPT_BOOLEAN, verify, verify, 'v', 0 }, + { 0, 0, 0, 0, 0 } +}; + +opt_index = parse_arguments (argc, argv, options, 1); +if (opt_index 0) { + /* diagnostics already printed */ + return 1; +} -params.entire_thread = 0; -params.raw = 0; -params.part = -1; -params.cryptoctx = NULL; -params.decrypt = 0; +params.entire_thread = entire_thread; If you make params.entire_thread a notmuch_bool_t (instead of an int), you could pass params.entire_thread in the notmuch_opt_desc_t and get rid of the local variable. You're right; I was a bit lazy and didn't consider changing notmuch_show_params_t. I'll change both this and params.decrypt to notmuch_bool_t. -argc--; argv++; /* skip subcommand argument */ +if (format_sel == NOTMUCH_FORMAT_NOT_SPECIFIED) { + /* if part was requested and format was not specified, use format=raw */ + if (params.part = 0) + format_sel = NOTMUCH_FORMAT_RAW; + else + format_sel = NOTMUCH_FORMAT_TEXT; +} -for (i = 0; i argc argv[i][0] == '-'; i++) { - if (strcmp (argv[i], --) == 0) { - i++; - break; +switch (format_sel) { +case NOTMUCH_FORMAT_JSON: + format = format_json; + params.entire_thread = 1; + break; +case NOTMUCH_FORMAT_TEXT: + format = format_text; + break; +case NOTMUCH_FORMAT_MBOX: + if (params.part 0) { + fprintf (stderr, Error: specifying parts is incompatible with mbox output format.\n); + return 1; } - if (STRNCMP_LITERAL (argv[i], --format=) == 0) { - opt = argv[i] + sizeof (--format=) - 1; - if (strcmp (opt, text) == 0) { - format = format_text; - } else if (strcmp (opt, json) == 0) { - format = format_json; - params.entire_thread = 1; - } else if (strcmp (opt, mbox) == 0) { - format = format_mbox; - mbox = 1;
[PATCH 1/2] cli: convert "notmuch show" to use the new argument parser
On Sat, 04 Feb 2012 00:00:00 +, Mark Walters wrote: > > On Sat, 4 Feb 2012 00:41:08 +0200, Jani Nikula wrote: > > Use the new notmuch argument parser to handle arguments in "notmuch > > show". There are two corner case functional changes: > > > > 1) Also set params.raw = 1 when defaulting to raw format when part is > >requested but format is not specified. > > > > 2) Do not set params.decrypt if crypto context creation fails. > > This looks great. As far as I can see it is fine (I haven't run or even > compiled it yet). I only have two query/nits below. > > Best wishes > > Mark > > > Signed-off-by: Jani Nikula > > --- > > notmuch-show.c | 153 > > +--- > > 1 files changed, 79 insertions(+), 74 deletions(-) > > > > diff --git a/notmuch-show.c b/notmuch-show.c > > index dec799c..f93e121 100644 > > --- a/notmuch-show.c > > +++ b/notmuch-show.c > > @@ -1049,6 +1049,14 @@ do_show (void *ctx, > > return 0; > > } > > > > +enum { > > +NOTMUCH_FORMAT_NOT_SPECIFIED, > > +NOTMUCH_FORMAT_JSON, > > +NOTMUCH_FORMAT_TEXT, > > +NOTMUCH_FORMAT_MBOX, > > +NOTMUCH_FORMAT_RAW > > +}; > > + > > int > > notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[])) > > { > > @@ -1056,92 +1064,98 @@ notmuch_show_command (void *ctx, unused (int argc), > > unused (char *argv[])) > > notmuch_database_t *notmuch; > > notmuch_query_t *query; > > char *query_string; > > -char *opt; > > +int opt_index; > > const notmuch_show_format_t *format = _text; > > I think you don't need the default value here. If you think it is > clearer with the default then that is fine. I think I slightly prefer > without since in some cases the default is raw but entirely up to you. I actually dropped this at first, but the compiler has no way of knowing that all the cases are covered in the switch, and thinks format may be uninitialized. I was wondering whether to have a default case in the switch (which would be just to make the compiler happy), but settled on this instead. > > > -notmuch_show_params_t params; > > -int mbox = 0; > > -int format_specified = 0; > > -int i; > > +notmuch_show_params_t params = { .part = -1 }; > > Does this initialize all the other params bits to zero/NULL? Yes. It's called a "designated initializer for aggregate type" if you want to look it up. Thanks for the review. BR, Jani. > > > > > > +int format_sel = NOTMUCH_FORMAT_NOT_SPECIFIED; > > +notmuch_bool_t decrypt = FALSE; > > +notmuch_bool_t verify = FALSE; > > +notmuch_bool_t entire_thread = FALSE; > > + > > +notmuch_opt_desc_t options[] = { > > + { NOTMUCH_OPT_KEYWORD, _sel, "format", 'f', > > + (notmuch_keyword_t []){ { "json", NOTMUCH_FORMAT_JSON }, > > + { "text", NOTMUCH_FORMAT_TEXT }, > > + { "mbox", NOTMUCH_FORMAT_MBOX }, > > + { "raw", NOTMUCH_FORMAT_RAW }, > > + { 0, 0 } } }, > > + { NOTMUCH_OPT_INT, , "part", 'p', 0 }, > > + { NOTMUCH_OPT_BOOLEAN, _thread, "entire-thread", 't', 0 }, > > + { NOTMUCH_OPT_BOOLEAN, , "decrypt", 'd', 0 }, > > + { NOTMUCH_OPT_BOOLEAN, , "verify", 'v', 0 }, > > + { 0, 0, 0, 0, 0 } > > +}; > > + > > +opt_index = parse_arguments (argc, argv, options, 1); > > +if (opt_index < 0) { > > + /* diagnostics already printed */ > > + return 1; > > +} > > > > -params.entire_thread = 0; > > -params.raw = 0; > > -params.part = -1; > > -params.cryptoctx = NULL; > > -params.decrypt = 0; > > +params.entire_thread = entire_thread; > > > > -argc--; argv++; /* skip subcommand argument */ > > +if (format_sel == NOTMUCH_FORMAT_NOT_SPECIFIED) { > > + /* if part was requested and format was not specified, use format=raw */ > > + if (params.part >= 0) > > + format_sel = NOTMUCH_FORMAT_RAW; > > + else > > + format_sel = NOTMUCH_FORMAT_TEXT; > > +} > > > > -for (i = 0; i < argc && argv[i][0] == '-'; i++) { > > - if (strcmp (argv[i], "--") == 0) { > > - i++; > > - break; > > +switch (format_sel) { > > +case NOTMUCH_FORMAT_JSON: > > + format = _json; > > + params.entire_thread = 1; > > + break; > > +case NOTMUCH_FORMAT_TEXT: > > + format = _text; > > + break; > > +case NOTMUCH_FORMAT_MBOX: > > + if (params.part > 0) { > > + fprintf (stderr, "Error: specifying parts is incompatible with mbox > > output format.\n"); > > + return 1; > > } > > - if (STRNCMP_LITERAL (argv[i], "--format=") == 0) { > > - opt = argv[i] + sizeof ("--format=") - 1; > > - if (strcmp (opt, "text") == 0) { > > - format = _text; > > - } else if (strcmp (opt, "json") == 0) { > > - format = _json; > > - params.entire_thread = 1; > > - } else if (strcmp (opt, "mbox") == 0) { > >
[PATCH 1/2] cli: convert "notmuch show" to use the new argument parser
Use the new notmuch argument parser to handle arguments in "notmuch show". There are two corner case functional changes: 1) Also set params.raw = 1 when defaulting to raw format when part is requested but format is not specified. 2) Do not set params.decrypt if crypto context creation fails. Signed-off-by: Jani Nikula --- notmuch-show.c | 153 +--- 1 files changed, 79 insertions(+), 74 deletions(-) diff --git a/notmuch-show.c b/notmuch-show.c index dec799c..f93e121 100644 --- a/notmuch-show.c +++ b/notmuch-show.c @@ -1049,6 +1049,14 @@ do_show (void *ctx, return 0; } +enum { +NOTMUCH_FORMAT_NOT_SPECIFIED, +NOTMUCH_FORMAT_JSON, +NOTMUCH_FORMAT_TEXT, +NOTMUCH_FORMAT_MBOX, +NOTMUCH_FORMAT_RAW +}; + int notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[])) { @@ -1056,92 +1064,98 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[])) notmuch_database_t *notmuch; notmuch_query_t *query; char *query_string; -char *opt; +int opt_index; const notmuch_show_format_t *format = _text; -notmuch_show_params_t params; -int mbox = 0; -int format_specified = 0; -int i; +notmuch_show_params_t params = { .part = -1 }; +int format_sel = NOTMUCH_FORMAT_NOT_SPECIFIED; +notmuch_bool_t decrypt = FALSE; +notmuch_bool_t verify = FALSE; +notmuch_bool_t entire_thread = FALSE; + +notmuch_opt_desc_t options[] = { + { NOTMUCH_OPT_KEYWORD, _sel, "format", 'f', + (notmuch_keyword_t []){ { "json", NOTMUCH_FORMAT_JSON }, + { "text", NOTMUCH_FORMAT_TEXT }, + { "mbox", NOTMUCH_FORMAT_MBOX }, + { "raw", NOTMUCH_FORMAT_RAW }, + { 0, 0 } } }, + { NOTMUCH_OPT_INT, , "part", 'p', 0 }, + { NOTMUCH_OPT_BOOLEAN, _thread, "entire-thread", 't', 0 }, + { NOTMUCH_OPT_BOOLEAN, , "decrypt", 'd', 0 }, + { NOTMUCH_OPT_BOOLEAN, , "verify", 'v', 0 }, + { 0, 0, 0, 0, 0 } +}; + +opt_index = parse_arguments (argc, argv, options, 1); +if (opt_index < 0) { + /* diagnostics already printed */ + return 1; +} -params.entire_thread = 0; -params.raw = 0; -params.part = -1; -params.cryptoctx = NULL; -params.decrypt = 0; +params.entire_thread = entire_thread; -argc--; argv++; /* skip subcommand argument */ +if (format_sel == NOTMUCH_FORMAT_NOT_SPECIFIED) { + /* if part was requested and format was not specified, use format=raw */ + if (params.part >= 0) + format_sel = NOTMUCH_FORMAT_RAW; + else + format_sel = NOTMUCH_FORMAT_TEXT; +} -for (i = 0; i < argc && argv[i][0] == '-'; i++) { - if (strcmp (argv[i], "--") == 0) { - i++; - break; +switch (format_sel) { +case NOTMUCH_FORMAT_JSON: + format = _json; + params.entire_thread = 1; + break; +case NOTMUCH_FORMAT_TEXT: + format = _text; + break; +case NOTMUCH_FORMAT_MBOX: + if (params.part > 0) { + fprintf (stderr, "Error: specifying parts is incompatible with mbox output format.\n"); + return 1; } - if (STRNCMP_LITERAL (argv[i], "--format=") == 0) { - opt = argv[i] + sizeof ("--format=") - 1; - if (strcmp (opt, "text") == 0) { - format = _text; - } else if (strcmp (opt, "json") == 0) { - format = _json; - params.entire_thread = 1; - } else if (strcmp (opt, "mbox") == 0) { - format = _mbox; - mbox = 1; - } else if (strcmp (opt, "raw") == 0) { - format = _raw; - params.raw = 1; - } else { - fprintf (stderr, "Invalid value for --format: %s\n", opt); - return 1; - } - format_specified = 1; - } else if (STRNCMP_LITERAL (argv[i], "--part=") == 0) { - params.part = atoi(argv[i] + sizeof ("--part=") - 1); - } else if (STRNCMP_LITERAL (argv[i], "--entire-thread") == 0) { - params.entire_thread = 1; - } else if ((STRNCMP_LITERAL (argv[i], "--verify") == 0) || - (STRNCMP_LITERAL (argv[i], "--decrypt") == 0)) { - if (params.cryptoctx == NULL) { + format = _mbox; + break; +case NOTMUCH_FORMAT_RAW: + format = _raw; + /* If --format=raw specified without specifying part, we can only +* output single message, so set part=0 */ + if (params.part < 0) + params.part = 0; + params.raw = 1; + break; +} + +if (decrypt || verify) { #ifdef GMIME_ATLEAST_26 - /* TODO: GMimePasswordRequestFunc */ - if (NULL == (params.cryptoctx = g_mime_gpg_context_new(NULL, "gpg"))) + /* TODO:
[PATCH 1/2] cli: convert "notmuch show" to use the new argument parser
On Sat, 4 Feb 2012 00:41:08 +0200, Jani Nikula wrote: > Use the new notmuch argument parser to handle arguments in "notmuch > show". There are two corner case functional changes: > > 1) Also set params.raw = 1 when defaulting to raw format when part is >requested but format is not specified. > > 2) Do not set params.decrypt if crypto context creation fails. This looks great. As far as I can see it is fine (I haven't run or even compiled it yet). I only have two query/nits below. Best wishes Mark > Signed-off-by: Jani Nikula > --- > notmuch-show.c | 153 > +--- > 1 files changed, 79 insertions(+), 74 deletions(-) > > diff --git a/notmuch-show.c b/notmuch-show.c > index dec799c..f93e121 100644 > --- a/notmuch-show.c > +++ b/notmuch-show.c > @@ -1049,6 +1049,14 @@ do_show (void *ctx, > return 0; > } > > +enum { > +NOTMUCH_FORMAT_NOT_SPECIFIED, > +NOTMUCH_FORMAT_JSON, > +NOTMUCH_FORMAT_TEXT, > +NOTMUCH_FORMAT_MBOX, > +NOTMUCH_FORMAT_RAW > +}; > + > int > notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[])) > { > @@ -1056,92 +1064,98 @@ notmuch_show_command (void *ctx, unused (int argc), > unused (char *argv[])) > notmuch_database_t *notmuch; > notmuch_query_t *query; > char *query_string; > -char *opt; > +int opt_index; > const notmuch_show_format_t *format = _text; I think you don't need the default value here. If you think it is clearer with the default then that is fine. I think I slightly prefer without since in some cases the default is raw but entirely up to you. > -notmuch_show_params_t params; > -int mbox = 0; > -int format_specified = 0; > -int i; > +notmuch_show_params_t params = { .part = -1 }; Does this initialize all the other params bits to zero/NULL? > +int format_sel = NOTMUCH_FORMAT_NOT_SPECIFIED; > +notmuch_bool_t decrypt = FALSE; > +notmuch_bool_t verify = FALSE; > +notmuch_bool_t entire_thread = FALSE; > + > +notmuch_opt_desc_t options[] = { > + { NOTMUCH_OPT_KEYWORD, _sel, "format", 'f', > + (notmuch_keyword_t []){ { "json", NOTMUCH_FORMAT_JSON }, > + { "text", NOTMUCH_FORMAT_TEXT }, > + { "mbox", NOTMUCH_FORMAT_MBOX }, > + { "raw", NOTMUCH_FORMAT_RAW }, > + { 0, 0 } } }, > + { NOTMUCH_OPT_INT, , "part", 'p', 0 }, > + { NOTMUCH_OPT_BOOLEAN, _thread, "entire-thread", 't', 0 }, > + { NOTMUCH_OPT_BOOLEAN, , "decrypt", 'd', 0 }, > + { NOTMUCH_OPT_BOOLEAN, , "verify", 'v', 0 }, > + { 0, 0, 0, 0, 0 } > +}; > + > +opt_index = parse_arguments (argc, argv, options, 1); > +if (opt_index < 0) { > + /* diagnostics already printed */ > + return 1; > +} > > -params.entire_thread = 0; > -params.raw = 0; > -params.part = -1; > -params.cryptoctx = NULL; > -params.decrypt = 0; > +params.entire_thread = entire_thread; > > -argc--; argv++; /* skip subcommand argument */ > +if (format_sel == NOTMUCH_FORMAT_NOT_SPECIFIED) { > + /* if part was requested and format was not specified, use format=raw */ > + if (params.part >= 0) > + format_sel = NOTMUCH_FORMAT_RAW; > + else > + format_sel = NOTMUCH_FORMAT_TEXT; > +} > > -for (i = 0; i < argc && argv[i][0] == '-'; i++) { > - if (strcmp (argv[i], "--") == 0) { > - i++; > - break; > +switch (format_sel) { > +case NOTMUCH_FORMAT_JSON: > + format = _json; > + params.entire_thread = 1; > + break; > +case NOTMUCH_FORMAT_TEXT: > + format = _text; > + break; > +case NOTMUCH_FORMAT_MBOX: > + if (params.part > 0) { > + fprintf (stderr, "Error: specifying parts is incompatible with mbox > output format.\n"); > + return 1; > } > - if (STRNCMP_LITERAL (argv[i], "--format=") == 0) { > - opt = argv[i] + sizeof ("--format=") - 1; > - if (strcmp (opt, "text") == 0) { > - format = _text; > - } else if (strcmp (opt, "json") == 0) { > - format = _json; > - params.entire_thread = 1; > - } else if (strcmp (opt, "mbox") == 0) { > - format = _mbox; > - mbox = 1; > - } else if (strcmp (opt, "raw") == 0) { > - format = _raw; > - params.raw = 1; > - } else { > - fprintf (stderr, "Invalid value for --format: %s\n", opt); > - return 1; > - } > - format_specified = 1; > - } else if (STRNCMP_LITERAL (argv[i], "--part=") == 0) { > - params.part = atoi(argv[i] + sizeof ("--part=") - 1); > - } else if (STRNCMP_LITERAL (argv[i], "--entire-thread") == 0) { > - params.entire_thread = 1; > - } else if ((STRNCMP_LITERAL (argv[i], "--verify") == 0) || > -
Re: [PATCH 1/2] cli: convert notmuch show to use the new argument parser
On Sat, 04 Feb 2012 00:00:00 +, Mark Walters markwalters1...@gmail.com wrote: On Sat, 4 Feb 2012 00:41:08 +0200, Jani Nikula j...@nikula.org wrote: Use the new notmuch argument parser to handle arguments in notmuch show. There are two corner case functional changes: 1) Also set params.raw = 1 when defaulting to raw format when part is requested but format is not specified. 2) Do not set params.decrypt if crypto context creation fails. This looks great. As far as I can see it is fine (I haven't run or even compiled it yet). I only have two query/nits below. Best wishes Mark Signed-off-by: Jani Nikula j...@nikula.org --- notmuch-show.c | 153 +--- 1 files changed, 79 insertions(+), 74 deletions(-) diff --git a/notmuch-show.c b/notmuch-show.c index dec799c..f93e121 100644 --- a/notmuch-show.c +++ b/notmuch-show.c @@ -1049,6 +1049,14 @@ do_show (void *ctx, return 0; } +enum { +NOTMUCH_FORMAT_NOT_SPECIFIED, +NOTMUCH_FORMAT_JSON, +NOTMUCH_FORMAT_TEXT, +NOTMUCH_FORMAT_MBOX, +NOTMUCH_FORMAT_RAW +}; + int notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[])) { @@ -1056,92 +1064,98 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[])) notmuch_database_t *notmuch; notmuch_query_t *query; char *query_string; -char *opt; +int opt_index; const notmuch_show_format_t *format = format_text; I think you don't need the default value here. If you think it is clearer with the default then that is fine. I think I slightly prefer without since in some cases the default is raw but entirely up to you. I actually dropped this at first, but the compiler has no way of knowing that all the cases are covered in the switch, and thinks format may be uninitialized. I was wondering whether to have a default case in the switch (which would be just to make the compiler happy), but settled on this instead. -notmuch_show_params_t params; -int mbox = 0; -int format_specified = 0; -int i; +notmuch_show_params_t params = { .part = -1 }; Does this initialize all the other params bits to zero/NULL? Yes. It's called a designated initializer for aggregate type if you want to look it up. Thanks for the review. BR, Jani. +int format_sel = NOTMUCH_FORMAT_NOT_SPECIFIED; +notmuch_bool_t decrypt = FALSE; +notmuch_bool_t verify = FALSE; +notmuch_bool_t entire_thread = FALSE; + +notmuch_opt_desc_t options[] = { + { NOTMUCH_OPT_KEYWORD, format_sel, format, 'f', + (notmuch_keyword_t []){ { json, NOTMUCH_FORMAT_JSON }, + { text, NOTMUCH_FORMAT_TEXT }, + { mbox, NOTMUCH_FORMAT_MBOX }, + { raw, NOTMUCH_FORMAT_RAW }, + { 0, 0 } } }, + { NOTMUCH_OPT_INT, params.part, part, 'p', 0 }, + { NOTMUCH_OPT_BOOLEAN, entire_thread, entire-thread, 't', 0 }, + { NOTMUCH_OPT_BOOLEAN, decrypt, decrypt, 'd', 0 }, + { NOTMUCH_OPT_BOOLEAN, verify, verify, 'v', 0 }, + { 0, 0, 0, 0, 0 } +}; + +opt_index = parse_arguments (argc, argv, options, 1); +if (opt_index 0) { + /* diagnostics already printed */ + return 1; +} -params.entire_thread = 0; -params.raw = 0; -params.part = -1; -params.cryptoctx = NULL; -params.decrypt = 0; +params.entire_thread = entire_thread; -argc--; argv++; /* skip subcommand argument */ +if (format_sel == NOTMUCH_FORMAT_NOT_SPECIFIED) { + /* if part was requested and format was not specified, use format=raw */ + if (params.part = 0) + format_sel = NOTMUCH_FORMAT_RAW; + else + format_sel = NOTMUCH_FORMAT_TEXT; +} -for (i = 0; i argc argv[i][0] == '-'; i++) { - if (strcmp (argv[i], --) == 0) { - i++; - break; +switch (format_sel) { +case NOTMUCH_FORMAT_JSON: + format = format_json; + params.entire_thread = 1; + break; +case NOTMUCH_FORMAT_TEXT: + format = format_text; + break; +case NOTMUCH_FORMAT_MBOX: + if (params.part 0) { + fprintf (stderr, Error: specifying parts is incompatible with mbox output format.\n); + return 1; } - if (STRNCMP_LITERAL (argv[i], --format=) == 0) { - opt = argv[i] + sizeof (--format=) - 1; - if (strcmp (opt, text) == 0) { - format = format_text; - } else if (strcmp (opt, json) == 0) { - format = format_json; - params.entire_thread = 1; - } else if (strcmp (opt, mbox) == 0) { - format = format_mbox; - mbox = 1; - } else if (strcmp (opt, raw) == 0) { - format = format_raw; - params.raw =
[PATCH 1/2] cli: convert notmuch show to use the new argument parser
Use the new notmuch argument parser to handle arguments in notmuch show. There are two corner case functional changes: 1) Also set params.raw = 1 when defaulting to raw format when part is requested but format is not specified. 2) Do not set params.decrypt if crypto context creation fails. Signed-off-by: Jani Nikula j...@nikula.org --- notmuch-show.c | 153 +--- 1 files changed, 79 insertions(+), 74 deletions(-) diff --git a/notmuch-show.c b/notmuch-show.c index dec799c..f93e121 100644 --- a/notmuch-show.c +++ b/notmuch-show.c @@ -1049,6 +1049,14 @@ do_show (void *ctx, return 0; } +enum { +NOTMUCH_FORMAT_NOT_SPECIFIED, +NOTMUCH_FORMAT_JSON, +NOTMUCH_FORMAT_TEXT, +NOTMUCH_FORMAT_MBOX, +NOTMUCH_FORMAT_RAW +}; + int notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[])) { @@ -1056,92 +1064,98 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[])) notmuch_database_t *notmuch; notmuch_query_t *query; char *query_string; -char *opt; +int opt_index; const notmuch_show_format_t *format = format_text; -notmuch_show_params_t params; -int mbox = 0; -int format_specified = 0; -int i; +notmuch_show_params_t params = { .part = -1 }; +int format_sel = NOTMUCH_FORMAT_NOT_SPECIFIED; +notmuch_bool_t decrypt = FALSE; +notmuch_bool_t verify = FALSE; +notmuch_bool_t entire_thread = FALSE; + +notmuch_opt_desc_t options[] = { + { NOTMUCH_OPT_KEYWORD, format_sel, format, 'f', + (notmuch_keyword_t []){ { json, NOTMUCH_FORMAT_JSON }, + { text, NOTMUCH_FORMAT_TEXT }, + { mbox, NOTMUCH_FORMAT_MBOX }, + { raw, NOTMUCH_FORMAT_RAW }, + { 0, 0 } } }, + { NOTMUCH_OPT_INT, params.part, part, 'p', 0 }, + { NOTMUCH_OPT_BOOLEAN, entire_thread, entire-thread, 't', 0 }, + { NOTMUCH_OPT_BOOLEAN, decrypt, decrypt, 'd', 0 }, + { NOTMUCH_OPT_BOOLEAN, verify, verify, 'v', 0 }, + { 0, 0, 0, 0, 0 } +}; + +opt_index = parse_arguments (argc, argv, options, 1); +if (opt_index 0) { + /* diagnostics already printed */ + return 1; +} -params.entire_thread = 0; -params.raw = 0; -params.part = -1; -params.cryptoctx = NULL; -params.decrypt = 0; +params.entire_thread = entire_thread; -argc--; argv++; /* skip subcommand argument */ +if (format_sel == NOTMUCH_FORMAT_NOT_SPECIFIED) { + /* if part was requested and format was not specified, use format=raw */ + if (params.part = 0) + format_sel = NOTMUCH_FORMAT_RAW; + else + format_sel = NOTMUCH_FORMAT_TEXT; +} -for (i = 0; i argc argv[i][0] == '-'; i++) { - if (strcmp (argv[i], --) == 0) { - i++; - break; +switch (format_sel) { +case NOTMUCH_FORMAT_JSON: + format = format_json; + params.entire_thread = 1; + break; +case NOTMUCH_FORMAT_TEXT: + format = format_text; + break; +case NOTMUCH_FORMAT_MBOX: + if (params.part 0) { + fprintf (stderr, Error: specifying parts is incompatible with mbox output format.\n); + return 1; } - if (STRNCMP_LITERAL (argv[i], --format=) == 0) { - opt = argv[i] + sizeof (--format=) - 1; - if (strcmp (opt, text) == 0) { - format = format_text; - } else if (strcmp (opt, json) == 0) { - format = format_json; - params.entire_thread = 1; - } else if (strcmp (opt, mbox) == 0) { - format = format_mbox; - mbox = 1; - } else if (strcmp (opt, raw) == 0) { - format = format_raw; - params.raw = 1; - } else { - fprintf (stderr, Invalid value for --format: %s\n, opt); - return 1; - } - format_specified = 1; - } else if (STRNCMP_LITERAL (argv[i], --part=) == 0) { - params.part = atoi(argv[i] + sizeof (--part=) - 1); - } else if (STRNCMP_LITERAL (argv[i], --entire-thread) == 0) { - params.entire_thread = 1; - } else if ((STRNCMP_LITERAL (argv[i], --verify) == 0) || - (STRNCMP_LITERAL (argv[i], --decrypt) == 0)) { - if (params.cryptoctx == NULL) { + format = format_mbox; + break; +case NOTMUCH_FORMAT_RAW: + format = format_raw; + /* If --format=raw specified without specifying part, we can only +* output single message, so set part=0 */ + if (params.part 0) + params.part = 0; + params.raw = 1; + break; +} + +if (decrypt || verify) { #ifdef GMIME_ATLEAST_26 - /* TODO: GMimePasswordRequestFunc */ - if (NULL == (params.cryptoctx =
Re: [PATCH 1/2] cli: convert notmuch show to use the new argument parser
On Sat, 4 Feb 2012 00:41:08 +0200, Jani Nikula j...@nikula.org wrote: Use the new notmuch argument parser to handle arguments in notmuch show. There are two corner case functional changes: 1) Also set params.raw = 1 when defaulting to raw format when part is requested but format is not specified. 2) Do not set params.decrypt if crypto context creation fails. This looks great. As far as I can see it is fine (I haven't run or even compiled it yet). I only have two query/nits below. Best wishes Mark Signed-off-by: Jani Nikula j...@nikula.org --- notmuch-show.c | 153 +--- 1 files changed, 79 insertions(+), 74 deletions(-) diff --git a/notmuch-show.c b/notmuch-show.c index dec799c..f93e121 100644 --- a/notmuch-show.c +++ b/notmuch-show.c @@ -1049,6 +1049,14 @@ do_show (void *ctx, return 0; } +enum { +NOTMUCH_FORMAT_NOT_SPECIFIED, +NOTMUCH_FORMAT_JSON, +NOTMUCH_FORMAT_TEXT, +NOTMUCH_FORMAT_MBOX, +NOTMUCH_FORMAT_RAW +}; + int notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[])) { @@ -1056,92 +1064,98 @@ notmuch_show_command (void *ctx, unused (int argc), unused (char *argv[])) notmuch_database_t *notmuch; notmuch_query_t *query; char *query_string; -char *opt; +int opt_index; const notmuch_show_format_t *format = format_text; I think you don't need the default value here. If you think it is clearer with the default then that is fine. I think I slightly prefer without since in some cases the default is raw but entirely up to you. -notmuch_show_params_t params; -int mbox = 0; -int format_specified = 0; -int i; +notmuch_show_params_t params = { .part = -1 }; Does this initialize all the other params bits to zero/NULL? +int format_sel = NOTMUCH_FORMAT_NOT_SPECIFIED; +notmuch_bool_t decrypt = FALSE; +notmuch_bool_t verify = FALSE; +notmuch_bool_t entire_thread = FALSE; + +notmuch_opt_desc_t options[] = { + { NOTMUCH_OPT_KEYWORD, format_sel, format, 'f', + (notmuch_keyword_t []){ { json, NOTMUCH_FORMAT_JSON }, + { text, NOTMUCH_FORMAT_TEXT }, + { mbox, NOTMUCH_FORMAT_MBOX }, + { raw, NOTMUCH_FORMAT_RAW }, + { 0, 0 } } }, + { NOTMUCH_OPT_INT, params.part, part, 'p', 0 }, + { NOTMUCH_OPT_BOOLEAN, entire_thread, entire-thread, 't', 0 }, + { NOTMUCH_OPT_BOOLEAN, decrypt, decrypt, 'd', 0 }, + { NOTMUCH_OPT_BOOLEAN, verify, verify, 'v', 0 }, + { 0, 0, 0, 0, 0 } +}; + +opt_index = parse_arguments (argc, argv, options, 1); +if (opt_index 0) { + /* diagnostics already printed */ + return 1; +} -params.entire_thread = 0; -params.raw = 0; -params.part = -1; -params.cryptoctx = NULL; -params.decrypt = 0; +params.entire_thread = entire_thread; -argc--; argv++; /* skip subcommand argument */ +if (format_sel == NOTMUCH_FORMAT_NOT_SPECIFIED) { + /* if part was requested and format was not specified, use format=raw */ + if (params.part = 0) + format_sel = NOTMUCH_FORMAT_RAW; + else + format_sel = NOTMUCH_FORMAT_TEXT; +} -for (i = 0; i argc argv[i][0] == '-'; i++) { - if (strcmp (argv[i], --) == 0) { - i++; - break; +switch (format_sel) { +case NOTMUCH_FORMAT_JSON: + format = format_json; + params.entire_thread = 1; + break; +case NOTMUCH_FORMAT_TEXT: + format = format_text; + break; +case NOTMUCH_FORMAT_MBOX: + if (params.part 0) { + fprintf (stderr, Error: specifying parts is incompatible with mbox output format.\n); + return 1; } - if (STRNCMP_LITERAL (argv[i], --format=) == 0) { - opt = argv[i] + sizeof (--format=) - 1; - if (strcmp (opt, text) == 0) { - format = format_text; - } else if (strcmp (opt, json) == 0) { - format = format_json; - params.entire_thread = 1; - } else if (strcmp (opt, mbox) == 0) { - format = format_mbox; - mbox = 1; - } else if (strcmp (opt, raw) == 0) { - format = format_raw; - params.raw = 1; - } else { - fprintf (stderr, Invalid value for --format: %s\n, opt); - return 1; - } - format_specified = 1; - } else if (STRNCMP_LITERAL (argv[i], --part=) == 0) { - params.part = atoi(argv[i] + sizeof (--part=) - 1); - } else if (STRNCMP_LITERAL (argv[i], --entire-thread) == 0) { - params.entire_thread = 1; - } else if ((STRNCMP_LITERAL (argv[i], --verify) == 0) || -(STRNCMP_LITERAL (argv[i], --decrypt) == 0)) { - if (params.cryptoctx ==