{"thread":{"id":"50334","subject":"[PATCH 00/14] nd/diff-parseopt part 1","startedAt":"2019-01-27T00:36:05Z","lastAt":"2019-04-16T14:24:43Z","messageCount":19,"participants":["Nguyễn Thái Ngọc Duy","Junio C Hamano","Derrick Stolee","Duy Nguyen"],"isPatch":true,"patchVersion":1,"patchTotal":14},"messages":[{"id":"367729","messageId":"20190127003535.28341-1-pclouds@gmail.com","threadId":"50334","inReplyTo":null,"subject":"[PATCH 00/14] nd/diff-parseopt part 1","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2019-01-27T00:35:21Z","receivedAt":"2019-01-27T00:36:05Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This is the first part of converting diff_opt_parse() to use\nparse_options(). Compared to the full series [1] I sent earlier, 03/76\nis dropped and 02/14 is updated to allow KEEP_UNKNOWN and\nSTOP_AT_NON_OPTION combination, but only for one shot mode.\n\n[1] https://public-inbox.org/git/20190117130615.18732-1-pclouds@gmail.com/\n\nNguyễn Thái Ngọc Duy (14):\n  parse-options.h: remove extern on function prototypes\n  parse-options: add one-shot mode\n  parse-options: disable option abbreviation with PARSE_OPT_KEEP_UNKNOWN\n  parse-options: add OPT_BITOP()\n  parse-options: stop abusing 'callback' for lowlevel callbacks\n  parse-options: avoid magic return codes\n  parse-options: allow ll_callback with OPTION_CALLBACK\n  diff.h: keep forward struct declarations sorted\n  diff.h: avoid bit fields in struct diff_flags\n  diff.c: prepare to use parse_options() for parsing\n  diff.c: convert -u|-p|--patch\n  diff.c: convert -U|--unified\n  diff.c: convert -W|--[no-]function-context\n  diff.c: convert --raw\n\n Documentation/diff-options.txt |   2 +-\n builtin/blame.c                |   2 +-\n builtin/merge.c                |   9 +-\n builtin/update-index.c         |  41 +++++----\n diff.c                         |  71 ++++++++++++---\n diff.h                         |  80 ++++++++---------\n parse-options-cb.c             |   7 +-\n parse-options.c                | 152 ++++++++++++++++++++++++---------\n parse-options.h                | 108 +++++++++++++----------\n t/t7800-difftool.sh            |   4 +-\n 10 files changed, 318 insertions(+), 158 deletions(-)\n\n-- \n2.20.1.560.g70ca8b83ee\n\n"},{"id":"367730","messageId":"20190127003535.28341-2-pclouds@gmail.com","threadId":"50334","inReplyTo":"20190127003535.28341-1-pclouds@gmail.com","subject":"[PATCH 01/14] parse-options.h: remove extern on function prototypes","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2019-01-27T00:35:22Z","receivedAt":"2019-01-27T00:36:09Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n parse-options.h | 58 ++++++++++++++++++++++++-------------------------\n 1 file changed, 29 insertions(+), 29 deletions(-)\n\ndiff --git a/parse-options.h b/parse-options.h\nindex 14fe32428e..f5e7ec7d23 100644\n--- a/parse-options.h\n+++ b/parse-options.h\n@@ -174,18 +174,18 @@ struct option {\n  * for translation with N_().\n  * Returns the number of arguments left in argv[].\n  */\n-extern int parse_options(int argc, const char **argv, const char *prefix,\n-\t\t\t const struct option *options,\n-\t\t\t const char * const usagestr[], int flags);\n+int parse_options(int argc, const char **argv, const char *prefix,\n+\t\t  const struct option *options,\n+\t\t  const char * const usagestr[], int flags);\n \n-extern NORETURN void usage_with_options(const char * const *usagestr,\n-\t\t\t\t\tconst struct option *options);\n+NORETURN void usage_with_options(const char * const *usagestr,\n+\t\t\t\t const struct option *options);\n \n-extern NORETURN void usage_msg_opt(const char *msg,\n-\t\t\t\t   const char * const *usagestr,\n-\t\t\t\t   const struct option *options);\n+NORETURN void usage_msg_opt(const char *msg,\n+\t\t\t    const char * const *usagestr,\n+\t\t\t    const struct option *options);\n \n-extern int optbug(const struct option *opt, const char *reason);\n+int optbug(const struct option *opt, const char *reason);\n const char *optname(const struct option *opt, int flags);\n \n /*\n@@ -227,31 +227,31 @@ struct parse_opt_ctx_t {\n \tconst char *prefix;\n };\n \n-extern void parse_options_start(struct parse_opt_ctx_t *ctx,\n-\t\t\t\tint argc, const char **argv, const char *prefix,\n-\t\t\t\tconst struct option *options, int flags);\n+void parse_options_start(struct parse_opt_ctx_t *ctx,\n+\t\t\t int argc, const char **argv, const char *prefix,\n+\t\t\t const struct option *options, int flags);\n \n-extern int parse_options_step(struct parse_opt_ctx_t *ctx,\n-\t\t\t      const struct option *options,\n-\t\t\t      const char * const usagestr[]);\n+int parse_options_step(struct parse_opt_ctx_t *ctx,\n+\t\t       const struct option *options,\n+\t\t       const char * const usagestr[]);\n \n-extern int parse_options_end(struct parse_opt_ctx_t *ctx);\n+int parse_options_end(struct parse_opt_ctx_t *ctx);\n \n-extern struct option *parse_options_concat(struct option *a, struct option *b);\n+struct option *parse_options_concat(struct option *a, struct option *b);\n \n /*----- some often used options -----*/\n-extern int parse_opt_abbrev_cb(const struct option *, const char *, int);\n-extern int parse_opt_expiry_date_cb(const struct option *, const char *, int);\n-extern int parse_opt_color_flag_cb(const struct option *, const char *, int);\n-extern int parse_opt_verbosity_cb(const struct option *, const char *, int);\n-extern int parse_opt_object_name(const struct option *, const char *, int);\n-extern int parse_opt_commits(const struct option *, const char *, int);\n-extern int parse_opt_tertiary(const struct option *, const char *, int);\n-extern int parse_opt_string_list(const struct option *, const char *, int);\n-extern int parse_opt_noop_cb(const struct option *, const char *, int);\n-extern int parse_opt_unknown_cb(const struct option *, const char *, int);\n-extern int parse_opt_passthru(const struct option *, const char *, int);\n-extern int parse_opt_passthru_argv(const struct option *, const char *, int);\n+int parse_opt_abbrev_cb(const struct option *, const char *, int);\n+int parse_opt_expiry_date_cb(const struct option *, const char *, int);\n+int parse_opt_color_flag_cb(const struct option *, const char *, int);\n+int parse_opt_verbosity_cb(const struct option *, const char *, int);\n+int parse_opt_object_name(const struct option *, const char *, int);\n+int parse_opt_commits(const struct option *, const char *, int);\n+int parse_opt_tertiary(const struct option *, const char *, int);\n+int parse_opt_string_list(const struct option *, const char *, int);\n+int parse_opt_noop_cb(const struct option *, const char *, int);\n+int parse_opt_unknown_cb(const struct option *, const char *, int);\n+int parse_opt_passthru(const struct option *, const char *, int);\n+int parse_opt_passthru_argv(const struct option *, const char *, int);\n \n #define OPT__VERBOSE(var, h)  OPT_COUNTUP('v', \"verbose\", (var), (h))\n #define OPT__QUIET(var, h)    OPT_COUNTUP('q', \"quiet\",   (var), (h))\n-- \n2.20.1.560.g70ca8b83ee\n\n"},{"id":"367731","messageId":"20190127003535.28341-3-pclouds@gmail.com","threadId":"50334","inReplyTo":"20190127003535.28341-1-pclouds@gmail.com","subject":"[PATCH 02/14] parse-options: add one-shot mode","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2019-01-27T00:35:23Z","receivedAt":"2019-01-27T00:36:15Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This is to help reimplement diff_opt_parse() using parse_options().\nThe behavior of parse_options() is changed to be the same as the\nother:\n\n- no argv0 in argv[], everything can be processed\n- argv[] must not be updated, it's the caller's job to do that\n- return the number of arguments processed\n- leave all unknown options / non-options alone (this one can already\n  be achieved with PARSE_OPT_KEEP_UNKNOWN and\n  PARSE_OPT_STOP_AT_NON_OPTION)\n\nThis mode is NOT supposed to stay here for long. It's to help\nconverting diff/rev option parsing. Once that work is over and we can\njust use parse_options() throughout the code base, this will be\ndeleted.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n parse-options.c | 26 ++++++++++++++++++++++----\n parse-options.h | 17 +++++++++++++----\n 2 files changed, 35 insertions(+), 8 deletions(-)\n\ndiff --git a/parse-options.c b/parse-options.c\nindex 9f84bacce6..740ae5438f 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -416,15 +416,24 @@ void parse_options_start(struct parse_opt_ctx_t *ctx,\n \t\t\t const struct option *options, int flags)\n {\n \tmemset(ctx, 0, sizeof(*ctx));\n-\tctx->argc = ctx->total = argc - 1;\n-\tctx->argv = argv + 1;\n-\tctx->out  = argv;\n+\tctx->argc = argc;\n+\tctx->argv = argv;\n+\tif (!(flags & PARSE_OPT_ONE_SHOT)) {\n+\t\tctx->argc--;\n+\t\tctx->argv++;\n+\t}\n+\tctx->total = ctx->argc;\n+\tctx->out   = argv;\n \tctx->prefix = prefix;\n \tctx->cpidx = ((flags & PARSE_OPT_KEEP_ARGV0) != 0);\n \tctx->flags = flags;\n \tif ((flags & PARSE_OPT_KEEP_UNKNOWN) &&\n-\t    (flags & PARSE_OPT_STOP_AT_NON_OPTION))\n+\t    (flags & PARSE_OPT_STOP_AT_NON_OPTION) &&\n+\t    !(flags & PARSE_OPT_ONE_SHOT))\n \t\tBUG(\"STOP_AT_NON_OPTION and KEEP_UNKNOWN don't go together\");\n+\tif ((flags & PARSE_OPT_ONE_SHOT) &&\n+\t    (flags & PARSE_OPT_KEEP_ARGV0))\n+\t\tBUG(\"Can't keep argv0 if you don't have it\");\n \tparse_options_check(options);\n }\n \n@@ -536,6 +545,10 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,\n \tfor (; ctx->argc; ctx->argc--, ctx->argv++) {\n \t\tconst char *arg = ctx->argv[0];\n \n+\t\tif (ctx->flags & PARSE_OPT_ONE_SHOT &&\n+\t\t    ctx->argc != ctx->total)\n+\t\t\tbreak;\n+\n \t\tif (*arg != '-' || !arg[1]) {\n \t\t\tif (parse_nodash_opt(ctx, arg, options) == 0)\n \t\t\t\tcontinue;\n@@ -610,6 +623,8 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,\n \t\t}\n \t\tcontinue;\n unknown:\n+\t\tif (ctx->flags & PARSE_OPT_ONE_SHOT)\n+\t\t\tbreak;\n \t\tif (!(ctx->flags & PARSE_OPT_KEEP_UNKNOWN))\n \t\t\treturn PARSE_OPT_UNKNOWN;\n \t\tctx->out[ctx->cpidx++] = ctx->argv[0];\n@@ -623,6 +638,9 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,\n \n int parse_options_end(struct parse_opt_ctx_t *ctx)\n {\n+\tif (ctx->flags & PARSE_OPT_ONE_SHOT)\n+\t\treturn ctx->total - ctx->argc;\n+\n \tMOVE_ARRAY(ctx->out + ctx->cpidx, ctx->argv, ctx->argc);\n \tctx->out[ctx->cpidx + ctx->argc] = NULL;\n \treturn ctx->cpidx + ctx->argc;\ndiff --git a/parse-options.h b/parse-options.h\nindex f5e7ec7d23..d663b83973 100644\n--- a/parse-options.h\n+++ b/parse-options.h\n@@ -27,7 +27,8 @@ enum parse_opt_flags {\n \tPARSE_OPT_STOP_AT_NON_OPTION = 2,\n \tPARSE_OPT_KEEP_ARGV0 = 4,\n \tPARSE_OPT_KEEP_UNKNOWN = 8,\n-\tPARSE_OPT_NO_INTERNAL_HELP = 16\n+\tPARSE_OPT_NO_INTERNAL_HELP = 16,\n+\tPARSE_OPT_ONE_SHOT = 32\n };\n \n enum parse_opt_option_flags {\n@@ -169,10 +170,18 @@ struct option {\n \t  N_(\"no-op (backward compatibility)\"),\t\t\\\n \t  PARSE_OPT_HIDDEN | PARSE_OPT_NOARG, parse_opt_noop_cb }\n \n-/* parse_options() will filter out the processed options and leave the\n- * non-option arguments in argv[]. usagestr strings should be marked\n- * for translation with N_().\n+/*\n+ * parse_options() will filter out the processed options and leave the\n+ * non-option arguments in argv[]. argv0 is assumed program name and\n+ * skipped.\n+ *\n+ * usagestr strings should be marked for translation with N_().\n+ *\n  * Returns the number of arguments left in argv[].\n+ *\n+ * In one-shot mode, argv0 is not a program name, argv[] is left\n+ * untouched and parse_options() returns the number of options\n+ * processed.\n  */\n int parse_options(int argc, const char **argv, const char *prefix,\n \t\t  const struct option *options,\n-- \n2.20.1.560.g70ca8b83ee\n\n"},{"id":"367732","messageId":"20190127003535.28341-4-pclouds@gmail.com","threadId":"50334","inReplyTo":"20190127003535.28341-1-pclouds@gmail.com","subject":"[PATCH 03/14] parse-options: disable option abbreviation with PARSE_OPT_KEEP_UNKNOWN","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2019-01-27T00:35:24Z","receivedAt":"2019-01-27T00:36:20Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"parse-options can unambiguously find an abbreviation only if it sees\nall available options. This is usually the case when you use\nparse_options(). But there are other callers like blame or shortlog\nwhich uses parse_options_start() in combination with a custom option\nparser, like rev-list. parse-options cannot see all options in this\ncase and will get abbrev detection wrong. Disable it.\n\nt7800 needs update because --symlink no longer expands to --symlinks\nand will be passed down to git-diff, which will not recognize it. I\nstill think this is the correct thing to do. But if --symlink has been\nactually used in the wild, we would just add an option alias for it.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n parse-options.c     | 3 ++-\n t/t7800-difftool.sh | 4 ++--\n 2 files changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/parse-options.c b/parse-options.c\nindex 740ae5438f..779034e1fd 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -266,7 +266,8 @@ static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg,\n \t\t}\n \t\tif (!rest) {\n \t\t\t/* abbreviated? */\n-\t\t\tif (!strncmp(long_name, arg, arg_end - arg)) {\n+\t\t\tif (!(p->flags & PARSE_OPT_KEEP_UNKNOWN) &&\n+\t\t\t    !strncmp(long_name, arg, arg_end - arg)) {\n is_abbreviated:\n \t\t\t\tif (abbrev_option) {\n \t\t\t\t\t/*\ndiff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh\nindex 22b9199d59..bb9a7f4ff9 100755\n--- a/t/t7800-difftool.sh\n+++ b/t/t7800-difftool.sh\n@@ -546,7 +546,7 @@ do\n done >actual\n EOF\n \n-test_expect_success SYMLINKS 'difftool --dir-diff --symlink without unstaged changes' '\n+test_expect_success SYMLINKS 'difftool --dir-diff --symlinks without unstaged changes' '\n \tcat >expect <<-EOF &&\n \tfile\n \t$PWD/file\n@@ -555,7 +555,7 @@ test_expect_success SYMLINKS 'difftool --dir-diff --symlink without unstaged cha\n \tsub/sub\n \t$PWD/sub/sub\n \tEOF\n-\tgit difftool --dir-diff --symlink \\\n+\tgit difftool --dir-diff --symlinks \\\n \t\t--extcmd \"./.git/CHECK_SYMLINKS\" branch HEAD &&\n \ttest_cmp expect actual\n '\n-- \n2.20.1.560.g70ca8b83ee\n\n"},{"id":"367733","messageId":"20190127003535.28341-5-pclouds@gmail.com","threadId":"50334","inReplyTo":"20190127003535.28341-1-pclouds@gmail.com","subject":"[PATCH 04/14] parse-options: add OPT_BITOP()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2019-01-27T00:35:25Z","receivedAt":"2019-01-27T00:36:25Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This is needed for diff_opt_parse() where we do\n\n   value = (value & ~mask) | some_more;\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n parse-options.c | 7 +++++++\n parse-options.h | 5 +++++\n 2 files changed, 12 insertions(+)\n\ndiff --git a/parse-options.c b/parse-options.c\nindex 779034e1fd..62d94ca2e0 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -109,6 +109,13 @@ static int get_value(struct parse_opt_ctx_t *p,\n \t\t\t*(int *)opt->value &= ~opt->defval;\n \t\treturn 0;\n \n+\tcase OPTION_BITOP:\n+\t\tif (unset)\n+\t\t\tBUG(\"BITOP can't have unset form\");\n+\t\t*(int *)opt->value &= ~opt->extra;\n+\t\t*(int *)opt->value |= opt->defval;\n+\t\treturn 0;\n+\n \tcase OPTION_COUNTUP:\n \t\tif (*(int *)opt->value < 0)\n \t\t\t*(int *)opt->value = 0;\ndiff --git a/parse-options.h b/parse-options.h\nindex d663b83973..c97324f576 100644\n--- a/parse-options.h\n+++ b/parse-options.h\n@@ -10,6 +10,7 @@ enum parse_opt_type {\n \t/* options with no arguments */\n \tOPTION_BIT,\n \tOPTION_NEGBIT,\n+\tOPTION_BITOP,\n \tOPTION_COUNTUP,\n \tOPTION_SET_INT,\n \tOPTION_CMDMODE,\n@@ -118,6 +119,7 @@ struct option {\n \tint flags;\n \tparse_opt_cb *callback;\n \tintptr_t defval;\n+\tintptr_t extra;\n };\n \n #define OPT_BIT_F(s, l, v, h, b, f) { OPTION_BIT, (s), (l), (v), NULL, (h), \\\n@@ -133,6 +135,9 @@ struct option {\n \t\t\t\t      (h), PARSE_OPT_NOARG}\n #define OPT_GROUP(h)                { OPTION_GROUP, 0, NULL, NULL, NULL, (h) }\n #define OPT_BIT(s, l, v, h, b)      OPT_BIT_F(s, l, v, h, b, 0)\n+#define OPT_BITOP(s, l, v, h, set, clear) { OPTION_BITOP, (s), (l), (v), NULL, (h), \\\n+\t\t\t\t\t    PARSE_OPT_NOARG|PARSE_OPT_NONEG, NULL, \\\n+\t\t\t\t\t    (set), (clear) }\n #define OPT_NEGBIT(s, l, v, h, b)   { OPTION_NEGBIT, (s), (l), (v), NULL, \\\n \t\t\t\t      (h), PARSE_OPT_NOARG, NULL, (b) }\n #define OPT_COUNTUP(s, l, v, h)     OPT_COUNTUP_F(s, l, v, h, 0)\n-- \n2.20.1.560.g70ca8b83ee\n\n"},{"id":"367734","messageId":"20190127003535.28341-6-pclouds@gmail.com","threadId":"50334","inReplyTo":"20190127003535.28341-1-pclouds@gmail.com","subject":"[PATCH 05/14] parse-options: stop abusing 'callback' for lowlevel callbacks","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2019-01-27T00:35:26Z","receivedAt":"2019-01-27T00:36:30Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Lowlevel callbacks have different function signatures. Add a new field\nin 'struct option' with the right type for lowlevel callbacks.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/blame.c        |  2 +-\n builtin/merge.c        |  2 +-\n builtin/update-index.c | 11 ++++++-----\n parse-options-cb.c     |  3 ++-\n parse-options.c        | 15 ++++++++++++++-\n parse-options.h        | 12 ++++++++----\n 6 files changed, 32 insertions(+), 13 deletions(-)\n\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex 6d798f9939..8dcc55dffa 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -814,7 +814,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n \t\t * and are only included here to get included in the \"-h\"\n \t\t * output:\n \t\t */\n-\t\t{ OPTION_LOWLEVEL_CALLBACK, 0, \"indent-heuristic\", NULL, NULL, N_(\"Use an experimental heuristic to improve diffs\"), PARSE_OPT_NOARG, parse_opt_unknown_cb },\n+\t\t{ OPTION_LOWLEVEL_CALLBACK, 0, \"indent-heuristic\", NULL, NULL, N_(\"Use an experimental heuristic to improve diffs\"), PARSE_OPT_NOARG, NULL, 0, parse_opt_unknown_cb },\n \n \t\tOPT_BIT(0, \"minimal\", &xdl_opts, N_(\"Spend extra cycles to find better match\"), XDF_NEED_MINIMAL),\n \t\tOPT_STRING('S', NULL, &revs_file, N_(\"file\"), N_(\"Use revisions from <file> instead of calling git-rev-list\")),\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex dc0b7cc521..07839b0bb8 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -261,7 +261,7 @@ static struct option builtin_merge_options[] = {\n \t\toption_parse_message),\n \t{ OPTION_LOWLEVEL_CALLBACK, 'F', \"file\", &merge_msg, N_(\"path\"),\n \t\tN_(\"read message from file\"), PARSE_OPT_NONEG,\n-\t\t(parse_opt_cb *) option_read_message },\n+\t\tNULL, 0, option_read_message },\n \tOPT__VERBOSITY(&verbosity),\n \tOPT_BOOL(0, \"abort\", &abort_current_merge,\n \t\tN_(\"abort the current in-progress merge\")),\ndiff --git a/builtin/update-index.c b/builtin/update-index.c\nindex e19da77edc..727a8118b8 100644\n--- a/builtin/update-index.c\n+++ b/builtin/update-index.c\n@@ -985,7 +985,8 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n \t\t\tN_(\"add the specified entry to the index\"),\n \t\t\tPARSE_OPT_NOARG | /* disallow --cacheinfo=<mode> form */\n \t\t\tPARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP,\n-\t\t\t(parse_opt_cb *) cacheinfo_callback},\n+\t\t\tNULL, 0,\n+\t\t\tcacheinfo_callback},\n \t\t{OPTION_CALLBACK, 0, \"chmod\", &set_executable_bit, \"(+|-)x\",\n \t\t\tN_(\"override the executable bit of the listed files\"),\n \t\t\tPARSE_OPT_NONEG,\n@@ -1011,19 +1012,19 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)\n \t\t{OPTION_LOWLEVEL_CALLBACK, 0, \"stdin\", &read_from_stdin, NULL,\n \t\t\tN_(\"read list of paths to be updated from standard input\"),\n \t\t\tPARSE_OPT_NONEG | PARSE_OPT_NOARG,\n-\t\t\t(parse_opt_cb *) stdin_callback},\n+\t\t\tNULL, 0, stdin_callback},\n \t\t{OPTION_LOWLEVEL_CALLBACK, 0, \"index-info\", &nul_term_line, NULL,\n \t\t\tN_(\"add entries from standard input to the index\"),\n \t\t\tPARSE_OPT_NONEG | PARSE_OPT_NOARG,\n-\t\t\t(parse_opt_cb *) stdin_cacheinfo_callback},\n+\t\t\tNULL, 0, stdin_cacheinfo_callback},\n \t\t{OPTION_LOWLEVEL_CALLBACK, 0, \"unresolve\", &has_errors, NULL,\n \t\t\tN_(\"repopulate stages #2 and #3 for the listed paths\"),\n \t\t\tPARSE_OPT_NONEG | PARSE_OPT_NOARG,\n-\t\t\t(parse_opt_cb *) unresolve_callback},\n+\t\t\tNULL, 0, unresolve_callback},\n \t\t{OPTION_LOWLEVEL_CALLBACK, 'g', \"again\", &has_errors, NULL,\n \t\t\tN_(\"only update entries that differ from HEAD\"),\n \t\t\tPARSE_OPT_NONEG | PARSE_OPT_NOARG,\n-\t\t\t(parse_opt_cb *) reupdate_callback},\n+\t\t\tNULL, 0, reupdate_callback},\n \t\tOPT_BIT(0, \"ignore-missing\", &refresh_args.flags,\n \t\t\tN_(\"ignore files missing from worktree\"),\n \t\t\tREFRESH_IGNORE_MISSING),\ndiff --git a/parse-options-cb.c b/parse-options-cb.c\nindex e2f3eaed07..e05bcea809 100644\n--- a/parse-options-cb.c\n+++ b/parse-options-cb.c\n@@ -170,7 +170,8 @@ int parse_opt_noop_cb(const struct option *opt, const char *arg, int unset)\n  * \"-h\" output even if it's not being handled directly by\n  * parse_options().\n  */\n-int parse_opt_unknown_cb(const struct option *opt, const char *arg, int unset)\n+int parse_opt_unknown_cb(struct parse_opt_ctx_t *ctx,\n+\t\t\t const struct option *opt, int unset)\n {\n \treturn -2;\n }\ndiff --git a/parse-options.c b/parse-options.c\nindex 62d94ca2e0..37a56d079a 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -93,7 +93,7 @@ static int get_value(struct parse_opt_ctx_t *p,\n \n \tswitch (opt->type) {\n \tcase OPTION_LOWLEVEL_CALLBACK:\n-\t\treturn (*(parse_opt_ll_cb *)opt->callback)(p, opt, unset);\n+\t\treturn opt->ll_callback(p, opt, unset);\n \n \tcase OPTION_BIT:\n \t\tif (unset)\n@@ -408,6 +408,19 @@ static void parse_options_check(const struct option *opts)\n \t\t\tif ((opts->flags & PARSE_OPT_OPTARG) ||\n \t\t\t    !(opts->flags & PARSE_OPT_NOARG))\n \t\t\t\terr |= optbug(opts, \"should not accept an argument\");\n+\t\t\tbreak;\n+\t\tcase OPTION_CALLBACK:\n+\t\t\tif (!opts->callback)\n+\t\t\t\tBUG(\"OPTION_CALLBACK needs a callback\");\n+\t\t\tif (opts->ll_callback)\n+\t\t\t\tBUG(\"OPTION_CALLBACK needs no ll_callback\");\n+\t\t\tbreak;\n+\t\tcase OPTION_LOWLEVEL_CALLBACK:\n+\t\t\tif (!opts->ll_callback)\n+\t\t\t\tBUG(\"OPTION_LOWLEVEL_CALLBACK needs a callback\");\n+\t\t\tif (opts->callback)\n+\t\t\t\tBUG(\"OPTION_LOWLEVEL_CALLBACK needs no high level callback\");\n+\t\t\tbreak;\n \t\tdefault:\n \t\t\t; /* ok. (usually accepts an argument) */\n \t\t}\ndiff --git a/parse-options.h b/parse-options.h\nindex c97324f576..f1f246387c 100644\n--- a/parse-options.h\n+++ b/parse-options.h\n@@ -100,13 +100,16 @@ typedef int parse_opt_ll_cb(struct parse_opt_ctx_t *ctx,\n  *\t\t\t the option takes optional argument.\n  *\n  * `callback`::\n- *   pointer to the callback to use for OPTION_CALLBACK or\n- *   OPTION_LOWLEVEL_CALLBACK.\n+ *   pointer to the callback to use for OPTION_CALLBACK\n  *\n  * `defval`::\n  *   default value to fill (*->value) with for PARSE_OPT_OPTARG.\n  *   OPTION_{BIT,SET_INT} store the {mask,integer} to put in the value when met.\n  *   CALLBACKS can use it like they want.\n+ *\n+ * `ll_callback`::\n+ *   pointer to the callback to use for OPTION_LOWLEVEL_CALLBACK\n+ *\n  */\n struct option {\n \tenum parse_opt_type type;\n@@ -119,6 +122,7 @@ struct option {\n \tint flags;\n \tparse_opt_cb *callback;\n \tintptr_t defval;\n+\tparse_opt_ll_cb *ll_callback;\n \tintptr_t extra;\n };\n \n@@ -137,7 +141,7 @@ struct option {\n #define OPT_BIT(s, l, v, h, b)      OPT_BIT_F(s, l, v, h, b, 0)\n #define OPT_BITOP(s, l, v, h, set, clear) { OPTION_BITOP, (s), (l), (v), NULL, (h), \\\n \t\t\t\t\t    PARSE_OPT_NOARG|PARSE_OPT_NONEG, NULL, \\\n-\t\t\t\t\t    (set), (clear) }\n+\t\t\t\t\t    (set), NULL, (clear) }\n #define OPT_NEGBIT(s, l, v, h, b)   { OPTION_NEGBIT, (s), (l), (v), NULL, \\\n \t\t\t\t      (h), PARSE_OPT_NOARG, NULL, (b) }\n #define OPT_COUNTUP(s, l, v, h)     OPT_COUNTUP_F(s, l, v, h, 0)\n@@ -263,7 +267,7 @@ int parse_opt_commits(const struct option *, const char *, int);\n int parse_opt_tertiary(const struct option *, const char *, int);\n int parse_opt_string_list(const struct option *, const char *, int);\n int parse_opt_noop_cb(const struct option *, const char *, int);\n-int parse_opt_unknown_cb(const struct option *, const char *, int);\n+int parse_opt_unknown_cb(struct parse_opt_ctx_t *ctx, const struct option *, int);\n int parse_opt_passthru(const struct option *, const char *, int);\n int parse_opt_passthru_argv(const struct option *, const char *, int);\n \n-- \n2.20.1.560.g70ca8b83ee\n\n"},{"id":"367735","messageId":"20190127003535.28341-7-pclouds@gmail.com","threadId":"50334","inReplyTo":"20190127003535.28341-1-pclouds@gmail.com","subject":"[PATCH 06/14] parse-options: avoid magic return codes","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2019-01-27T00:35:27Z","receivedAt":"2019-01-27T00:36:37Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Give names to these magic negative numbers. Make parse_opt_ll_cb\nreturn an enum to make clear it can actually control parse_options()\nwith different return values (parse_opt_cb can too, but nobody needs\nit).\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/merge.c        |  5 +--\n builtin/update-index.c | 20 ++++++------\n parse-options-cb.c     |  6 ++--\n parse-options.c        | 69 +++++++++++++++++++++++++++---------------\n parse-options.h        | 14 ++++-----\n 5 files changed, 68 insertions(+), 46 deletions(-)\n\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 07839b0bb8..de64d7850e 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -112,8 +112,9 @@ static int option_parse_message(const struct option *opt,\n \treturn 0;\n }\n \n-static int option_read_message(struct parse_opt_ctx_t *ctx,\n-\t\t\t       const struct option *opt, int unset)\n+static enum parse_opt_result option_read_message(struct parse_opt_ctx_t *ctx,\n+\t\t\t\t\t\t const struct option *opt,\n+\t\t\t\t\t\t int unset)\n {\n \tstruct strbuf *buf = opt->value;\n \tconst char *arg;\ndiff --git a/builtin/update-index.c b/builtin/update-index.c\nindex 727a8118b8..21c84e5590 100644\n--- a/builtin/update-index.c\n+++ b/builtin/update-index.c\n@@ -847,8 +847,8 @@ static int parse_new_style_cacheinfo(const char *arg,\n \treturn 0;\n }\n \n-static int cacheinfo_callback(struct parse_opt_ctx_t *ctx,\n-\t\t\t\tconst struct option *opt, int unset)\n+static enum parse_opt_result cacheinfo_callback(\n+\tstruct parse_opt_ctx_t *ctx, const struct option *opt, int unset)\n {\n \tstruct object_id oid;\n \tunsigned int mode;\n@@ -873,8 +873,8 @@ static int cacheinfo_callback(struct parse_opt_ctx_t *ctx,\n \treturn 0;\n }\n \n-static int stdin_cacheinfo_callback(struct parse_opt_ctx_t *ctx,\n-\t\t\t      const struct option *opt, int unset)\n+static enum parse_opt_result stdin_cacheinfo_callback(\n+\tstruct parse_opt_ctx_t *ctx, const struct option *opt, int unset)\n {\n \tint *nul_term_line = opt->value;\n \n@@ -887,8 +887,8 @@ static int stdin_cacheinfo_callback(struct parse_opt_ctx_t *ctx,\n \treturn 0;\n }\n \n-static int stdin_callback(struct parse_opt_ctx_t *ctx,\n-\t\t\t\tconst struct option *opt, int unset)\n+static enum parse_opt_result stdin_callback(\n+\tstruct parse_opt_ctx_t *ctx, const struct option *opt, int unset)\n {\n \tint *read_from_stdin = opt->value;\n \n@@ -900,8 +900,8 @@ static int stdin_callback(struct parse_opt_ctx_t *ctx,\n \treturn 0;\n }\n \n-static int unresolve_callback(struct parse_opt_ctx_t *ctx,\n-\t\t\t\tconst struct option *opt, int unset)\n+static enum parse_opt_result unresolve_callback(\n+\tstruct parse_opt_ctx_t *ctx, const struct option *opt, int unset)\n {\n \tint *has_errors = opt->value;\n \tconst char *prefix = startup_info->prefix;\n@@ -919,8 +919,8 @@ static int unresolve_callback(struct parse_opt_ctx_t *ctx,\n \treturn 0;\n }\n \n-static int reupdate_callback(struct parse_opt_ctx_t *ctx,\n-\t\t\t\tconst struct option *opt, int unset)\n+static enum parse_opt_result reupdate_callback(\n+\tstruct parse_opt_ctx_t *ctx, const struct option *opt, int unset)\n {\n \tint *has_errors = opt->value;\n \tconst char *prefix = startup_info->prefix;\ndiff --git a/parse-options-cb.c b/parse-options-cb.c\nindex e05bcea809..ec01ef722b 100644\n--- a/parse-options-cb.c\n+++ b/parse-options-cb.c\n@@ -170,10 +170,10 @@ int parse_opt_noop_cb(const struct option *opt, const char *arg, int unset)\n  * \"-h\" output even if it's not being handled directly by\n  * parse_options().\n  */\n-int parse_opt_unknown_cb(struct parse_opt_ctx_t *ctx,\n-\t\t\t const struct option *opt, int unset)\n+enum parse_opt_result parse_opt_unknown_cb(struct parse_opt_ctx_t *ctx,\n+\t\t\t\t\t   const struct option *opt, int unset)\n {\n-\treturn -2;\n+\treturn PARSE_OPT_UNKNOWN;\n }\n \n /**\ndiff --git a/parse-options.c b/parse-options.c\nindex 37a56d079a..50c340474c 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -20,8 +20,9 @@ int optbug(const struct option *opt, const char *reason)\n \treturn error(\"BUG: switch '%c' %s\", opt->short_name, reason);\n }\n \n-static int get_arg(struct parse_opt_ctx_t *p, const struct option *opt,\n-\t\t   int flags, const char **arg)\n+static enum parse_opt_result get_arg(struct parse_opt_ctx_t *p,\n+\t\t\t\t     const struct option *opt,\n+\t\t\t\t     int flags, const char **arg)\n {\n \tif (p->opt) {\n \t\t*arg = p->opt;\n@@ -44,9 +45,10 @@ static void fix_filename(const char *prefix, const char **file)\n \t*file = prefix_filename(prefix, *file);\n }\n \n-static int opt_command_mode_error(const struct option *opt,\n-\t\t\t\t  const struct option *all_opts,\n-\t\t\t\t  int flags)\n+static enum parse_opt_result opt_command_mode_error(\n+\tconst struct option *opt,\n+\tconst struct option *all_opts,\n+\tint flags)\n {\n \tconst struct option *that;\n \tstruct strbuf that_name = STRBUF_INIT;\n@@ -69,16 +71,16 @@ static int opt_command_mode_error(const struct option *opt,\n \t\terror(_(\"%s is incompatible with %s\"),\n \t\t      optname(opt, flags), that_name.buf);\n \t\tstrbuf_release(&that_name);\n-\t\treturn -1;\n+\t\treturn PARSE_OPT_ERROR;\n \t}\n \treturn error(_(\"%s : incompatible with something else\"),\n \t\t     optname(opt, flags));\n }\n \n-static int get_value(struct parse_opt_ctx_t *p,\n-\t\t     const struct option *opt,\n-\t\t     const struct option *all_opts,\n-\t\t     int flags)\n+static enum parse_opt_result get_value(struct parse_opt_ctx_t *p,\n+\t\t\t\t       const struct option *opt,\n+\t\t\t\t       const struct option *all_opts,\n+\t\t\t\t       int flags)\n {\n \tconst char *s, *arg;\n \tconst int unset = flags & OPT_UNSET;\n@@ -208,7 +210,8 @@ static int get_value(struct parse_opt_ctx_t *p,\n \t}\n }\n \n-static int parse_short_opt(struct parse_opt_ctx_t *p, const struct option *options)\n+static enum parse_opt_result parse_short_opt(struct parse_opt_ctx_t *p,\n+\t\t\t\t\t     const struct option *options)\n {\n \tconst struct option *all_opts = options;\n \tconst struct option *numopt = NULL;\n@@ -239,11 +242,12 @@ static int parse_short_opt(struct parse_opt_ctx_t *p, const struct option *optio\n \t\tfree(arg);\n \t\treturn rc;\n \t}\n-\treturn -2;\n+\treturn PARSE_OPT_UNKNOWN;\n }\n \n-static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg,\n-\t\t\t  const struct option *options)\n+static enum parse_opt_result parse_long_opt(\n+\tstruct parse_opt_ctx_t *p, const char *arg,\n+\tconst struct option *options)\n {\n \tconst struct option *all_opts = options;\n \tconst char *arg_end = strchrnul(arg, '=');\n@@ -269,7 +273,7 @@ static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg,\n \t\t\tif (*rest)\n \t\t\t\tcontinue;\n \t\t\tp->out[p->cpidx++] = arg - 2;\n-\t\t\treturn 0;\n+\t\t\treturn PARSE_OPT_DONE;\n \t\t}\n \t\tif (!rest) {\n \t\t\t/* abbreviated? */\n@@ -334,11 +338,11 @@ static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg,\n \t\t\tambiguous_option->long_name,\n \t\t\t(abbrev_flags & OPT_UNSET) ?  \"no-\" : \"\",\n \t\t\tabbrev_option->long_name);\n-\t\treturn -3;\n+\t\treturn PARSE_OPT_HELP;\n \t}\n \tif (abbrev_option)\n \t\treturn get_value(p, abbrev_option, all_opts, abbrev_flags);\n-\treturn -2;\n+\treturn PARSE_OPT_UNKNOWN;\n }\n \n static int parse_nodash_opt(struct parse_opt_ctx_t *p, const char *arg,\n@@ -590,22 +594,28 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,\n \t\tif (arg[1] != '-') {\n \t\t\tctx->opt = arg + 1;\n \t\t\tswitch (parse_short_opt(ctx, options)) {\n-\t\t\tcase -1:\n+\t\t\tcase PARSE_OPT_ERROR:\n \t\t\t\treturn PARSE_OPT_ERROR;\n-\t\t\tcase -2:\n+\t\t\tcase PARSE_OPT_UNKNOWN:\n \t\t\t\tif (ctx->opt)\n \t\t\t\t\tcheck_typos(arg + 1, options);\n \t\t\t\tif (internal_help && *ctx->opt == 'h')\n \t\t\t\t\tgoto show_usage;\n \t\t\t\tgoto unknown;\n+\t\t\tcase PARSE_OPT_NON_OPTION:\n+\t\t\tcase PARSE_OPT_HELP:\n+\t\t\tcase PARSE_OPT_COMPLETE:\n+\t\t\t\tBUG(\"parse_short_opt() cannot return these\");\n+\t\t\tcase PARSE_OPT_DONE:\n+\t\t\t\tbreak;\n \t\t\t}\n \t\t\tif (ctx->opt)\n \t\t\t\tcheck_typos(arg + 1, options);\n \t\t\twhile (ctx->opt) {\n \t\t\t\tswitch (parse_short_opt(ctx, options)) {\n-\t\t\t\tcase -1:\n+\t\t\t\tcase PARSE_OPT_ERROR:\n \t\t\t\t\treturn PARSE_OPT_ERROR;\n-\t\t\t\tcase -2:\n+\t\t\t\tcase PARSE_OPT_UNKNOWN:\n \t\t\t\t\tif (internal_help && *ctx->opt == 'h')\n \t\t\t\t\t\tgoto show_usage;\n \n@@ -617,6 +627,12 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,\n \t\t\t\t\tctx->argv[0] = xstrdup(ctx->opt - 1);\n \t\t\t\t\t*(char *)ctx->argv[0] = '-';\n \t\t\t\t\tgoto unknown;\n+\t\t\t\tcase PARSE_OPT_NON_OPTION:\n+\t\t\t\tcase PARSE_OPT_COMPLETE:\n+\t\t\t\tcase PARSE_OPT_HELP:\n+\t\t\t\t\tBUG(\"parse_short_opt() cannot return these\");\n+\t\t\t\tcase PARSE_OPT_DONE:\n+\t\t\t\t\tbreak;\n \t\t\t\t}\n \t\t\t}\n \t\t\tcontinue;\n@@ -635,12 +651,17 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,\n \t\tif (internal_help && !strcmp(arg + 2, \"help\"))\n \t\t\tgoto show_usage;\n \t\tswitch (parse_long_opt(ctx, arg + 2, options)) {\n-\t\tcase -1:\n+\t\tcase PARSE_OPT_ERROR:\n \t\t\treturn PARSE_OPT_ERROR;\n-\t\tcase -2:\n+\t\tcase PARSE_OPT_UNKNOWN:\n \t\t\tgoto unknown;\n-\t\tcase -3:\n+\t\tcase PARSE_OPT_HELP:\n \t\t\tgoto show_usage;\n+\t\tcase PARSE_OPT_NON_OPTION:\n+\t\tcase PARSE_OPT_COMPLETE:\n+\t\t\tBUG(\"parse_long_opt() cannot return these\");\n+\t\tcase PARSE_OPT_DONE:\n+\t\t\tbreak;\n \t\t}\n \t\tcontinue;\n unknown:\ndiff --git a/parse-options.h b/parse-options.h\nindex f1f246387c..4e49185027 100644\n--- a/parse-options.h\n+++ b/parse-options.h\n@@ -49,8 +49,8 @@ struct option;\n typedef int parse_opt_cb(const struct option *, const char *arg, int unset);\n \n struct parse_opt_ctx_t;\n-typedef int parse_opt_ll_cb(struct parse_opt_ctx_t *ctx,\n-\t\t\t\tconst struct option *opt, int unset);\n+typedef enum parse_opt_result parse_opt_ll_cb(struct parse_opt_ctx_t *ctx,\n+\t\t\t\t\t      const struct option *opt, int unset);\n \n /*\n  * `type`::\n@@ -222,12 +222,12 @@ const char *optname(const struct option *opt, int flags);\n \n /*----- incremental advanced APIs -----*/\n \n-enum {\n-\tPARSE_OPT_COMPLETE = -2,\n-\tPARSE_OPT_HELP = -1,\n-\tPARSE_OPT_DONE,\n+enum parse_opt_result {\n+\tPARSE_OPT_COMPLETE = -3,\n+\tPARSE_OPT_HELP = -2,\n+\tPARSE_OPT_ERROR = -1,\t/* must be the same as error() */\n+\tPARSE_OPT_DONE = 0,\t/* fixed so that \"return 0\" works */\n \tPARSE_OPT_NON_OPTION,\n-\tPARSE_OPT_ERROR,\n \tPARSE_OPT_UNKNOWN\n };\n \n-- \n2.20.1.560.g70ca8b83ee\n\n"},{"id":"367736","messageId":"20190127003535.28341-8-pclouds@gmail.com","threadId":"50334","inReplyTo":"20190127003535.28341-1-pclouds@gmail.com","subject":"[PATCH 07/14] parse-options: allow ll_callback with OPTION_CALLBACK","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2019-01-27T00:35:28Z","receivedAt":"2019-01-27T00:36:42Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"OPTION_CALLBACK is much simpler/safer to use, but parse_opt_cb does\nnot allow access to parse_opt_ctx_t, which sometimes is useful\n(e.g. to obtain the prefix).\n\nExtending parse_opt_cb to take parse_opt_cb could result in a lot of\nchanges. Instead let's just allow ll_callback to be used with\nOPTION_CALLBACK. The user will have to be careful, not to change\nanything in ctx, or return wrong result code. But that's the price for\nll_callback.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/merge.c        |  2 ++\n builtin/update-index.c | 20 +++++++++++++++-----\n parse-options-cb.c     |  4 +++-\n parse-options.c        | 42 ++++++++++++++++++++++++++++--------------\n parse-options.h        |  5 +++--\n 5 files changed, 51 insertions(+), 22 deletions(-)\n\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex de64d7850e..563a16f38a 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -114,11 +114,13 @@ static int option_parse_message(const struct option *opt,\n \n static enum parse_opt_result option_read_message(struct parse_opt_ctx_t *ctx,\n \t\t\t\t\t\t const struct option *opt,\n+\t\t\t\t\t\t const char *arg_not_used,\n \t\t\t\t\t\t int unset)\n {\n \tstruct strbuf *buf = opt->value;\n \tconst char *arg;\n \n+\tBUG_ON_OPT_ARG(arg_not_used);\n \tif (unset)\n \t\tBUG(\"-F cannot be negated\");\n \ndiff --git a/builtin/update-index.c b/builtin/update-index.c\nindex 21c84e5590..7abde20169 100644\n--- a/builtin/update-index.c\n+++ b/builtin/update-index.c\n@@ -848,13 +848,15 @@ static int parse_new_style_cacheinfo(const char *arg,\n }\n \n static enum parse_opt_result cacheinfo_callback(\n-\tstruct parse_opt_ctx_t *ctx, const struct option *opt, int unset)\n+\tstruct parse_opt_ctx_t *ctx, const struct option *opt,\n+\tconst char *arg, int unset)\n {\n \tstruct object_id oid;\n \tunsigned int mode;\n \tconst char *path;\n \n \tBUG_ON_OPT_NEG(unset);\n+\tBUG_ON_OPT_ARG(arg);\n \n \tif (!parse_new_style_cacheinfo(ctx->argv[1], &mode, &oid, &path)) {\n \t\tif (add_cacheinfo(mode, &oid, path, 0))\n@@ -874,11 +876,13 @@ static enum parse_opt_result cacheinfo_callback(\n }\n \n static enum parse_opt_result stdin_cacheinfo_callback(\n-\tstruct parse_opt_ctx_t *ctx, const struct option *opt, int unset)\n+\tstruct parse_opt_ctx_t *ctx, const struct option *opt,\n+\tconst char *arg, int unset)\n {\n \tint *nul_term_line = opt->value;\n \n \tBUG_ON_OPT_NEG(unset);\n+\tBUG_ON_OPT_ARG(arg);\n \n \tif (ctx->argc != 1)\n \t\treturn error(\"option '%s' must be the last argument\", opt->long_name);\n@@ -888,11 +892,13 @@ static enum parse_opt_result stdin_cacheinfo_callback(\n }\n \n static enum parse_opt_result stdin_callback(\n-\tstruct parse_opt_ctx_t *ctx, const struct option *opt, int unset)\n+\tstruct parse_opt_ctx_t *ctx, const struct option *opt,\n+\tconst char *arg, int unset)\n {\n \tint *read_from_stdin = opt->value;\n \n \tBUG_ON_OPT_NEG(unset);\n+\tBUG_ON_OPT_ARG(arg);\n \n \tif (ctx->argc != 1)\n \t\treturn error(\"option '%s' must be the last argument\", opt->long_name);\n@@ -901,12 +907,14 @@ static enum parse_opt_result stdin_callback(\n }\n \n static enum parse_opt_result unresolve_callback(\n-\tstruct parse_opt_ctx_t *ctx, const struct option *opt, int unset)\n+\tstruct parse_opt_ctx_t *ctx, const struct option *opt,\n+\tconst char *arg, int unset)\n {\n \tint *has_errors = opt->value;\n \tconst char *prefix = startup_info->prefix;\n \n \tBUG_ON_OPT_NEG(unset);\n+\tBUG_ON_OPT_ARG(arg);\n \n \t/* consume remaining arguments. */\n \t*has_errors = do_unresolve(ctx->argc, ctx->argv,\n@@ -920,12 +928,14 @@ static enum parse_opt_result unresolve_callback(\n }\n \n static enum parse_opt_result reupdate_callback(\n-\tstruct parse_opt_ctx_t *ctx, const struct option *opt, int unset)\n+\tstruct parse_opt_ctx_t *ctx, const struct option *opt,\n+\tconst char *arg, int unset)\n {\n \tint *has_errors = opt->value;\n \tconst char *prefix = startup_info->prefix;\n \n \tBUG_ON_OPT_NEG(unset);\n+\tBUG_ON_OPT_ARG(arg);\n \n \t/* consume remaining arguments. */\n \tsetup_work_tree();\ndiff --git a/parse-options-cb.c b/parse-options-cb.c\nindex ec01ef722b..2733393546 100644\n--- a/parse-options-cb.c\n+++ b/parse-options-cb.c\n@@ -171,8 +171,10 @@ int parse_opt_noop_cb(const struct option *opt, const char *arg, int unset)\n  * parse_options().\n  */\n enum parse_opt_result parse_opt_unknown_cb(struct parse_opt_ctx_t *ctx,\n-\t\t\t\t\t   const struct option *opt, int unset)\n+\t\t\t\t\t   const struct option *opt,\n+\t\t\t\t\t   const char *arg, int unset)\n {\n+\tBUG_ON_OPT_ARG(arg);\n \treturn PARSE_OPT_UNKNOWN;\n }\n \ndiff --git a/parse-options.c b/parse-options.c\nindex 50c340474c..cec74522e5 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -95,7 +95,7 @@ static enum parse_opt_result get_value(struct parse_opt_ctx_t *p,\n \n \tswitch (opt->type) {\n \tcase OPTION_LOWLEVEL_CALLBACK:\n-\t\treturn opt->ll_callback(p, opt, unset);\n+\t\treturn opt->ll_callback(p, opt, NULL, unset);\n \n \tcase OPTION_BIT:\n \t\tif (unset)\n@@ -161,16 +161,27 @@ static enum parse_opt_result get_value(struct parse_opt_ctx_t *p,\n \t\treturn err;\n \n \tcase OPTION_CALLBACK:\n+\t{\n+\t\tconst char *p_arg = NULL;\n+\t\tint p_unset;\n+\n \t\tif (unset)\n-\t\t\treturn (*opt->callback)(opt, NULL, 1) ? (-1) : 0;\n-\t\tif (opt->flags & PARSE_OPT_NOARG)\n-\t\t\treturn (*opt->callback)(opt, NULL, 0) ? (-1) : 0;\n-\t\tif (opt->flags & PARSE_OPT_OPTARG && !p->opt)\n-\t\t\treturn (*opt->callback)(opt, NULL, 0) ? (-1) : 0;\n-\t\tif (get_arg(p, opt, flags, &arg))\n+\t\t\tp_unset = 1;\n+\t\telse if (opt->flags & PARSE_OPT_NOARG)\n+\t\t\tp_unset = 0;\n+\t\telse if (opt->flags & PARSE_OPT_OPTARG && !p->opt)\n+\t\t\tp_unset = 0;\n+\t\telse if (get_arg(p, opt, flags, &arg))\n \t\t\treturn -1;\n-\t\treturn (*opt->callback)(opt, arg, 0) ? (-1) : 0;\n-\n+\t\telse {\n+\t\t\tp_unset = 0;\n+\t\t\tp_arg = arg;\n+\t\t}\n+\t\tif (opt->callback)\n+\t\t\treturn (*opt->callback)(opt, p_arg, p_unset) ? (-1) : 0;\n+\t\telse\n+\t\t\treturn (*opt->ll_callback)(p, opt, p_arg, p_unset);\n+\t}\n \tcase OPTION_INTEGER:\n \t\tif (unset) {\n \t\t\t*(int *)opt->value = 0;\n@@ -238,7 +249,10 @@ static enum parse_opt_result parse_short_opt(struct parse_opt_ctx_t *p,\n \t\t\tlen++;\n \t\targ = xmemdupz(p->opt, len);\n \t\tp->opt = p->opt[len] ? p->opt + len : NULL;\n-\t\trc = (*numopt->callback)(numopt, arg, 0) ? (-1) : 0;\n+\t\tif (numopt->callback)\n+\t\t\trc = (*numopt->callback)(numopt, arg, 0) ? (-1) : 0;\n+\t\telse\n+\t\t\trc = (*numopt->ll_callback)(p, numopt, arg, 0);\n \t\tfree(arg);\n \t\treturn rc;\n \t}\n@@ -414,10 +428,10 @@ static void parse_options_check(const struct option *opts)\n \t\t\t\terr |= optbug(opts, \"should not accept an argument\");\n \t\t\tbreak;\n \t\tcase OPTION_CALLBACK:\n-\t\t\tif (!opts->callback)\n-\t\t\t\tBUG(\"OPTION_CALLBACK needs a callback\");\n-\t\t\tif (opts->ll_callback)\n-\t\t\t\tBUG(\"OPTION_CALLBACK needs no ll_callback\");\n+\t\t\tif (!opts->callback && !opts->ll_callback)\n+\t\t\t\tBUG(\"OPTION_CALLBACK needs one callback\");\n+\t\t\tif (opts->callback && opts->ll_callback)\n+\t\t\t\tBUG(\"OPTION_CALLBACK can't have two callbacks\");\n \t\t\tbreak;\n \t\tcase OPTION_LOWLEVEL_CALLBACK:\n \t\t\tif (!opts->ll_callback)\ndiff --git a/parse-options.h b/parse-options.h\nindex 4e49185027..ce75278804 100644\n--- a/parse-options.h\n+++ b/parse-options.h\n@@ -50,7 +50,8 @@ typedef int parse_opt_cb(const struct option *, const char *arg, int unset);\n \n struct parse_opt_ctx_t;\n typedef enum parse_opt_result parse_opt_ll_cb(struct parse_opt_ctx_t *ctx,\n-\t\t\t\t\t      const struct option *opt, int unset);\n+\t\t\t\t\t      const struct option *opt,\n+\t\t\t\t\t      const char *arg, int unset);\n \n /*\n  * `type`::\n@@ -267,7 +268,7 @@ int parse_opt_commits(const struct option *, const char *, int);\n int parse_opt_tertiary(const struct option *, const char *, int);\n int parse_opt_string_list(const struct option *, const char *, int);\n int parse_opt_noop_cb(const struct option *, const char *, int);\n-int parse_opt_unknown_cb(struct parse_opt_ctx_t *ctx, const struct option *, int);\n+int parse_opt_unknown_cb(struct parse_opt_ctx_t *ctx, const struct option *, const char *, int);\n int parse_opt_passthru(const struct option *, const char *, int);\n int parse_opt_passthru_argv(const struct option *, const char *, int);\n \n-- \n2.20.1.560.g70ca8b83ee\n\n"},{"id":"367737","messageId":"20190127003535.28341-9-pclouds@gmail.com","threadId":"50334","inReplyTo":"20190127003535.28341-1-pclouds@gmail.com","subject":"[PATCH 08/14] diff.h: keep forward struct declarations sorted","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2019-01-27T00:35:29Z","receivedAt":"2019-01-27T00:36:47Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n diff.h | 12 ++++++------\n 1 file changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/diff.h b/diff.h\nindex b512d0477a..c872a41344 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -9,16 +9,16 @@\n #include \"object.h\"\n #include \"oidset.h\"\n \n-struct rev_info;\n+struct combine_diff_path;\n+struct commit;\n+struct diff_filespec;\n struct diff_options;\n struct diff_queue_struct;\n-struct strbuf;\n-struct diff_filespec;\n-struct userdiff_driver;\n struct oid_array;\n-struct commit;\n-struct combine_diff_path;\n struct repository;\n+struct rev_info;\n+struct strbuf;\n+struct userdiff_driver;\n \n typedef int (*pathchange_fn_t)(struct diff_options *options,\n \t\t struct combine_diff_path *path);\n-- \n2.20.1.560.g70ca8b83ee\n\n"},{"id":"367738","messageId":"20190127003535.28341-10-pclouds@gmail.com","threadId":"50334","inReplyTo":"20190127003535.28341-1-pclouds@gmail.com","subject":"[PATCH 09/14] diff.h: avoid bit fields in struct diff_flags","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2019-01-27T00:35:30Z","receivedAt":"2019-01-27T00:36:53Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Bitfield addresses cannot be passed around in a pointer. This makes it\nhard to use parse-options to set/unset them. Turn this struct to\nnormal integers. This of course increases the size of this struct\nmultiple times, but since we only have a handful of diff_options\nvariables around, memory consumption is not at all a concern.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n diff.h | 66 +++++++++++++++++++++++++++++-----------------------------\n 1 file changed, 33 insertions(+), 33 deletions(-)\n\ndiff --git a/diff.h b/diff.h\nindex c872a41344..8abe1649d0 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -64,39 +64,39 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)\n \n #define DIFF_FLAGS_INIT { 0 }\n struct diff_flags {\n-\tunsigned recursive:1;\n-\tunsigned tree_in_recursive:1;\n-\tunsigned binary:1;\n-\tunsigned text:1;\n-\tunsigned full_index:1;\n-\tunsigned silent_on_remove:1;\n-\tunsigned find_copies_harder:1;\n-\tunsigned follow_renames:1;\n-\tunsigned rename_empty:1;\n-\tunsigned has_changes:1;\n-\tunsigned quick:1;\n-\tunsigned no_index:1;\n-\tunsigned allow_external:1;\n-\tunsigned exit_with_status:1;\n-\tunsigned reverse_diff:1;\n-\tunsigned check_failed:1;\n-\tunsigned relative_name:1;\n-\tunsigned ignore_submodules:1;\n-\tunsigned dirstat_cumulative:1;\n-\tunsigned dirstat_by_file:1;\n-\tunsigned allow_textconv:1;\n-\tunsigned textconv_set_via_cmdline:1;\n-\tunsigned diff_from_contents:1;\n-\tunsigned dirty_submodules:1;\n-\tunsigned ignore_untracked_in_submodules:1;\n-\tunsigned ignore_dirty_submodules:1;\n-\tunsigned override_submodule_config:1;\n-\tunsigned dirstat_by_line:1;\n-\tunsigned funccontext:1;\n-\tunsigned default_follow_renames:1;\n-\tunsigned stat_with_summary:1;\n-\tunsigned suppress_diff_headers:1;\n-\tunsigned dual_color_diffed_diffs:1;\n+\tunsigned recursive;\n+\tunsigned tree_in_recursive;\n+\tunsigned binary;\n+\tunsigned text;\n+\tunsigned full_index;\n+\tunsigned silent_on_remove;\n+\tunsigned find_copies_harder;\n+\tunsigned follow_renames;\n+\tunsigned rename_empty;\n+\tunsigned has_changes;\n+\tunsigned quick;\n+\tunsigned no_index;\n+\tunsigned allow_external;\n+\tunsigned exit_with_status;\n+\tunsigned reverse_diff;\n+\tunsigned check_failed;\n+\tunsigned relative_name;\n+\tunsigned ignore_submodules;\n+\tunsigned dirstat_cumulative;\n+\tunsigned dirstat_by_file;\n+\tunsigned allow_textconv;\n+\tunsigned textconv_set_via_cmdline;\n+\tunsigned diff_from_contents;\n+\tunsigned dirty_submodules;\n+\tunsigned ignore_untracked_in_submodules;\n+\tunsigned ignore_dirty_submodules;\n+\tunsigned override_submodule_config;\n+\tunsigned dirstat_by_line;\n+\tunsigned funccontext;\n+\tunsigned default_follow_renames;\n+\tunsigned stat_with_summary;\n+\tunsigned suppress_diff_headers;\n+\tunsigned dual_color_diffed_diffs;\n };\n \n static inline void diff_flags_or(struct diff_flags *a,\n-- \n2.20.1.560.g70ca8b83ee\n\n"},{"id":"367739","messageId":"20190127003535.28341-11-pclouds@gmail.com","threadId":"50334","inReplyTo":"20190127003535.28341-1-pclouds@gmail.com","subject":"[PATCH 10/14] diff.c: prepare to use parse_options() for parsing","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2019-01-27T00:35:31Z","receivedAt":"2019-01-27T00:36:59Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This is a preparation step to start using parse_options() to parse\ndiff/revision options instead of what we have now. There are a couple\nof good things from using parse_options():\n\n- better help usage\n- easier to add new options\n- better completion support\n- help usage generation\n- better integration with main command option parser. We can just\n  concat the main command's option array and diffopt's together and\n  parse all in one go.\n- detect colidding options (e.g. --reverse is used by revision code,\n  so diff code can't use it as long name for -R)\n- consistent syntax, e.g. option that takes mandatory argument will\n  now accept both \"--option=value\" and \"--option value\".\n\nThe plan is migrate all diff/rev options to parse_options(). Then we\ncould get rid of diff_opt_parse() and expose parseopts[] directly to\nthe caller.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n diff.c | 27 +++++++++++++++++++++++++++\n diff.h |  2 ++\n 2 files changed, 29 insertions(+)\n\ndiff --git a/diff.c b/diff.c\nindex 1b5f276360..80b4af23d7 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -23,6 +23,7 @@\n #include \"argv-array.h\"\n #include \"graph.h\"\n #include \"packfile.h\"\n+#include \"parse-options.h\"\n #include \"help.h\"\n \n #ifdef NO_FAST_WORKING_DIRECTORY\n@@ -4425,6 +4426,8 @@ static void run_checkdiff(struct diff_filepair *p, struct diff_options *o)\n \tbuiltin_checkdiff(name, other, attr_path, p->one, p->two, o);\n }\n \n+static void prep_parse_options(struct diff_options *options);\n+\n void repo_diff_setup(struct repository *r, struct diff_options *options)\n {\n \tmemcpy(options, &default_diff_options, sizeof(*options));\n@@ -4466,6 +4469,8 @@ void repo_diff_setup(struct repository *r, struct diff_options *options)\n \n \toptions->color_moved = diff_color_moved_default;\n \toptions->color_moved_ws_handling = diff_color_moved_ws_default;\n+\n+\tprep_parse_options(options);\n }\n \n void diff_setup_done(struct diff_options *options)\n@@ -4569,6 +4574,8 @@ void diff_setup_done(struct diff_options *options)\n \n \tif (!options->use_color || external_diff())\n \t\toptions->color_moved = 0;\n+\n+\tFREE_AND_NULL(options->parseopts);\n }\n \n static int opt_arg(const char *arg, int arg_short, const char *arg_long, int *val)\n@@ -4860,6 +4867,16 @@ static int parse_objfind_opt(struct diff_options *opt, const char *arg)\n \treturn 1;\n }\n \n+static void prep_parse_options(struct diff_options *options)\n+{\n+\tstruct option parseopts[] = {\n+\t\tOPT_END()\n+\t};\n+\n+\tALLOC_ARRAY(options->parseopts, ARRAY_SIZE(parseopts));\n+\tmemcpy(options->parseopts, parseopts, sizeof(parseopts));\n+}\n+\n int diff_opt_parse(struct diff_options *options,\n \t\t   const char **av, int ac, const char *prefix)\n {\n@@ -4870,6 +4887,16 @@ int diff_opt_parse(struct diff_options *options,\n \tif (!prefix)\n \t\tprefix = \"\";\n \n+\tac = parse_options(ac, av, prefix, options->parseopts, NULL,\n+\t\t\t   PARSE_OPT_KEEP_DASHDASH |\n+\t\t\t   PARSE_OPT_KEEP_UNKNOWN |\n+\t\t\t   PARSE_OPT_NO_INTERNAL_HELP |\n+\t\t\t   PARSE_OPT_ONE_SHOT |\n+\t\t\t   PARSE_OPT_STOP_AT_NON_OPTION);\n+\n+\tif (ac)\n+\t\treturn ac;\n+\n \t/* Output format options */\n \tif (!strcmp(arg, \"-p\") || !strcmp(arg, \"-u\") || !strcmp(arg, \"--patch\")\n \t    || opt_arg(arg, 'U', \"unified\", &options->context))\ndiff --git a/diff.h b/diff.h\nindex 8abe1649d0..d9ad73f0e1 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -15,6 +15,7 @@ struct diff_filespec;\n struct diff_options;\n struct diff_queue_struct;\n struct oid_array;\n+struct option;\n struct repository;\n struct rev_info;\n struct strbuf;\n@@ -229,6 +230,7 @@ struct diff_options {\n \tunsigned color_moved_ws_handling;\n \n \tstruct repository *repo;\n+\tstruct option *parseopts;\n };\n \n void diff_emit_submodule_del(struct diff_options *o, const char *line);\n-- \n2.20.1.560.g70ca8b83ee\n\n"},{"id":"367740","messageId":"20190127003535.28341-12-pclouds@gmail.com","threadId":"50334","inReplyTo":"20190127003535.28341-1-pclouds@gmail.com","subject":"[PATCH 11/14] diff.c: convert -u|-p|--patch","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2019-01-27T00:35:32Z","receivedAt":"2019-01-27T00:37:04Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n diff.c | 10 ++++++++--\n 1 file changed, 8 insertions(+), 2 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 80b4af23d7..a4a40e4aa8 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -4870,6 +4870,13 @@ static int parse_objfind_opt(struct diff_options *opt, const char *arg)\n static void prep_parse_options(struct diff_options *options)\n {\n \tstruct option parseopts[] = {\n+\t\tOPT_GROUP(N_(\"Diff output format options\")),\n+\t\tOPT_BITOP('p', \"patch\", &options->output_format,\n+\t\t\t  N_(\"generate patch\"),\n+\t\t\t  DIFF_FORMAT_PATCH, DIFF_FORMAT_NO_OUTPUT),\n+\t\tOPT_BITOP('u', NULL, &options->output_format,\n+\t\t\t  N_(\"generate patch\"),\n+\t\t\t  DIFF_FORMAT_PATCH, DIFF_FORMAT_NO_OUTPUT),\n \t\tOPT_END()\n \t};\n \n@@ -4898,8 +4905,7 @@ int diff_opt_parse(struct diff_options *options,\n \t\treturn ac;\n \n \t/* Output format options */\n-\tif (!strcmp(arg, \"-p\") || !strcmp(arg, \"-u\") || !strcmp(arg, \"--patch\")\n-\t    || opt_arg(arg, 'U', \"unified\", &options->context))\n+\tif (opt_arg(arg, 'U', \"unified\", &options->context))\n \t\tenable_patch_output(&options->output_format);\n \telse if (!strcmp(arg, \"--raw\"))\n \t\toptions->output_format |= DIFF_FORMAT_RAW;\n-- \n2.20.1.560.g70ca8b83ee\n\n"},{"id":"367741","messageId":"20190127003535.28341-13-pclouds@gmail.com","threadId":"50334","inReplyTo":"20190127003535.28341-1-pclouds@gmail.com","subject":"[PATCH 12/14] diff.c: convert -U|--unified","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2019-01-27T00:35:33Z","receivedAt":"2019-01-27T00:37:09Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n Documentation/diff-options.txt |  2 +-\n diff.c                         | 23 ++++++++++++++++++++---\n parse-options.h                |  5 +++--\n 3 files changed, 24 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex b94d332f71..0711734b12 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -36,7 +36,7 @@ endif::git-format-patch[]\n -U<n>::\n --unified=<n>::\n \tGenerate diffs with <n> lines of context instead of\n-\tthe usual three.\n+\tthe usual three. Implies `--patch`.\n ifndef::git-format-patch[]\n \tImplies `-p`.\n endif::git-format-patch[]\ndiff --git a/diff.c b/diff.c\nindex a4a40e4aa8..093158244e 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -4867,6 +4867,22 @@ static int parse_objfind_opt(struct diff_options *opt, const char *arg)\n \treturn 1;\n }\n \n+static int diff_opt_unified(const struct option *opt,\n+\t\t\t    const char *arg, int unset)\n+{\n+\tstruct diff_options *options = opt->value;\n+\tchar *s;\n+\n+\tBUG_ON_OPT_NEG(unset);\n+\n+\toptions->context = strtol(arg, &s, 10);\n+\tif (*s)\n+\t\treturn error(_(\"%s expects a numerical value\"), \"--unified\");\n+\tenable_patch_output(&options->output_format);\n+\n+\treturn 0;\n+}\n+\n static void prep_parse_options(struct diff_options *options)\n {\n \tstruct option parseopts[] = {\n@@ -4877,6 +4893,9 @@ static void prep_parse_options(struct diff_options *options)\n \t\tOPT_BITOP('u', NULL, &options->output_format,\n \t\t\t  N_(\"generate patch\"),\n \t\t\t  DIFF_FORMAT_PATCH, DIFF_FORMAT_NO_OUTPUT),\n+\t\tOPT_CALLBACK_F('U', \"unified\", options, N_(\"<n>\"),\n+\t\t\t       N_(\"generate diffs with <n> lines context\"),\n+\t\t\t       PARSE_OPT_NONEG, diff_opt_unified),\n \t\tOPT_END()\n \t};\n \n@@ -4905,9 +4924,7 @@ int diff_opt_parse(struct diff_options *options,\n \t\treturn ac;\n \n \t/* Output format options */\n-\tif (opt_arg(arg, 'U', \"unified\", &options->context))\n-\t\tenable_patch_output(&options->output_format);\n-\telse if (!strcmp(arg, \"--raw\"))\n+\tif (!strcmp(arg, \"--raw\"))\n \t\toptions->output_format |= DIFF_FORMAT_RAW;\n \telse if (!strcmp(arg, \"--patch-with-raw\")) {\n \t\tenable_patch_output(&options->output_format);\ndiff --git a/parse-options.h b/parse-options.h\nindex ce75278804..7d83e2971d 100644\n--- a/parse-options.h\n+++ b/parse-options.h\n@@ -134,6 +134,8 @@ struct option {\n #define OPT_SET_INT_F(s, l, v, h, i, f) { OPTION_SET_INT, (s), (l), (v), NULL, \\\n \t\t\t\t\t  (h), PARSE_OPT_NOARG | (f), NULL, (i) }\n #define OPT_BOOL_F(s, l, v, h, f)   OPT_SET_INT_F(s, l, v, h, 1, f)\n+#define OPT_CALLBACK_F(s, l, v, a, h, f, cb)\t\t\t\\\n+\t{ OPTION_CALLBACK, (s), (l), (v), (a), (h), (f), (cb) }\n \n #define OPT_END()                   { OPTION_END }\n #define OPT_ARGUMENT(l, h)          { OPTION_ARGUMENT, 0, (l), NULL, NULL, \\\n@@ -164,8 +166,7 @@ struct option {\n #define OPT_EXPIRY_DATE(s, l, v, h) \\\n \t{ OPTION_CALLBACK, (s), (l), (v), N_(\"expiry-date\"),(h), 0,\t\\\n \t  parse_opt_expiry_date_cb }\n-#define OPT_CALLBACK(s, l, v, a, h, f) \\\n-\t{ OPTION_CALLBACK, (s), (l), (v), (a), (h), 0, (f) }\n+#define OPT_CALLBACK(s, l, v, a, h, f) OPT_CALLBACK_F(s, l, v, a, h, 0, f)\n #define OPT_NUMBER_CALLBACK(v, h, f) \\\n \t{ OPTION_NUMBER, 0, NULL, (v), NULL, (h), \\\n \t  PARSE_OPT_NOARG | PARSE_OPT_NONEG, (f) }\n-- \n2.20.1.560.g70ca8b83ee\n\n"},{"id":"367742","messageId":"20190127003535.28341-14-pclouds@gmail.com","threadId":"50334","inReplyTo":"20190127003535.28341-1-pclouds@gmail.com","subject":"[PATCH 13/14] diff.c: convert -W|--[no-]function-context","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2019-01-27T00:35:34Z","receivedAt":"2019-01-27T00:37:14Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n diff.c | 8 ++------\n 1 file changed, 2 insertions(+), 6 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 093158244e..8f70101362 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -4896,6 +4896,8 @@ static void prep_parse_options(struct diff_options *options)\n \t\tOPT_CALLBACK_F('U', \"unified\", options, N_(\"<n>\"),\n \t\t\t       N_(\"generate diffs with <n> lines context\"),\n \t\t\t       PARSE_OPT_NONEG, diff_opt_unified),\n+\t\tOPT_BOOL('W', \"function-context\", &options->flags.funccontext,\n+\t\t\t N_(\"generate diffs with <n> lines context\")),\n \t\tOPT_END()\n \t};\n \n@@ -5212,12 +5214,6 @@ int diff_opt_parse(struct diff_options *options,\n \telse if (opt_arg(arg, '\\0', \"inter-hunk-context\",\n \t\t\t &options->interhunkcontext))\n \t\t;\n-\telse if (!strcmp(arg, \"-W\"))\n-\t\toptions->flags.funccontext = 1;\n-\telse if (!strcmp(arg, \"--function-context\"))\n-\t\toptions->flags.funccontext = 1;\n-\telse if (!strcmp(arg, \"--no-function-context\"))\n-\t\toptions->flags.funccontext = 0;\n \telse if ((argcount = parse_long_opt(\"output\", av, &optarg))) {\n \t\tchar *path = prefix_filename(prefix, optarg);\n \t\toptions->file = xfopen(path, \"w\");\n-- \n2.20.1.560.g70ca8b83ee\n\n"},{"id":"367743","messageId":"20190127003535.28341-15-pclouds@gmail.com","threadId":"50334","inReplyTo":"20190127003535.28341-1-pclouds@gmail.com","subject":"[PATCH 14/14] diff.c: convert --raw","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2019-01-27T00:35:35Z","receivedAt":"2019-01-27T00:37:19Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n diff.c | 7 ++++---\n 1 file changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 8f70101362..4bc9df7362 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -4898,6 +4898,9 @@ static void prep_parse_options(struct diff_options *options)\n \t\t\t       PARSE_OPT_NONEG, diff_opt_unified),\n \t\tOPT_BOOL('W', \"function-context\", &options->flags.funccontext,\n \t\t\t N_(\"generate diffs with <n> lines context\")),\n+\t\tOPT_BIT_F(0, \"raw\", &options->output_format,\n+\t\t\t  N_(\"generate the diff in raw format\"),\n+\t\t\t  DIFF_FORMAT_RAW, PARSE_OPT_NONEG),\n \t\tOPT_END()\n \t};\n \n@@ -4926,9 +4929,7 @@ int diff_opt_parse(struct diff_options *options,\n \t\treturn ac;\n \n \t/* Output format options */\n-\tif (!strcmp(arg, \"--raw\"))\n-\t\toptions->output_format |= DIFF_FORMAT_RAW;\n-\telse if (!strcmp(arg, \"--patch-with-raw\")) {\n+\tif (!strcmp(arg, \"--patch-with-raw\")) {\n \t\tenable_patch_output(&options->output_format);\n \t\toptions->output_format |= DIFF_FORMAT_RAW;\n \t} else if (!strcmp(arg, \"--numstat\"))\n-- \n2.20.1.560.g70ca8b83ee\n\n"},{"id":"367808","messageId":"xmqq8sz5ei3y.fsf@gitster-ct.c.googlers.com","threadId":"50334","inReplyTo":"20190127003535.28341-1-pclouds@gmail.com","subject":"Re: [PATCH 00/14] nd/diff-parseopt part 1","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-01-28T00:33:21Z","receivedAt":"2019-01-28T00:33:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n\n> This is the first part of converting diff_opt_parse() to use\n> parse_options(). Compared to the full series [1] I sent earlier, 03/76\n> is dropped and 02/14 is updated to allow KEEP_UNKNOWN and\n> STOP_AT_NON_OPTION combination, but only for one shot mode.\n\nLooked reasonable.  Will re-queue.\n\nThanks.\n"},{"id":"373875","messageId":"02d2a828-b191-9d9a-7422-d76cdca69ef1@gmail.com","threadId":"50334","inReplyTo":"20190127003535.28341-8-pclouds@gmail.com","subject":"Re: [PATCH 07/14] parse-options: allow ll_callback with OPTION_CALLBACK","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2019-04-15T14:06:33Z","receivedAt":"2019-04-15T14:06:38Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 1/26/2019 7:35 PM, Nguyễn Thái Ngọc Duy wrote:\n> @@ -238,7 +249,10 @@ static enum parse_opt_result parse_short_opt(struct parse_opt_ctx_t *p,\n>  \t\t\tlen++;\n>  \t\targ = xmemdupz(p->opt, len);\n>  \t\tp->opt = p->opt[len] ? p->opt + len : NULL;\n> -\t\trc = (*numopt->callback)(numopt, arg, 0) ? (-1) : 0;\n> +\t\tif (numopt->callback)\n> +\t\t\trc = (*numopt->callback)(numopt, arg, 0) ? (-1) : 0;\n> +\t\telse\n> +\t\t\trc = (*numopt->ll_callback)(p, numopt, arg, 0);\n>  \t\tfree(arg);\n>  \t\treturn rc;\n>  \t}\n\nHi Duy,\n\nThis \"else\" condition is unreachable. This block is only hit when we have a \"-<n>\"\noption, using OPT_NUMBER_CALLBACK, which is implemented by filling \"callback\", never\n\"ll_callback\".\n\nI recommend reverting this diff segment, but please let me know if I'm missing something.\n\nThanks,\n-Stolee\n"},{"id":"373931","messageId":"CACsJy8DDKuK5VYhh0GNYSJK1_y3MZgK5Vcq99N4jYcusVFnvQQ@mail.gmail.com","threadId":"50334","inReplyTo":"02d2a828-b191-9d9a-7422-d76cdca69ef1@gmail.com","subject":"Re: [PATCH 07/14] parse-options: allow ll_callback with OPTION_CALLBACK","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2019-04-16T08:52:45Z","receivedAt":"2019-04-16T08:53:13Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Apr 15, 2019 at 9:06 PM Derrick Stolee <stolee@gmail.com> wrote:\n>\n> On 1/26/2019 7:35 PM, Nguyễn Thái Ngọc Duy wrote:\n> > @@ -238,7 +249,10 @@ static enum parse_opt_result parse_short_opt(struct parse_opt_ctx_t *p,\n> >                       len++;\n> >               arg = xmemdupz(p->opt, len);\n> >               p->opt = p->opt[len] ? p->opt + len : NULL;\n> > -             rc = (*numopt->callback)(numopt, arg, 0) ? (-1) : 0;\n> > +             if (numopt->callback)\n> > +                     rc = (*numopt->callback)(numopt, arg, 0) ? (-1) : 0;\n> > +             else\n> > +                     rc = (*numopt->ll_callback)(p, numopt, arg, 0);\n> >               free(arg);\n> >               return rc;\n> >       }\n>\n> Hi Duy,\n>\n> This \"else\" condition is unreachable. This block is only hit when we have a \"-<n>\"\n> option, using OPT_NUMBER_CALLBACK, which is implemented by filling \"callback\", never\n> \"ll_callback\".\n\nIt does not mean ll_callback cannot be used in the future though. We\nhave three options\n\n1. drop the else clause\n2. replace with \"else BUG();\"\n3. implement proper else clause\n\nOption #1 to me sounds wrong. If you don't support something, yell up.\nSilently ignoring it only makes it harder to track down to this\nunsupported location when it becomes reachable, however unlikely that\nis.\n\nWhich leaves options #2 and #3. If you think this one line is risky\nenough, I'll send a patch to replace it with BUG().\n\n> I recommend reverting this diff segment, but please let me know if I'm missing something.\n>\n> Thanks,\n> -Stolee\n\n\n\n-- \nDuy\n"},{"id":"373974","messageId":"6b731f57-eada-8c1c-8e5d-de386b0c414e@gmail.com","threadId":"50334","inReplyTo":"CACsJy8DDKuK5VYhh0GNYSJK1_y3MZgK5Vcq99N4jYcusVFnvQQ@mail.gmail.com","subject":"Re: [PATCH 07/14] parse-options: allow ll_callback with OPTION_CALLBACK","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2019-04-16T14:24:39Z","receivedAt":"2019-04-16T14:24:43Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 4/16/2019 4:52 AM, Duy Nguyen wrote:\n> On Mon, Apr 15, 2019 at 9:06 PM Derrick Stolee <stolee@gmail.com> wrote:\n>>\n>> On 1/26/2019 7:35 PM, Nguyễn Thái Ngọc Duy wrote:\n>>> @@ -238,7 +249,10 @@ static enum parse_opt_result parse_short_opt(struct parse_opt_ctx_t *p,\n>>>                       len++;\n>>>               arg = xmemdupz(p->opt, len);\n>>>               p->opt = p->opt[len] ? p->opt + len : NULL;\n>>> -             rc = (*numopt->callback)(numopt, arg, 0) ? (-1) : 0;\n>>> +             if (numopt->callback)\n>>> +                     rc = (*numopt->callback)(numopt, arg, 0) ? (-1) : 0;\n>>> +             else\n>>> +                     rc = (*numopt->ll_callback)(p, numopt, arg, 0);\n>>>               free(arg);\n>>>               return rc;\n>>>       }\n>>\n>> Hi Duy,\n>>\n>> This \"else\" condition is unreachable. This block is only hit when we have a \"-<n>\"\n>> option, using OPT_NUMBER_CALLBACK, which is implemented by filling \"callback\", never\n>> \"ll_callback\".\n> \n> It does not mean ll_callback cannot be used in the future though.\n\nThat's not a very good reason to add it now. YAGNI.\n\n> We\n> have three options\n> \n> 1. drop the else clause\n> 2. replace with \"else BUG();\"\n> 3. implement proper else clause\n> \n> Option #1 to me sounds wrong. If you don't support something, yell up.\n> Silently ignoring it only makes it harder to track down to this\n> unsupported location when it becomes reachable, however unlikely that\n> is.\n> \n> Which leaves options #2 and #3. If you think this one line is risky\n> enough, I'll send a patch to replace it with BUG().\n\nIt's not about risk, but the fact that it is pointless. The only way to get to\nthis block is to create a 'struct option' with type OPTION_NUMBER manually\n(ignoring the OPT_NUMBER_CALLBACK macro), which _should_ be unsupported. If\nsomeone goes to the pain of adding a way to instantiate with a low-level callback,\nthen they should add this 'if' statement.\n\nIf you are going to add protection from an incorrect instantiation of 'callback'\nin a use of OPT_NUMBER_CALLBACK, then the proper place to do that is probably in\nparse_options_check(), where you were already doing callback-vs-ll_callback checks\nin the case of OPTION_CALLBACK and OPTION_LOWLEVEL_CALLBACK.\n\nHowever, the only places where the OPT_NUMBER_CALLBACK option appears are\nbuiltin/grep.c and t/helper/test-parse-options.c. I don't imagine a need to\nadd low-level callbacks any time soon.\n\n>> I recommend reverting this diff segment, but please let me know if I'm missing something.\n\nI still think this is the easiest way to remove the dead code.\n\nI'll try to submit a patch later. I'm still not in full work mode, so I may be slow.\n\nThanks,\n-Stolee\n\n"}]}