{"thread":{"id":"50115","subject":"[PATCH 00/75] Convert diff opt parser to parse_options()","startedAt":"2018-12-27T16:25:48Z","lastAt":"2019-01-09T20:22:08Z","messageCount":18,"participants":["Nguyễn Thái Ngọc Duy","Eric Sunshine","Stefan Beller"],"isPatch":true,"patchVersion":1,"patchTotal":75},"messages":[{"id":"365852","messageId":"20181227162536.15895-1-pclouds@gmail.com","threadId":"50115","inReplyTo":null,"subject":"[PATCH 00/75] Convert diff opt parser to parse_options()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-12-27T16:25:21Z","receivedAt":"2018-12-27T16:25:48Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This is a sneak peek of converting diff_opt_parse() to 'struct\noption'. The end game is builtin commands can just add diff options to\ntheir 'struct option[]' array, do parse_options() like usual and have\ndiff parsing for free.\n\nBesides simplifying the parsing code, there are also more benefits of\nusing struct option, e.g. tab completion and \"git cmd -h\".\n\nI'm not sending full series because it's looong, the middle is not\nthat interesting, and I would need to wait for\nsb/diff-color-moved-config-option-fixup (and Brian's --literally too,\nprobably) to land before sending something mergeable. But main patches\nare sent here for review.\n\nThis is of course just the beginning. Once revision.c parser is\nconverted, diff_opt_parse() will be killed off, more commands can take\nuse diff option array directly. And perhaps the diff option array will\nbe split up into smaller ones to give callers more control what's part\nof the command.\n\nNguyễn Thái Ngọc Duy (75):\n  parse-options.h: remove extern on function prototypes\n  parse-options: add one-shot mode\n  parse-options: allow keep-unknown + stop-at-non-opt combination\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  diff.c: convert --patch-with-raw\n  diff.c: convert --numstat and --shortstat\n  diff.c: convert --dirstat and friends\n  diff.c: convert --check\n  diff.c: convert --summary\n  diff.c: convert --patch-with-stat\n  diff.c: convert --name-only\n  diff.c: convert --name-status\n  diff.c: convert -s|--no-patch\n  diff.c: convert --stat*\n  diff.c: convert --[no-]compact-summary\n  diff.c: convert --output-*\n  diff.c: convert -B|--break-rewrites\n  diff.c: convert -M|--find-renames\n  diff.c: convert -D|--irreversible-delete\n  diff.c: convert -C|--find-copies\n  diff.c: convert --find-copies-harder\n  diff.c: convert --no-renames|--[no--rename-empty\n  diff.c: convert --relative\n  diff.c: convert --[no-]minimal\n  diff.c: convert --ignore-some-changes\n  diff.c: convert --[no-]indent-heuristic\n  diff.c: convert --patience\n  diff.c: convert --histogram\n  diff.c: convert --diff-algorithm\n  diff.c: convert --anchored\n  diff.c: convert --binary\n  diff.c: convert --full-index\n  diff.c: convert -a|--text\n  diff.c: convert -R\n  diff.c: convert --[no-]follow\n  diff.c: convert --[no-]color\n  diff.c: convert --word-diff\n  diff.c: convert --word-diff-regex\n  diff.c: convert --color-words\n  diff.c: convert --exit-code\n  diff.c: convert --quiet\n  diff.c: convert --ext-diff\n  diff.c: convert --textconv\n  diff.c: convert --ignore-submodules\n  diff.c: convert --submodule\n  diff.c: convert --ws-error-highlight\n  diff.c: convert --ita-[in]visible-in-index\n  diff.c: convert -z\n  diff.c: convert -l\n  diff.c: convert -S|-G\n  diff.c: convert --pickaxe-all|--pickaxe-regex\n  diff.c: convert -O\n  diff.c: convert --find-object\n  diff.c: convert --diff-filter\n  diff.c: convert --[no-]abbrev\n  diff.c: convert --[src|dst]-prefix\n  diff.c: convert --line-prefix\n  diff.c: convert --no-prefix\n  diff.c: convert --inter-hunk-context\n  diff.c: convert --color-moved\n  diff.c: convert --color-moved-ws\n  range-diff: use parse_options() instead of diff_opt_parse()\n  diff --no-index: use parse_options() instead of diff_opt_parse()\n  am: avoid diff_opt_parse()\n\n Documentation/diff-options.txt |   24 +-\n builtin/am.c                   |    4 +-\n builtin/blame.c                |    2 +-\n builtin/diff.c                 |   21 +-\n builtin/merge.c                |    9 +-\n builtin/range-diff.c           |   26 +-\n builtin/update-index.c         |   41 +-\n diff-no-index.c                |   49 +-\n diff.c                         | 1132 ++++++++++++++++++++------------\n diff.h                         |   85 +--\n parse-options-cb.c             |   11 +-\n parse-options.c                |  152 +++--\n parse-options.h                |  116 ++--\n t/t4053-diff-no-index.sh       |    3 +-\n t/t7800-difftool.sh            |    4 +-\n 15 files changed, 1051 insertions(+), 628 deletions(-)\n\n-- \n2.20.0.482.g66447595a7\n\n"},{"id":"365853","messageId":"20181227162536.15895-2-pclouds@gmail.com","threadId":"50115","inReplyTo":"20181227162536.15895-1-pclouds@gmail.com","subject":"[PATCH 01/75] parse-options.h: remove extern on function prototypes","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-12-27T16:25:22Z","receivedAt":"2018-12-27T16:25:52Z","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>\n---\n parse-options.h | 60 ++++++++++++++++++++++++-------------------------\n 1 file changed, 30 insertions(+), 30 deletions(-)\n\ndiff --git a/parse-options.h b/parse-options.h\nindex a650a7d220..1947cb27cf 100644\n--- a/parse-options.h\n+++ b/parse-options.h\n@@ -174,19 +174,19 @@ 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-                         const struct option *options,\n-                         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-                                        const 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-extern int opterror(const struct option *opt, const char *reason, int flags);\n+int optbug(const struct option *opt, const char *reason);\n+int opterror(const struct option *opt, const char *reason, int flags);\n #if defined(__GNUC__)\n #define opterror(o,r,f) (opterror((o),(r),(f)), const_error())\n #endif\n@@ -230,31 +230,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.0.482.g66447595a7\n\n"},{"id":"365854","messageId":"20181227162536.15895-3-pclouds@gmail.com","threadId":"50115","inReplyTo":"20181227162536.15895-1-pclouds@gmail.com","subject":"[PATCH 02/75] parse-options: add one-shot mode","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-12-27T16:25:23Z","receivedAt":"2018-12-27T16:25:53Z","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 | 23 ++++++++++++++++++++---\n parse-options.h | 17 +++++++++++++----\n 2 files changed, 33 insertions(+), 7 deletions(-)\n\ndiff --git a/parse-options.c b/parse-options.c\nindex 6932eaff61..d47e217b07 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -415,15 +415,23 @@ 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\tdie(\"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@@ -535,6 +543,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@@ -609,6 +621,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@@ -622,6 +636,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 1947cb27cf..043d296ea4 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.0.482.g66447595a7\n\n"},{"id":"365855","messageId":"20181227162536.15895-5-pclouds@gmail.com","threadId":"50115","inReplyTo":"20181227162536.15895-1-pclouds@gmail.com","subject":"[PATCH 04/75] parse-options: disable option abbreviation with PARSE_OPT_KEEP_UNKNOWN","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-12-27T16:25:25Z","receivedAt":"2018-12-27T16:25:55Z","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>\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 4cbcefc262..81e66b9374 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -265,7 +265,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.0.482.g66447595a7\n\n"},{"id":"365856","messageId":"20181227162536.15895-4-pclouds@gmail.com","threadId":"50115","inReplyTo":"20181227162536.15895-1-pclouds@gmail.com","subject":"[PATCH 03/75] parse-options: allow keep-unknown + stop-at-non-opt combination","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-12-27T16:25:24Z","receivedAt":"2018-12-27T16:25:57Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"These two are orthogonal. One is about unknown _option_ while the\nother non-option.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n parse-options.c | 3 ---\n 1 file changed, 3 deletions(-)\n\ndiff --git a/parse-options.c b/parse-options.c\nindex d47e217b07..4cbcefc262 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -426,9 +426,6 @@ void parse_options_start(struct parse_opt_ctx_t *ctx,\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\tdie(\"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-- \n2.20.0.482.g66447595a7\n\n"},{"id":"365857","messageId":"20181227162536.15895-6-pclouds@gmail.com","threadId":"50115","inReplyTo":"20181227162536.15895-1-pclouds@gmail.com","subject":"[PATCH 05/75] parse-options: add OPT_BITOP()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-12-27T16:25:26Z","receivedAt":"2018-12-27T16:25:59Z","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>\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 81e66b9374..9e19d64cc9 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -110,6 +110,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 043d296ea4..69afd0bce6 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.0.482.g66447595a7\n\n"},{"id":"365858","messageId":"20181227162536.15895-8-pclouds@gmail.com","threadId":"50115","inReplyTo":"20181227162536.15895-1-pclouds@gmail.com","subject":"[PATCH 07/75] parse-options: avoid magic return codes","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-12-27T16:25:28Z","receivedAt":"2018-12-27T16:26:00Z","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>\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 0ad0023f97..1c8652e455 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 8d6a44a29d..720a8663eb 100644\n--- a/parse-options-cb.c\n+++ b/parse-options-cb.c\n@@ -169,10 +169,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 5a717f219f..634484d8f0 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 message = STRBUF_INIT;\n@@ -71,15 +73,15 @@ static int opt_command_mode_error(const struct option *opt,\n \t\tstrbuf_release(&that_name);\n \t\topterror(opt, message.buf, flags);\n \t\tstrbuf_release(&message);\n-\t\treturn -1;\n+\t\treturn PARSE_OPT_ERROR;\n \t}\n \treturn opterror(opt, \": incompatible with something else\", 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-                          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@@ -268,7 +272,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@@ -333,11 +337,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@@ -585,22 +589,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@@ -612,6 +622,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@@ -630,12 +646,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 277a879c1c..617bca8efe 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@@ -225,12 +225,12 @@ int opterror(const struct option *opt, const char *reason, 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.0.482.g66447595a7\n\n"},{"id":"365859","messageId":"20181227162536.15895-7-pclouds@gmail.com","threadId":"50115","inReplyTo":"20181227162536.15895-1-pclouds@gmail.com","subject":"[PATCH 06/75] parse-options: stop abusing 'callback' for lowlevel callbacks","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-12-27T16:25:27Z","receivedAt":"2018-12-27T16:26:01Z","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>\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 c3c976d471..0ad0023f97 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 8c9edce52f..8d6a44a29d 100644\n--- a/parse-options-cb.c\n+++ b/parse-options-cb.c\n@@ -169,7 +169,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 9e19d64cc9..5a717f219f 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -94,7 +94,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@@ -407,6 +407,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 69afd0bce6..277a879c1c 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@@ -266,7 +270,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.0.482.g66447595a7\n\n"},{"id":"365860","messageId":"20181227162536.15895-9-pclouds@gmail.com","threadId":"50115","inReplyTo":"20181227162536.15895-1-pclouds@gmail.com","subject":"[PATCH 08/75] parse-options: allow ll_callback with OPTION_CALLBACK","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-12-27T16:25:29Z","receivedAt":"2018-12-27T16:26:02Z","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>\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 1c8652e455..b4f4b401eb 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 720a8663eb..b336b4fa97 100644\n--- a/parse-options-cb.c\n+++ b/parse-options-cb.c\n@@ -170,8 +170,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 634484d8f0..8e0aefe86b 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -96,7 +96,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@@ -162,16 +162,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@@ -413,10 +427,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 617bca8efe..75c953a887 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@@ -270,7 +271,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.0.482.g66447595a7\n\n"},{"id":"365861","messageId":"20181227162536.15895-12-pclouds@gmail.com","threadId":"50115","inReplyTo":"20181227162536.15895-1-pclouds@gmail.com","subject":"[PATCH 11/75] diff.c: prepare to use parse_options() for parsing","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-12-27T16:25:32Z","receivedAt":"2018-12-27T16:26:04Z","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>\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 6954f4e39a..a866f13c76 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@@ -4424,6 +4425,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@@ -4465,6 +4468,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@@ -4568,6 +4573,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@@ -4858,6 +4865,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@@ -4868,6 +4885,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 b71062c2d2..8c4748ebcf 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.0.482.g66447595a7\n\n"},{"id":"365862","messageId":"20181227162536.15895-15-pclouds@gmail.com","threadId":"50115","inReplyTo":"20181227162536.15895-1-pclouds@gmail.com","subject":"[PATCH 74/75] diff --no-index: use parse_options() instead of diff_opt_parse()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-12-27T16:25:35Z","receivedAt":"2018-12-27T16:26:05Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"While at there, move exit() back to the caller. It's easier to see the\nflow that way then burying it in diff-no-index.c\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n builtin/diff.c           | 21 +++--------------\n diff-no-index.c          | 49 ++++++++++++++++++++++++----------------\n diff.h                   |  3 ++-\n t/t4053-diff-no-index.sh |  3 +--\n 4 files changed, 35 insertions(+), 41 deletions(-)\n\ndiff --git a/builtin/diff.c b/builtin/diff.c\nindex f0393bba23..52dc3e136f 100644\n--- a/builtin/diff.c\n+++ b/builtin/diff.c\n@@ -320,26 +320,11 @@ int cmd_diff(int argc, const char **argv, const char *prefix)\n \n \trepo_init_revisions(the_repository, &rev, prefix);\n \n-\tif (no_index && argc != i + 2) {\n-\t\tif (no_index == DIFF_NO_INDEX_IMPLICIT) {\n-\t\t\t/*\n-\t\t\t * There was no --no-index and there were not two\n-\t\t\t * paths. It is possible that the user intended\n-\t\t\t * to do an inside-repository operation.\n-\t\t\t */\n-\t\t\tfprintf(stderr, \"Not a git repository\\n\");\n-\t\t\tfprintf(stderr,\n-\t\t\t\t\"To compare two paths outside a working tree:\\n\");\n-\t\t}\n-\t\t/* Give the usage message for non-repository usage and exit. */\n-\t\tusagef(\"git diff %s <path> <path>\",\n-\t\t       no_index == DIFF_NO_INDEX_EXPLICIT ?\n-\t\t       \"--no-index\" : \"[--no-index]\");\n-\n-\t}\n \tif (no_index)\n \t\t/* If this is a no-index diff, just run it and exit there. */\n-\t\tdiff_no_index(the_repository, &rev, argc, argv);\n+\t\texit(diff_no_index(the_repository, &rev,\n+\t\t\t\t   no_index == DIFF_NO_INDEX_IMPLICIT,\n+\t\t\t\t   argc, argv));\n \n \t/* Otherwise, we are doing the usual \"git\" diff */\n \trev.diffopt.skip_stat_unmatch = !!diff_auto_refresh_index;\ndiff --git a/diff-no-index.c b/diff-no-index.c\nindex 9414e922d1..a879f45862 100644\n--- a/diff-no-index.c\n+++ b/diff-no-index.c\n@@ -14,6 +14,7 @@\n #include \"revision.h\"\n #include \"log-tree.h\"\n #include \"builtin.h\"\n+#include \"parse-options.h\"\n #include \"string-list.h\"\n #include \"dir.h\"\n \n@@ -233,35 +234,43 @@ static void fixup_paths(const char **path, struct strbuf *replacement)\n \t}\n }\n \n-void diff_no_index(struct repository *r,\n-\t\t   struct rev_info *revs,\n-\t\t   int argc, const char **argv)\n+static const char * const diff_no_index_usage[] = {\n+\tN_(\"git diff --no-index [<options>] <path> <path>\"),\n+\tNULL\n+};\n+\n+int diff_no_index(struct repository *r,\n+\t\t  struct rev_info *revs,\n+\t\t  int implicit_no_index,\n+\t\t  int argc, const char **argv)\n {\n-\tint i;\n+\tint i, no_index;\n \tconst char *paths[2];\n \tstruct strbuf replacement = STRBUF_INIT;\n \tconst char *prefix = revs->prefix;\n+\tstruct option no_index_options[] = {\n+\t\tOPT_BOOL_F(0, \"no-index\", &no_index, \"\",\n+\t\t\t   PARSE_OPT_NONEG | PARSE_OPT_HIDDEN),\n+\t\tOPT_END(),\n+\t};\n+\tstruct option *options;\n \n \t/*\n \t * FIXME: --no-index should not look at index and we should be\n \t * able to pass NULL repo. Maybe later.\n \t */\n \trepo_diff_setup(r, &revs->diffopt);\n-\tfor (i = 1; i < argc - 2; ) {\n-\t\tint j;\n-\t\tif (!strcmp(argv[i], \"--no-index\"))\n-\t\t\ti++;\n-\t\telse if (!strcmp(argv[i], \"--\"))\n-\t\t\ti++;\n-\t\telse {\n-\t\t\tj = diff_opt_parse(&revs->diffopt, argv + i, argc - i,\n-\t\t\t\t\t   revs->prefix);\n-\t\t\tif (j <= 0)\n-\t\t\t\tdie(\"invalid diff option/value: %s\", argv[i]);\n-\t\t\ti += j;\n-\t\t}\n+\toptions = parse_options_concat(no_index_options,\n+\t\t\t\t       revs->diffopt.parseopts);\n+\targc = parse_options(argc, argv, revs->prefix, options,\n+\t\t\t     diff_no_index_usage, 0);\n+\tif (argc != 2) {\n+\t\tif (implicit_no_index)\n+\t\t\twarning(_(\"Not a git repository. Use --no-index to \"\n+\t\t\t\t  \"compare two paths outside a working tree\"));\n+\t\tusage_with_options(diff_no_index_usage, options);\n \t}\n-\n+\tFREE_AND_NULL(options);\n \tfor (i = 0; i < 2; i++) {\n \t\tconst char *p = argv[argc - 2 + i];\n \t\tif (!strcmp(p, \"-\"))\n@@ -293,7 +302,7 @@ void diff_no_index(struct repository *r,\n \trevs->diffopt.flags.exit_with_status = 1;\n \n \tif (queue_diff(&revs->diffopt, paths[0], paths[1]))\n-\t\texit(1);\n+\t\treturn 1;\n \tdiff_set_mnemonic_prefix(&revs->diffopt, \"1/\", \"2/\");\n \tdiffcore_std(&revs->diffopt);\n \tdiff_flush(&revs->diffopt);\n@@ -304,5 +313,5 @@ void diff_no_index(struct repository *r,\n \t * The return code for --no-index imitates diff(1):\n \t * 0 = no changes, 1 = changes, else error\n \t */\n-\texit(diff_result_code(&revs->diffopt, 0));\n+\treturn diff_result_code(&revs->diffopt, 0);\n }\ndiff --git a/diff.h b/diff.h\nindex 8c4748ebcf..44843d8929 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -437,7 +437,8 @@ int diff_flush_patch_id(struct diff_options *, struct object_id *, int);\n \n int diff_result_code(struct diff_options *, int);\n \n-void diff_no_index(struct repository *, struct rev_info *, int, const char **);\n+int diff_no_index(struct repository *, struct rev_info *,\n+\t\t  int implicit_no_index, int, const char **);\n \n int index_differs_from(const char *def, const struct diff_flags *flags,\n \t\t       int ita_invisible_in_index);\ndiff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh\nindex 6e0dd6f9e5..fb25cdb789 100755\n--- a/t/t4053-diff-no-index.sh\n+++ b/t/t4053-diff-no-index.sh\n@@ -50,8 +50,7 @@ test_expect_success 'git diff --no-index executed outside repo gives correct err\n \t\texport GIT_CEILING_DIRECTORIES &&\n \t\tcd non/git &&\n \t\ttest_must_fail git diff --no-index a 2>actual.err &&\n-\t\techo \"usage: git diff --no-index <path> <path>\" >expect.err &&\n-\t\ttest_cmp expect.err actual.err\n+\t\ttest_i18ngrep \"usage: git diff --no-index\" actual.err\n \t)\n '\n \n-- \n2.20.0.482.g66447595a7\n\n"},{"id":"365863","messageId":"20181227162536.15895-11-pclouds@gmail.com","threadId":"50115","inReplyTo":"20181227162536.15895-1-pclouds@gmail.com","subject":"[PATCH 10/75] diff.h: avoid bit fields in struct diff_flags","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-12-27T16:25:31Z","receivedAt":"2018-12-27T16:26:06Z","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>\n---\n diff.h | 66 +++++++++++++++++++++++++++++-----------------------------\n 1 file changed, 33 insertions(+), 33 deletions(-)\n\ndiff --git a/diff.h b/diff.h\nindex 4b65ff739f..b71062c2d2 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.0.482.g66447595a7\n\n"},{"id":"365864","messageId":"20181227162536.15895-16-pclouds@gmail.com","threadId":"50115","inReplyTo":"20181227162536.15895-1-pclouds@gmail.com","subject":"[PATCH 75/75] am: avoid diff_opt_parse()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-12-27T16:25:36Z","receivedAt":"2018-12-27T16:26:07Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"diff_opt_parse() is a heavy hammer to just set diff filter. But it's\nthe only way because of the diff_status_letters[] mapping. Add a new\nAPI to set diff filter and use it in git-am. diff_opt_parse()'s only\nremaining call site in revision.c will be gone soon and having it here\njust because of git-am does not make sense.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n builtin/am.c | 4 ++--\n diff.c       | 6 ++++++\n diff.h       | 2 ++\n 3 files changed, 10 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/am.c b/builtin/am.c\nindex 8f27f3375b..a98e65f440 100644\n--- a/builtin/am.c\n+++ b/builtin/am.c\n@@ -1515,11 +1515,11 @@ static int fall_back_threeway(const struct am_state *state, const char *index_pa\n \t\t * review them with extra care to spot mismerges.\n \t\t */\n \t\tstruct rev_info rev_info;\n-\t\tconst char *diff_filter_str = \"--diff-filter=AM\";\n \n \t\trepo_init_revisions(the_repository, &rev_info, NULL);\n \t\trev_info.diffopt.output_format = DIFF_FORMAT_NAME_STATUS;\n-\t\tdiff_opt_parse(&rev_info.diffopt, &diff_filter_str, 1, rev_info.prefix);\n+\t\trev_info.diffopt.filter |= diff_filter_bit('A');\n+\t\trev_info.diffopt.filter |= diff_filter_bit('M');\n \t\tadd_pending_oid(&rev_info, \"HEAD\", &our_tree, 0);\n \t\tdiff_setup_done(&rev_info.diffopt);\n \t\trun_diff_index(&rev_info, 1);\ndiff --git a/diff.c b/diff.c\nindex 43ae8d4d2c..8a7011b928 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -4756,6 +4756,12 @@ static unsigned filter_bit_tst(char status, const struct diff_options *opt)\n \treturn opt->filter & filter_bit[(int) status];\n }\n \n+unsigned diff_filter_bit(char status)\n+{\n+\tprepare_filter_bits();\n+\treturn filter_bit[(int) status];\n+}\n+\n static int diff_opt_diff_filter(const struct option *option,\n \t\t\t\tconst char *optarg, int unset)\n {\ndiff --git a/diff.h b/diff.h\nindex 44843d8929..48cfa3f0a6 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -233,6 +233,8 @@ struct diff_options {\n \tstruct option *parseopts;\n };\n \n+unsigned diff_filter_bit(char status);\n+\n void diff_emit_submodule_del(struct diff_options *o, const char *line);\n void diff_emit_submodule_add(struct diff_options *o, const char *line);\n void diff_emit_submodule_untracked(struct diff_options *o, const char *path);\n-- \n2.20.0.482.g66447595a7\n\n"},{"id":"365865","messageId":"20181227162536.15895-14-pclouds@gmail.com","threadId":"50115","inReplyTo":"20181227162536.15895-1-pclouds@gmail.com","subject":"[PATCH 73/75] range-diff: use parse_options() instead of diff_opt_parse()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-12-27T16:25:34Z","receivedAt":"2018-12-27T16:26:10Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Diff's internal option parsing is now done with 'struct option', which\nmakes it possible to combine all diff options to range-diff and parse\neverything all at once. Parsing code becomes simpler, and we get a\nlooong 'git range-diff -h'\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n builtin/range-diff.c | 26 ++++++--------------------\n 1 file changed, 6 insertions(+), 20 deletions(-)\n\ndiff --git a/builtin/range-diff.c b/builtin/range-diff.c\nindex f01a0be851..784bd19321 100644\n--- a/builtin/range-diff.c\n+++ b/builtin/range-diff.c\n@@ -16,42 +16,27 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)\n \tint creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT;\n \tstruct diff_options diffopt = { NULL };\n \tint simple_color = -1;\n-\tstruct option options[] = {\n+\tstruct option range_diff_options[] = {\n \t\tOPT_INTEGER(0, \"creation-factor\", &creation_factor,\n \t\t\t    N_(\"Percentage by which creation is weighted\")),\n \t\tOPT_BOOL(0, \"no-dual-color\", &simple_color,\n \t\t\t    N_(\"use simple diff colors\")),\n \t\tOPT_END()\n \t};\n-\tint i, j, res = 0;\n+\tstruct option *options;\n+\tint res = 0;\n \tstruct strbuf range1 = STRBUF_INIT, range2 = STRBUF_INIT;\n \n \tgit_config(git_diff_ui_config, NULL);\n \n \trepo_diff_setup(the_repository, &diffopt);\n \n+\toptions = parse_options_concat(range_diff_options, diffopt.parseopts);\n \targc = parse_options(argc, argv, NULL, options,\n-\t\t\t     builtin_range_diff_usage, PARSE_OPT_KEEP_UNKNOWN |\n-\t\t\t     PARSE_OPT_KEEP_DASHDASH | PARSE_OPT_KEEP_ARGV0);\n-\n-\tfor (i = j = 1; i < argc && strcmp(\"--\", argv[i]); ) {\n-\t\tint c = diff_opt_parse(&diffopt, argv + i, argc - i, prefix);\n+\t\t\t     builtin_range_diff_usage, 0);\n \n-\t\tif (!c)\n-\t\t\targv[j++] = argv[i++];\n-\t\telse\n-\t\t\ti += c;\n-\t}\n-\twhile (i < argc)\n-\t\targv[j++] = argv[i++];\n-\targc = j;\n \tdiff_setup_done(&diffopt);\n \n-\t/* Make sure that there are no unparsed options */\n-\targc = parse_options(argc, argv, NULL,\n-\t\t\t     options + ARRAY_SIZE(options) - 1, /* OPT_END */\n-\t\t\t     builtin_range_diff_usage, 0);\n-\n \t/* force color when --dual-color was used */\n \tif (!simple_color)\n \t\tdiffopt.use_color = 1;\n@@ -90,6 +75,7 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)\n \t\terror(_(\"need two commit ranges\"));\n \t\tusage_with_options(builtin_range_diff_usage, options);\n \t}\n+\tFREE_AND_NULL(options);\n \n \tres = show_range_diff(range1.buf, range2.buf, creation_factor,\n \t\t\t      simple_color < 1, &diffopt);\n-- \n2.20.0.482.g66447595a7\n\n"},{"id":"365866","messageId":"20181227162536.15895-10-pclouds@gmail.com","threadId":"50115","inReplyTo":"20181227162536.15895-1-pclouds@gmail.com","subject":"[PATCH 09/75] diff.h: keep forward struct declarations sorted","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-12-27T16:25:30Z","receivedAt":"2018-12-27T16:26:12Z","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>\n---\n diff.h | 12 ++++++------\n 1 file changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/diff.h b/diff.h\nindex 9e8061ca29..4b65ff739f 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.0.482.g66447595a7\n\n"},{"id":"365867","messageId":"20181227162536.15895-13-pclouds@gmail.com","threadId":"50115","inReplyTo":"20181227162536.15895-1-pclouds@gmail.com","subject":"[PATCH 12/75] diff.c: convert -u|-p|--patch","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2018-12-27T16:25:33Z","receivedAt":"2018-12-27T16:26:12Z","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>\n---\n diff.c | 10 ++++++++--\n 1 file changed, 8 insertions(+), 2 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex a866f13c76..59132ceaff 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -4868,6 +4868,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@@ -4896,8 +4903,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.0.482.g66447595a7\n\n"},{"id":"365883","messageId":"CAPig+cR5xo75B3sjHWjh+2Z5Wn+CTt-S2mcd0BrdfyVV=yv0Bg@mail.gmail.com","threadId":"50115","inReplyTo":"20181227162536.15895-15-pclouds@gmail.com","subject":"Re: [PATCH 74/75] diff --no-index: use parse_options() instead of diff_opt_parse()","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-12-27T20:34:00Z","receivedAt":"2018-12-27T20:34:13Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Dec 27, 2018 at 11:26 AM Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:\n> While at there, move exit() back to the caller. It's easier to see the\n> flow that way then burying it in diff-no-index.c\n\ns/then/than/\n\n> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n"},{"id":"366446","messageId":"CAGZ79kagNUFYhe_juPrjWgGHGPdkow2+LjrUdXR84oNToh69kg@mail.gmail.com","threadId":"50115","inReplyTo":"20181227162536.15895-11-pclouds@gmail.com","subject":"Re: [PATCH 10/75] diff.h: avoid bit fields in struct diff_flags","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2019-01-09T20:21:54Z","receivedAt":"2019-01-09T20:22:08Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Dec 27, 2018 at 8:26 AM Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:\n>\n> Bitfield addresses cannot be passed around in a pointer. This makes it\n> hard to use parse-options to set/unset them. Turn this struct to\n> normal integers. This of course increases the size of this struct\n> multiple times, but since we only have a handful of diff_options\n> variables around, memory consumption is not at all a concern.\n\nMakes sense, and continues\n8cc633286a (Merge branch 'bw/diff-opt-impl-to-bitfields', 2017-11-09)\n"}]}