* [PATCH 01/75] parse-options.h: remove extern on function prototypes
2018-12-27 16:25 [PATCH 00/75] Convert diff opt parser to parse_options() Nguyễn Thái Ngọc Duy
@ 2018-12-27 16:25 ` Nguyễn Thái Ngọc Duy
2018-12-27 16:25 ` [PATCH 02/75] parse-options: add one-shot mode Nguyễn Thái Ngọc Duy
` (13 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-12-27 16:25 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
parse-options.h | 60 ++++++++++++++++++++++++-------------------------
1 file changed, 30 insertions(+), 30 deletions(-)
diff --git a/parse-options.h b/parse-options.h
index a650a7d220..1947cb27cf 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -174,19 +174,19 @@ struct option {
* for translation with N_().
* Returns the number of arguments left in argv[].
*/
-extern int parse_options(int argc, const char **argv, const char *prefix,
- const struct option *options,
- const char * const usagestr[], int flags);
+int parse_options(int argc, const char **argv, const char *prefix,
+ const struct option *options,
+ const char * const usagestr[], int flags);
-extern NORETURN void usage_with_options(const char * const *usagestr,
- const struct option *options);
+NORETURN void usage_with_options(const char * const *usagestr,
+ const struct option *options);
-extern NORETURN void usage_msg_opt(const char *msg,
- const char * const *usagestr,
- const struct option *options);
+NORETURN void usage_msg_opt(const char *msg,
+ const char * const *usagestr,
+ const struct option *options);
-extern int optbug(const struct option *opt, const char *reason);
-extern int opterror(const struct option *opt, const char *reason, int flags);
+int optbug(const struct option *opt, const char *reason);
+int opterror(const struct option *opt, const char *reason, int flags);
#if defined(__GNUC__)
#define opterror(o,r,f) (opterror((o),(r),(f)), const_error())
#endif
@@ -230,31 +230,31 @@ struct parse_opt_ctx_t {
const char *prefix;
};
-extern void parse_options_start(struct parse_opt_ctx_t *ctx,
- int argc, const char **argv, const char *prefix,
- const struct option *options, int flags);
+void parse_options_start(struct parse_opt_ctx_t *ctx,
+ int argc, const char **argv, const char *prefix,
+ const struct option *options, int flags);
-extern int parse_options_step(struct parse_opt_ctx_t *ctx,
- const struct option *options,
- const char * const usagestr[]);
+int parse_options_step(struct parse_opt_ctx_t *ctx,
+ const struct option *options,
+ const char * const usagestr[]);
-extern int parse_options_end(struct parse_opt_ctx_t *ctx);
+int parse_options_end(struct parse_opt_ctx_t *ctx);
-extern struct option *parse_options_concat(struct option *a, struct option *b);
+struct option *parse_options_concat(struct option *a, struct option *b);
/*----- some often used options -----*/
-extern int parse_opt_abbrev_cb(const struct option *, const char *, int);
-extern int parse_opt_expiry_date_cb(const struct option *, const char *, int);
-extern int parse_opt_color_flag_cb(const struct option *, const char *, int);
-extern int parse_opt_verbosity_cb(const struct option *, const char *, int);
-extern int parse_opt_object_name(const struct option *, const char *, int);
-extern int parse_opt_commits(const struct option *, const char *, int);
-extern int parse_opt_tertiary(const struct option *, const char *, int);
-extern int parse_opt_string_list(const struct option *, const char *, int);
-extern int parse_opt_noop_cb(const struct option *, const char *, int);
-extern int parse_opt_unknown_cb(const struct option *, const char *, int);
-extern int parse_opt_passthru(const struct option *, const char *, int);
-extern int parse_opt_passthru_argv(const struct option *, const char *, int);
+int parse_opt_abbrev_cb(const struct option *, const char *, int);
+int parse_opt_expiry_date_cb(const struct option *, const char *, int);
+int parse_opt_color_flag_cb(const struct option *, const char *, int);
+int parse_opt_verbosity_cb(const struct option *, const char *, int);
+int parse_opt_object_name(const struct option *, const char *, int);
+int parse_opt_commits(const struct option *, const char *, int);
+int parse_opt_tertiary(const struct option *, const char *, int);
+int parse_opt_string_list(const struct option *, const char *, int);
+int parse_opt_noop_cb(const struct option *, const char *, int);
+int parse_opt_unknown_cb(const struct option *, const char *, int);
+int parse_opt_passthru(const struct option *, const char *, int);
+int parse_opt_passthru_argv(const struct option *, const char *, int);
#define OPT__VERBOSE(var, h) OPT_COUNTUP('v', "verbose", (var), (h))
#define OPT__QUIET(var, h) OPT_COUNTUP('q', "quiet", (var), (h))
--
2.20.0.482.g66447595a7
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 02/75] parse-options: add one-shot mode
2018-12-27 16:25 [PATCH 00/75] Convert diff opt parser to parse_options() Nguyễn Thái Ngọc Duy
2018-12-27 16:25 ` [PATCH 01/75] parse-options.h: remove extern on function prototypes Nguyễn Thái Ngọc Duy
@ 2018-12-27 16:25 ` Nguyễn Thái Ngọc Duy
2018-12-27 16:25 ` [PATCH 03/75] parse-options: allow keep-unknown + stop-at-non-opt combination Nguyễn Thái Ngọc Duy
` (12 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-12-27 16:25 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
This is to help reimplement diff_opt_parse() using parse_options().
The behavior of parse_options() is changed to be the same as the
other:
- no argv0 in argv[], everything can be processed
- argv[] must not be updated, it's the caller's job to do that
- return the number of arguments processed
- leave all unknown options / non-options alone (this one can already
be achieved with PARSE_OPT_KEEP_UNKNOWN and
PARSE_OPT_STOP_AT_NON_OPTION)
This mode is NOT supposed to stay here for long. It's to help
converting diff/rev option parsing. Once that work is over and we can
just use parse_options() throughout the code base, this will be
deleted.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
parse-options.c | 23 ++++++++++++++++++++---
parse-options.h | 17 +++++++++++++----
2 files changed, 33 insertions(+), 7 deletions(-)
diff --git a/parse-options.c b/parse-options.c
index 6932eaff61..d47e217b07 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -415,15 +415,23 @@ void parse_options_start(struct parse_opt_ctx_t *ctx,
const struct option *options, int flags)
{
memset(ctx, 0, sizeof(*ctx));
- ctx->argc = ctx->total = argc - 1;
- ctx->argv = argv + 1;
- ctx->out = argv;
+ ctx->argc = argc;
+ ctx->argv = argv;
+ if (!(flags & PARSE_OPT_ONE_SHOT)) {
+ ctx->argc--;
+ ctx->argv++;
+ }
+ ctx->total = ctx->argc;
+ ctx->out = argv;
ctx->prefix = prefix;
ctx->cpidx = ((flags & PARSE_OPT_KEEP_ARGV0) != 0);
ctx->flags = flags;
if ((flags & PARSE_OPT_KEEP_UNKNOWN) &&
(flags & PARSE_OPT_STOP_AT_NON_OPTION))
die("STOP_AT_NON_OPTION and KEEP_UNKNOWN don't go together");
+ if ((flags & PARSE_OPT_ONE_SHOT) &&
+ (flags & PARSE_OPT_KEEP_ARGV0))
+ BUG("Can't keep argv0 if you don't have it");
parse_options_check(options);
}
@@ -535,6 +543,10 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
for (; ctx->argc; ctx->argc--, ctx->argv++) {
const char *arg = ctx->argv[0];
+ if (ctx->flags & PARSE_OPT_ONE_SHOT &&
+ ctx->argc != ctx->total)
+ break;
+
if (*arg != '-' || !arg[1]) {
if (parse_nodash_opt(ctx, arg, options) == 0)
continue;
@@ -609,6 +621,8 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
}
continue;
unknown:
+ if (ctx->flags & PARSE_OPT_ONE_SHOT)
+ break;
if (!(ctx->flags & PARSE_OPT_KEEP_UNKNOWN))
return PARSE_OPT_UNKNOWN;
ctx->out[ctx->cpidx++] = ctx->argv[0];
@@ -622,6 +636,9 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
int parse_options_end(struct parse_opt_ctx_t *ctx)
{
+ if (ctx->flags & PARSE_OPT_ONE_SHOT)
+ return ctx->total - ctx->argc;
+
MOVE_ARRAY(ctx->out + ctx->cpidx, ctx->argv, ctx->argc);
ctx->out[ctx->cpidx + ctx->argc] = NULL;
return ctx->cpidx + ctx->argc;
diff --git a/parse-options.h b/parse-options.h
index 1947cb27cf..043d296ea4 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -27,7 +27,8 @@ enum parse_opt_flags {
PARSE_OPT_STOP_AT_NON_OPTION = 2,
PARSE_OPT_KEEP_ARGV0 = 4,
PARSE_OPT_KEEP_UNKNOWN = 8,
- PARSE_OPT_NO_INTERNAL_HELP = 16
+ PARSE_OPT_NO_INTERNAL_HELP = 16,
+ PARSE_OPT_ONE_SHOT = 32
};
enum parse_opt_option_flags {
@@ -169,10 +170,18 @@ struct option {
N_("no-op (backward compatibility)"), \
PARSE_OPT_HIDDEN | PARSE_OPT_NOARG, parse_opt_noop_cb }
-/* parse_options() will filter out the processed options and leave the
- * non-option arguments in argv[]. usagestr strings should be marked
- * for translation with N_().
+/*
+ * parse_options() will filter out the processed options and leave the
+ * non-option arguments in argv[]. argv0 is assumed program name and
+ * skipped.
+ *
+ * usagestr strings should be marked for translation with N_().
+ *
* Returns the number of arguments left in argv[].
+ *
+ * In one-shot mode, argv0 is not a program name, argv[] is left
+ * untouched and parse_options() returns the number of options
+ * processed.
*/
int parse_options(int argc, const char **argv, const char *prefix,
const struct option *options,
--
2.20.0.482.g66447595a7
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 03/75] parse-options: allow keep-unknown + stop-at-non-opt combination
2018-12-27 16:25 [PATCH 00/75] Convert diff opt parser to parse_options() Nguyễn Thái Ngọc Duy
2018-12-27 16:25 ` [PATCH 01/75] parse-options.h: remove extern on function prototypes Nguyễn Thái Ngọc Duy
2018-12-27 16:25 ` [PATCH 02/75] parse-options: add one-shot mode Nguyễn Thái Ngọc Duy
@ 2018-12-27 16:25 ` Nguyễn Thái Ngọc Duy
2018-12-27 16:25 ` [PATCH 04/75] parse-options: disable option abbreviation with PARSE_OPT_KEEP_UNKNOWN Nguyễn Thái Ngọc Duy
` (11 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-12-27 16:25 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
These two are orthogonal. One is about unknown _option_ while the
other non-option.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
parse-options.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/parse-options.c b/parse-options.c
index d47e217b07..4cbcefc262 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -426,9 +426,6 @@ void parse_options_start(struct parse_opt_ctx_t *ctx,
ctx->prefix = prefix;
ctx->cpidx = ((flags & PARSE_OPT_KEEP_ARGV0) != 0);
ctx->flags = flags;
- if ((flags & PARSE_OPT_KEEP_UNKNOWN) &&
- (flags & PARSE_OPT_STOP_AT_NON_OPTION))
- die("STOP_AT_NON_OPTION and KEEP_UNKNOWN don't go together");
if ((flags & PARSE_OPT_ONE_SHOT) &&
(flags & PARSE_OPT_KEEP_ARGV0))
BUG("Can't keep argv0 if you don't have it");
--
2.20.0.482.g66447595a7
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 04/75] parse-options: disable option abbreviation with PARSE_OPT_KEEP_UNKNOWN
2018-12-27 16:25 [PATCH 00/75] Convert diff opt parser to parse_options() Nguyễn Thái Ngọc Duy
` (2 preceding siblings ...)
2018-12-27 16:25 ` [PATCH 03/75] parse-options: allow keep-unknown + stop-at-non-opt combination Nguyễn Thái Ngọc Duy
@ 2018-12-27 16:25 ` Nguyễn Thái Ngọc Duy
2018-12-27 16:25 ` [PATCH 05/75] parse-options: add OPT_BITOP() Nguyễn Thái Ngọc Duy
` (10 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-12-27 16:25 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
parse-options can unambiguously find an abbreviation only if it sees
all available options. This is usually the case when you use
parse_options(). But there are other callers like blame or shortlog
which uses parse_options_start() in combination with a custom option
parser, like rev-list. parse-options cannot see all options in this
case and will get abbrev detection wrong. Disable it.
t7800 needs update because --symlink no longer expands to --symlinks
and will be passed down to git-diff, which will not recognize it. I
still think this is the correct thing to do. But if --symlink has been
actually used in the wild, we would just add an option alias for it.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
parse-options.c | 3 ++-
t/t7800-difftool.sh | 4 ++--
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/parse-options.c b/parse-options.c
index 4cbcefc262..81e66b9374 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -265,7 +265,8 @@ static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg,
}
if (!rest) {
/* abbreviated? */
- if (!strncmp(long_name, arg, arg_end - arg)) {
+ if (!(p->flags & PARSE_OPT_KEEP_UNKNOWN) &&
+ !strncmp(long_name, arg, arg_end - arg)) {
is_abbreviated:
if (abbrev_option) {
/*
diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index 22b9199d59..bb9a7f4ff9 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -546,7 +546,7 @@ do
done >actual
EOF
-test_expect_success SYMLINKS 'difftool --dir-diff --symlink without unstaged changes' '
+test_expect_success SYMLINKS 'difftool --dir-diff --symlinks without unstaged changes' '
cat >expect <<-EOF &&
file
$PWD/file
@@ -555,7 +555,7 @@ test_expect_success SYMLINKS 'difftool --dir-diff --symlink without unstaged cha
sub/sub
$PWD/sub/sub
EOF
- git difftool --dir-diff --symlink \
+ git difftool --dir-diff --symlinks \
--extcmd "./.git/CHECK_SYMLINKS" branch HEAD &&
test_cmp expect actual
'
--
2.20.0.482.g66447595a7
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 05/75] parse-options: add OPT_BITOP()
2018-12-27 16:25 [PATCH 00/75] Convert diff opt parser to parse_options() Nguyễn Thái Ngọc Duy
` (3 preceding siblings ...)
2018-12-27 16:25 ` [PATCH 04/75] parse-options: disable option abbreviation with PARSE_OPT_KEEP_UNKNOWN Nguyễn Thái Ngọc Duy
@ 2018-12-27 16:25 ` Nguyễn Thái Ngọc Duy
2018-12-27 16:25 ` [PATCH 06/75] parse-options: stop abusing 'callback' for lowlevel callbacks Nguyễn Thái Ngọc Duy
` (9 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-12-27 16:25 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
This is needed for diff_opt_parse() where we do
value = (value & ~mask) | some_more;
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
parse-options.c | 7 +++++++
parse-options.h | 5 +++++
2 files changed, 12 insertions(+)
diff --git a/parse-options.c b/parse-options.c
index 81e66b9374..9e19d64cc9 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -110,6 +110,13 @@ static int get_value(struct parse_opt_ctx_t *p,
*(int *)opt->value &= ~opt->defval;
return 0;
+ case OPTION_BITOP:
+ if (unset)
+ BUG("BITOP can't have unset form");
+ *(int *)opt->value &= ~opt->extra;
+ *(int *)opt->value |= opt->defval;
+ return 0;
+
case OPTION_COUNTUP:
if (*(int *)opt->value < 0)
*(int *)opt->value = 0;
diff --git a/parse-options.h b/parse-options.h
index 043d296ea4..69afd0bce6 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -10,6 +10,7 @@ enum parse_opt_type {
/* options with no arguments */
OPTION_BIT,
OPTION_NEGBIT,
+ OPTION_BITOP,
OPTION_COUNTUP,
OPTION_SET_INT,
OPTION_CMDMODE,
@@ -118,6 +119,7 @@ struct option {
int flags;
parse_opt_cb *callback;
intptr_t defval;
+ intptr_t extra;
};
#define OPT_BIT_F(s, l, v, h, b, f) { OPTION_BIT, (s), (l), (v), NULL, (h), \
@@ -133,6 +135,9 @@ struct option {
(h), PARSE_OPT_NOARG}
#define OPT_GROUP(h) { OPTION_GROUP, 0, NULL, NULL, NULL, (h) }
#define OPT_BIT(s, l, v, h, b) OPT_BIT_F(s, l, v, h, b, 0)
+#define OPT_BITOP(s, l, v, h, set, clear) { OPTION_BITOP, (s), (l), (v), NULL, (h), \
+ PARSE_OPT_NOARG|PARSE_OPT_NONEG, NULL, \
+ (set), (clear) }
#define OPT_NEGBIT(s, l, v, h, b) { OPTION_NEGBIT, (s), (l), (v), NULL, \
(h), PARSE_OPT_NOARG, NULL, (b) }
#define OPT_COUNTUP(s, l, v, h) OPT_COUNTUP_F(s, l, v, h, 0)
--
2.20.0.482.g66447595a7
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 06/75] parse-options: stop abusing 'callback' for lowlevel callbacks
2018-12-27 16:25 [PATCH 00/75] Convert diff opt parser to parse_options() Nguyễn Thái Ngọc Duy
` (4 preceding siblings ...)
2018-12-27 16:25 ` [PATCH 05/75] parse-options: add OPT_BITOP() Nguyễn Thái Ngọc Duy
@ 2018-12-27 16:25 ` Nguyễn Thái Ngọc Duy
2018-12-27 16:25 ` [PATCH 07/75] parse-options: avoid magic return codes Nguyễn Thái Ngọc Duy
` (8 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-12-27 16:25 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
Lowlevel callbacks have different function signatures. Add a new field
in 'struct option' with the right type for lowlevel callbacks.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/blame.c | 2 +-
builtin/merge.c | 2 +-
builtin/update-index.c | 11 ++++++-----
parse-options-cb.c | 3 ++-
parse-options.c | 15 ++++++++++++++-
parse-options.h | 12 ++++++++----
6 files changed, 32 insertions(+), 13 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index 6d798f9939..8dcc55dffa 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -814,7 +814,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
* and are only included here to get included in the "-h"
* output:
*/
- { OPTION_LOWLEVEL_CALLBACK, 0, "indent-heuristic", NULL, NULL, N_("Use an experimental heuristic to improve diffs"), PARSE_OPT_NOARG, parse_opt_unknown_cb },
+ { 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 },
OPT_BIT(0, "minimal", &xdl_opts, N_("Spend extra cycles to find better match"), XDF_NEED_MINIMAL),
OPT_STRING('S', NULL, &revs_file, N_("file"), N_("Use revisions from <file> instead of calling git-rev-list")),
diff --git a/builtin/merge.c b/builtin/merge.c
index c3c976d471..0ad0023f97 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -261,7 +261,7 @@ static struct option builtin_merge_options[] = {
option_parse_message),
{ OPTION_LOWLEVEL_CALLBACK, 'F', "file", &merge_msg, N_("path"),
N_("read message from file"), PARSE_OPT_NONEG,
- (parse_opt_cb *) option_read_message },
+ NULL, 0, option_read_message },
OPT__VERBOSITY(&verbosity),
OPT_BOOL(0, "abort", &abort_current_merge,
N_("abort the current in-progress merge")),
diff --git a/builtin/update-index.c b/builtin/update-index.c
index e19da77edc..727a8118b8 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -985,7 +985,8 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
N_("add the specified entry to the index"),
PARSE_OPT_NOARG | /* disallow --cacheinfo=<mode> form */
PARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP,
- (parse_opt_cb *) cacheinfo_callback},
+ NULL, 0,
+ cacheinfo_callback},
{OPTION_CALLBACK, 0, "chmod", &set_executable_bit, "(+|-)x",
N_("override the executable bit of the listed files"),
PARSE_OPT_NONEG,
@@ -1011,19 +1012,19 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
{OPTION_LOWLEVEL_CALLBACK, 0, "stdin", &read_from_stdin, NULL,
N_("read list of paths to be updated from standard input"),
PARSE_OPT_NONEG | PARSE_OPT_NOARG,
- (parse_opt_cb *) stdin_callback},
+ NULL, 0, stdin_callback},
{OPTION_LOWLEVEL_CALLBACK, 0, "index-info", &nul_term_line, NULL,
N_("add entries from standard input to the index"),
PARSE_OPT_NONEG | PARSE_OPT_NOARG,
- (parse_opt_cb *) stdin_cacheinfo_callback},
+ NULL, 0, stdin_cacheinfo_callback},
{OPTION_LOWLEVEL_CALLBACK, 0, "unresolve", &has_errors, NULL,
N_("repopulate stages #2 and #3 for the listed paths"),
PARSE_OPT_NONEG | PARSE_OPT_NOARG,
- (parse_opt_cb *) unresolve_callback},
+ NULL, 0, unresolve_callback},
{OPTION_LOWLEVEL_CALLBACK, 'g', "again", &has_errors, NULL,
N_("only update entries that differ from HEAD"),
PARSE_OPT_NONEG | PARSE_OPT_NOARG,
- (parse_opt_cb *) reupdate_callback},
+ NULL, 0, reupdate_callback},
OPT_BIT(0, "ignore-missing", &refresh_args.flags,
N_("ignore files missing from worktree"),
REFRESH_IGNORE_MISSING),
diff --git a/parse-options-cb.c b/parse-options-cb.c
index 8c9edce52f..8d6a44a29d 100644
--- a/parse-options-cb.c
+++ b/parse-options-cb.c
@@ -169,7 +169,8 @@ int parse_opt_noop_cb(const struct option *opt, const char *arg, int unset)
* "-h" output even if it's not being handled directly by
* parse_options().
*/
-int parse_opt_unknown_cb(const struct option *opt, const char *arg, int unset)
+int parse_opt_unknown_cb(struct parse_opt_ctx_t *ctx,
+ const struct option *opt, int unset)
{
return -2;
}
diff --git a/parse-options.c b/parse-options.c
index 9e19d64cc9..5a717f219f 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -94,7 +94,7 @@ static int get_value(struct parse_opt_ctx_t *p,
switch (opt->type) {
case OPTION_LOWLEVEL_CALLBACK:
- return (*(parse_opt_ll_cb *)opt->callback)(p, opt, unset);
+ return opt->ll_callback(p, opt, unset);
case OPTION_BIT:
if (unset)
@@ -407,6 +407,19 @@ static void parse_options_check(const struct option *opts)
if ((opts->flags & PARSE_OPT_OPTARG) ||
!(opts->flags & PARSE_OPT_NOARG))
err |= optbug(opts, "should not accept an argument");
+ break;
+ case OPTION_CALLBACK:
+ if (!opts->callback)
+ BUG("OPTION_CALLBACK needs a callback");
+ if (opts->ll_callback)
+ BUG("OPTION_CALLBACK needs no ll_callback");
+ break;
+ case OPTION_LOWLEVEL_CALLBACK:
+ if (!opts->ll_callback)
+ BUG("OPTION_LOWLEVEL_CALLBACK needs a callback");
+ if (opts->callback)
+ BUG("OPTION_LOWLEVEL_CALLBACK needs no high level callback");
+ break;
default:
; /* ok. (usually accepts an argument) */
}
diff --git a/parse-options.h b/parse-options.h
index 69afd0bce6..277a879c1c 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -100,13 +100,16 @@ typedef int parse_opt_ll_cb(struct parse_opt_ctx_t *ctx,
* the option takes optional argument.
*
* `callback`::
- * pointer to the callback to use for OPTION_CALLBACK or
- * OPTION_LOWLEVEL_CALLBACK.
+ * pointer to the callback to use for OPTION_CALLBACK
*
* `defval`::
* default value to fill (*->value) with for PARSE_OPT_OPTARG.
* OPTION_{BIT,SET_INT} store the {mask,integer} to put in the value when met.
* CALLBACKS can use it like they want.
+ *
+ * `ll_callback`::
+ * pointer to the callback to use for OPTION_LOWLEVEL_CALLBACK
+ *
*/
struct option {
enum parse_opt_type type;
@@ -119,6 +122,7 @@ struct option {
int flags;
parse_opt_cb *callback;
intptr_t defval;
+ parse_opt_ll_cb *ll_callback;
intptr_t extra;
};
@@ -137,7 +141,7 @@ struct option {
#define OPT_BIT(s, l, v, h, b) OPT_BIT_F(s, l, v, h, b, 0)
#define OPT_BITOP(s, l, v, h, set, clear) { OPTION_BITOP, (s), (l), (v), NULL, (h), \
PARSE_OPT_NOARG|PARSE_OPT_NONEG, NULL, \
- (set), (clear) }
+ (set), NULL, (clear) }
#define OPT_NEGBIT(s, l, v, h, b) { OPTION_NEGBIT, (s), (l), (v), NULL, \
(h), PARSE_OPT_NOARG, NULL, (b) }
#define OPT_COUNTUP(s, l, v, h) OPT_COUNTUP_F(s, l, v, h, 0)
@@ -266,7 +270,7 @@ int parse_opt_commits(const struct option *, const char *, int);
int parse_opt_tertiary(const struct option *, const char *, int);
int parse_opt_string_list(const struct option *, const char *, int);
int parse_opt_noop_cb(const struct option *, const char *, int);
-int parse_opt_unknown_cb(const struct option *, const char *, int);
+int parse_opt_unknown_cb(struct parse_opt_ctx_t *ctx, const struct option *, int);
int parse_opt_passthru(const struct option *, const char *, int);
int parse_opt_passthru_argv(const struct option *, const char *, int);
--
2.20.0.482.g66447595a7
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 07/75] parse-options: avoid magic return codes
2018-12-27 16:25 [PATCH 00/75] Convert diff opt parser to parse_options() Nguyễn Thái Ngọc Duy
` (5 preceding siblings ...)
2018-12-27 16:25 ` [PATCH 06/75] parse-options: stop abusing 'callback' for lowlevel callbacks Nguyễn Thái Ngọc Duy
@ 2018-12-27 16:25 ` Nguyễn Thái Ngọc Duy
2018-12-27 16:25 ` [PATCH 08/75] parse-options: allow ll_callback with OPTION_CALLBACK Nguyễn Thái Ngọc Duy
` (7 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-12-27 16:25 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
Give names to these magic negative numbers. Make parse_opt_ll_cb
return an enum to make clear it can actually control parse_options()
with different return values (parse_opt_cb can too, but nobody needs
it).
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/merge.c | 5 +--
builtin/update-index.c | 20 ++++++------
parse-options-cb.c | 6 ++--
parse-options.c | 69 +++++++++++++++++++++++++++---------------
parse-options.h | 14 ++++-----
5 files changed, 68 insertions(+), 46 deletions(-)
diff --git a/builtin/merge.c b/builtin/merge.c
index 0ad0023f97..1c8652e455 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -112,8 +112,9 @@ static int option_parse_message(const struct option *opt,
return 0;
}
-static int option_read_message(struct parse_opt_ctx_t *ctx,
- const struct option *opt, int unset)
+static enum parse_opt_result option_read_message(struct parse_opt_ctx_t *ctx,
+ const struct option *opt,
+ int unset)
{
struct strbuf *buf = opt->value;
const char *arg;
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 727a8118b8..21c84e5590 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -847,8 +847,8 @@ static int parse_new_style_cacheinfo(const char *arg,
return 0;
}
-static int cacheinfo_callback(struct parse_opt_ctx_t *ctx,
- const struct option *opt, int unset)
+static enum parse_opt_result cacheinfo_callback(
+ struct parse_opt_ctx_t *ctx, const struct option *opt, int unset)
{
struct object_id oid;
unsigned int mode;
@@ -873,8 +873,8 @@ static int cacheinfo_callback(struct parse_opt_ctx_t *ctx,
return 0;
}
-static int stdin_cacheinfo_callback(struct parse_opt_ctx_t *ctx,
- const struct option *opt, int unset)
+static enum parse_opt_result stdin_cacheinfo_callback(
+ struct parse_opt_ctx_t *ctx, const struct option *opt, int unset)
{
int *nul_term_line = opt->value;
@@ -887,8 +887,8 @@ static int stdin_cacheinfo_callback(struct parse_opt_ctx_t *ctx,
return 0;
}
-static int stdin_callback(struct parse_opt_ctx_t *ctx,
- const struct option *opt, int unset)
+static enum parse_opt_result stdin_callback(
+ struct parse_opt_ctx_t *ctx, const struct option *opt, int unset)
{
int *read_from_stdin = opt->value;
@@ -900,8 +900,8 @@ static int stdin_callback(struct parse_opt_ctx_t *ctx,
return 0;
}
-static int unresolve_callback(struct parse_opt_ctx_t *ctx,
- const struct option *opt, int unset)
+static enum parse_opt_result unresolve_callback(
+ struct parse_opt_ctx_t *ctx, const struct option *opt, int unset)
{
int *has_errors = opt->value;
const char *prefix = startup_info->prefix;
@@ -919,8 +919,8 @@ static int unresolve_callback(struct parse_opt_ctx_t *ctx,
return 0;
}
-static int reupdate_callback(struct parse_opt_ctx_t *ctx,
- const struct option *opt, int unset)
+static enum parse_opt_result reupdate_callback(
+ struct parse_opt_ctx_t *ctx, const struct option *opt, int unset)
{
int *has_errors = opt->value;
const char *prefix = startup_info->prefix;
diff --git a/parse-options-cb.c b/parse-options-cb.c
index 8d6a44a29d..720a8663eb 100644
--- a/parse-options-cb.c
+++ b/parse-options-cb.c
@@ -169,10 +169,10 @@ int parse_opt_noop_cb(const struct option *opt, const char *arg, int unset)
* "-h" output even if it's not being handled directly by
* parse_options().
*/
-int parse_opt_unknown_cb(struct parse_opt_ctx_t *ctx,
- const struct option *opt, int unset)
+enum parse_opt_result parse_opt_unknown_cb(struct parse_opt_ctx_t *ctx,
+ const struct option *opt, int unset)
{
- return -2;
+ return PARSE_OPT_UNKNOWN;
}
/**
diff --git a/parse-options.c b/parse-options.c
index 5a717f219f..634484d8f0 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -20,8 +20,9 @@ int optbug(const struct option *opt, const char *reason)
return error("BUG: switch '%c' %s", opt->short_name, reason);
}
-static int get_arg(struct parse_opt_ctx_t *p, const struct option *opt,
- int flags, const char **arg)
+static enum parse_opt_result get_arg(struct parse_opt_ctx_t *p,
+ const struct option *opt,
+ int flags, const char **arg)
{
if (p->opt) {
*arg = p->opt;
@@ -44,9 +45,10 @@ static void fix_filename(const char *prefix, const char **file)
*file = prefix_filename(prefix, *file);
}
-static int opt_command_mode_error(const struct option *opt,
- const struct option *all_opts,
- int flags)
+static enum parse_opt_result opt_command_mode_error(
+ const struct option *opt,
+ const struct option *all_opts,
+ int flags)
{
const struct option *that;
struct strbuf message = STRBUF_INIT;
@@ -71,15 +73,15 @@ static int opt_command_mode_error(const struct option *opt,
strbuf_release(&that_name);
opterror(opt, message.buf, flags);
strbuf_release(&message);
- return -1;
+ return PARSE_OPT_ERROR;
}
return opterror(opt, ": incompatible with something else", flags);
}
-static int get_value(struct parse_opt_ctx_t *p,
- const struct option *opt,
- const struct option *all_opts,
- int flags)
+static enum parse_opt_result get_value(struct parse_opt_ctx_t *p,
+ const struct option *opt,
+ const struct option *all_opts,
+ int flags)
{
const char *s, *arg;
const int unset = flags & OPT_UNSET;
@@ -208,7 +210,8 @@ static int get_value(struct parse_opt_ctx_t *p,
}
}
-static int parse_short_opt(struct parse_opt_ctx_t *p, const struct option *options)
+static enum parse_opt_result parse_short_opt(struct parse_opt_ctx_t *p,
+ const struct option *options)
{
const struct option *all_opts = options;
const struct option *numopt = NULL;
@@ -239,11 +242,12 @@ static int parse_short_opt(struct parse_opt_ctx_t *p, const struct option *optio
free(arg);
return rc;
}
- return -2;
+ return PARSE_OPT_UNKNOWN;
}
-static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg,
- const struct option *options)
+static enum parse_opt_result parse_long_opt(
+ struct parse_opt_ctx_t *p, const char *arg,
+ const struct option *options)
{
const struct option *all_opts = options;
const char *arg_end = strchrnul(arg, '=');
@@ -268,7 +272,7 @@ static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg,
if (*rest)
continue;
p->out[p->cpidx++] = arg - 2;
- return 0;
+ return PARSE_OPT_DONE;
}
if (!rest) {
/* abbreviated? */
@@ -333,11 +337,11 @@ static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg,
ambiguous_option->long_name,
(abbrev_flags & OPT_UNSET) ? "no-" : "",
abbrev_option->long_name);
- return -3;
+ return PARSE_OPT_HELP;
}
if (abbrev_option)
return get_value(p, abbrev_option, all_opts, abbrev_flags);
- return -2;
+ return PARSE_OPT_UNKNOWN;
}
static int parse_nodash_opt(struct parse_opt_ctx_t *p, const char *arg,
@@ -585,22 +589,28 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
if (arg[1] != '-') {
ctx->opt = arg + 1;
switch (parse_short_opt(ctx, options)) {
- case -1:
+ case PARSE_OPT_ERROR:
return PARSE_OPT_ERROR;
- case -2:
+ case PARSE_OPT_UNKNOWN:
if (ctx->opt)
check_typos(arg + 1, options);
if (internal_help && *ctx->opt == 'h')
goto show_usage;
goto unknown;
+ case PARSE_OPT_NON_OPTION:
+ case PARSE_OPT_HELP:
+ case PARSE_OPT_COMPLETE:
+ BUG("parse_short_opt() cannot return these");
+ case PARSE_OPT_DONE:
+ break;
}
if (ctx->opt)
check_typos(arg + 1, options);
while (ctx->opt) {
switch (parse_short_opt(ctx, options)) {
- case -1:
+ case PARSE_OPT_ERROR:
return PARSE_OPT_ERROR;
- case -2:
+ case PARSE_OPT_UNKNOWN:
if (internal_help && *ctx->opt == 'h')
goto show_usage;
@@ -612,6 +622,12 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
ctx->argv[0] = xstrdup(ctx->opt - 1);
*(char *)ctx->argv[0] = '-';
goto unknown;
+ case PARSE_OPT_NON_OPTION:
+ case PARSE_OPT_COMPLETE:
+ case PARSE_OPT_HELP:
+ BUG("parse_short_opt() cannot return these");
+ case PARSE_OPT_DONE:
+ break;
}
}
continue;
@@ -630,12 +646,17 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
if (internal_help && !strcmp(arg + 2, "help"))
goto show_usage;
switch (parse_long_opt(ctx, arg + 2, options)) {
- case -1:
+ case PARSE_OPT_ERROR:
return PARSE_OPT_ERROR;
- case -2:
+ case PARSE_OPT_UNKNOWN:
goto unknown;
- case -3:
+ case PARSE_OPT_HELP:
goto show_usage;
+ case PARSE_OPT_NON_OPTION:
+ case PARSE_OPT_COMPLETE:
+ BUG("parse_long_opt() cannot return these");
+ case PARSE_OPT_DONE:
+ break;
}
continue;
unknown:
diff --git a/parse-options.h b/parse-options.h
index 277a879c1c..617bca8efe 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -49,8 +49,8 @@ struct option;
typedef int parse_opt_cb(const struct option *, const char *arg, int unset);
struct parse_opt_ctx_t;
-typedef int parse_opt_ll_cb(struct parse_opt_ctx_t *ctx,
- const struct option *opt, int unset);
+typedef enum parse_opt_result parse_opt_ll_cb(struct parse_opt_ctx_t *ctx,
+ const struct option *opt, int unset);
/*
* `type`::
@@ -225,12 +225,12 @@ int opterror(const struct option *opt, const char *reason, int flags);
/*----- incremental advanced APIs -----*/
-enum {
- PARSE_OPT_COMPLETE = -2,
- PARSE_OPT_HELP = -1,
- PARSE_OPT_DONE,
+enum parse_opt_result {
+ PARSE_OPT_COMPLETE = -3,
+ PARSE_OPT_HELP = -2,
+ PARSE_OPT_ERROR = -1, /* must be the same as error() */
+ PARSE_OPT_DONE = 0, /* fixed so that "return 0" works */
PARSE_OPT_NON_OPTION,
- PARSE_OPT_ERROR,
PARSE_OPT_UNKNOWN
};
--
2.20.0.482.g66447595a7
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 08/75] parse-options: allow ll_callback with OPTION_CALLBACK
2018-12-27 16:25 [PATCH 00/75] Convert diff opt parser to parse_options() Nguyễn Thái Ngọc Duy
` (6 preceding siblings ...)
2018-12-27 16:25 ` [PATCH 07/75] parse-options: avoid magic return codes Nguyễn Thái Ngọc Duy
@ 2018-12-27 16:25 ` Nguyễn Thái Ngọc Duy
2018-12-27 16:25 ` [PATCH 09/75] diff.h: keep forward struct declarations sorted Nguyễn Thái Ngọc Duy
` (6 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-12-27 16:25 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
OPTION_CALLBACK is much simpler/safer to use, but parse_opt_cb does
not allow access to parse_opt_ctx_t, which sometimes is useful
(e.g. to obtain the prefix).
Extending parse_opt_cb to take parse_opt_cb could result in a lot of
changes. Instead let's just allow ll_callback to be used with
OPTION_CALLBACK. The user will have to be careful, not to change
anything in ctx, or return wrong result code. But that's the price for
ll_callback.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/merge.c | 2 ++
builtin/update-index.c | 20 +++++++++++++++-----
parse-options-cb.c | 4 +++-
parse-options.c | 42 ++++++++++++++++++++++++++++--------------
parse-options.h | 5 +++--
5 files changed, 51 insertions(+), 22 deletions(-)
diff --git a/builtin/merge.c b/builtin/merge.c
index 1c8652e455..b4f4b401eb 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -114,11 +114,13 @@ static int option_parse_message(const struct option *opt,
static enum parse_opt_result option_read_message(struct parse_opt_ctx_t *ctx,
const struct option *opt,
+ const char *arg_not_used,
int unset)
{
struct strbuf *buf = opt->value;
const char *arg;
+ BUG_ON_OPT_ARG(arg_not_used);
if (unset)
BUG("-F cannot be negated");
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 21c84e5590..7abde20169 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -848,13 +848,15 @@ static int parse_new_style_cacheinfo(const char *arg,
}
static enum parse_opt_result cacheinfo_callback(
- struct parse_opt_ctx_t *ctx, const struct option *opt, int unset)
+ struct parse_opt_ctx_t *ctx, const struct option *opt,
+ const char *arg, int unset)
{
struct object_id oid;
unsigned int mode;
const char *path;
BUG_ON_OPT_NEG(unset);
+ BUG_ON_OPT_ARG(arg);
if (!parse_new_style_cacheinfo(ctx->argv[1], &mode, &oid, &path)) {
if (add_cacheinfo(mode, &oid, path, 0))
@@ -874,11 +876,13 @@ static enum parse_opt_result cacheinfo_callback(
}
static enum parse_opt_result stdin_cacheinfo_callback(
- struct parse_opt_ctx_t *ctx, const struct option *opt, int unset)
+ struct parse_opt_ctx_t *ctx, const struct option *opt,
+ const char *arg, int unset)
{
int *nul_term_line = opt->value;
BUG_ON_OPT_NEG(unset);
+ BUG_ON_OPT_ARG(arg);
if (ctx->argc != 1)
return error("option '%s' must be the last argument", opt->long_name);
@@ -888,11 +892,13 @@ static enum parse_opt_result stdin_cacheinfo_callback(
}
static enum parse_opt_result stdin_callback(
- struct parse_opt_ctx_t *ctx, const struct option *opt, int unset)
+ struct parse_opt_ctx_t *ctx, const struct option *opt,
+ const char *arg, int unset)
{
int *read_from_stdin = opt->value;
BUG_ON_OPT_NEG(unset);
+ BUG_ON_OPT_ARG(arg);
if (ctx->argc != 1)
return error("option '%s' must be the last argument", opt->long_name);
@@ -901,12 +907,14 @@ static enum parse_opt_result stdin_callback(
}
static enum parse_opt_result unresolve_callback(
- struct parse_opt_ctx_t *ctx, const struct option *opt, int unset)
+ struct parse_opt_ctx_t *ctx, const struct option *opt,
+ const char *arg, int unset)
{
int *has_errors = opt->value;
const char *prefix = startup_info->prefix;
BUG_ON_OPT_NEG(unset);
+ BUG_ON_OPT_ARG(arg);
/* consume remaining arguments. */
*has_errors = do_unresolve(ctx->argc, ctx->argv,
@@ -920,12 +928,14 @@ static enum parse_opt_result unresolve_callback(
}
static enum parse_opt_result reupdate_callback(
- struct parse_opt_ctx_t *ctx, const struct option *opt, int unset)
+ struct parse_opt_ctx_t *ctx, const struct option *opt,
+ const char *arg, int unset)
{
int *has_errors = opt->value;
const char *prefix = startup_info->prefix;
BUG_ON_OPT_NEG(unset);
+ BUG_ON_OPT_ARG(arg);
/* consume remaining arguments. */
setup_work_tree();
diff --git a/parse-options-cb.c b/parse-options-cb.c
index 720a8663eb..b336b4fa97 100644
--- a/parse-options-cb.c
+++ b/parse-options-cb.c
@@ -170,8 +170,10 @@ int parse_opt_noop_cb(const struct option *opt, const char *arg, int unset)
* parse_options().
*/
enum parse_opt_result parse_opt_unknown_cb(struct parse_opt_ctx_t *ctx,
- const struct option *opt, int unset)
+ const struct option *opt,
+ const char *arg, int unset)
{
+ BUG_ON_OPT_ARG(arg);
return PARSE_OPT_UNKNOWN;
}
diff --git a/parse-options.c b/parse-options.c
index 634484d8f0..8e0aefe86b 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -96,7 +96,7 @@ static enum parse_opt_result get_value(struct parse_opt_ctx_t *p,
switch (opt->type) {
case OPTION_LOWLEVEL_CALLBACK:
- return opt->ll_callback(p, opt, unset);
+ return opt->ll_callback(p, opt, NULL, unset);
case OPTION_BIT:
if (unset)
@@ -162,16 +162,27 @@ static enum parse_opt_result get_value(struct parse_opt_ctx_t *p,
return err;
case OPTION_CALLBACK:
+ {
+ const char *p_arg = NULL;
+ int p_unset;
+
if (unset)
- return (*opt->callback)(opt, NULL, 1) ? (-1) : 0;
- if (opt->flags & PARSE_OPT_NOARG)
- return (*opt->callback)(opt, NULL, 0) ? (-1) : 0;
- if (opt->flags & PARSE_OPT_OPTARG && !p->opt)
- return (*opt->callback)(opt, NULL, 0) ? (-1) : 0;
- if (get_arg(p, opt, flags, &arg))
+ p_unset = 1;
+ else if (opt->flags & PARSE_OPT_NOARG)
+ p_unset = 0;
+ else if (opt->flags & PARSE_OPT_OPTARG && !p->opt)
+ p_unset = 0;
+ else if (get_arg(p, opt, flags, &arg))
return -1;
- return (*opt->callback)(opt, arg, 0) ? (-1) : 0;
-
+ else {
+ p_unset = 0;
+ p_arg = arg;
+ }
+ if (opt->callback)
+ return (*opt->callback)(opt, p_arg, p_unset) ? (-1) : 0;
+ else
+ return (*opt->ll_callback)(p, opt, p_arg, p_unset);
+ }
case OPTION_INTEGER:
if (unset) {
*(int *)opt->value = 0;
@@ -238,7 +249,10 @@ static enum parse_opt_result parse_short_opt(struct parse_opt_ctx_t *p,
len++;
arg = xmemdupz(p->opt, len);
p->opt = p->opt[len] ? p->opt + len : NULL;
- rc = (*numopt->callback)(numopt, arg, 0) ? (-1) : 0;
+ if (numopt->callback)
+ rc = (*numopt->callback)(numopt, arg, 0) ? (-1) : 0;
+ else
+ rc = (*numopt->ll_callback)(p, numopt, arg, 0);
free(arg);
return rc;
}
@@ -413,10 +427,10 @@ static void parse_options_check(const struct option *opts)
err |= optbug(opts, "should not accept an argument");
break;
case OPTION_CALLBACK:
- if (!opts->callback)
- BUG("OPTION_CALLBACK needs a callback");
- if (opts->ll_callback)
- BUG("OPTION_CALLBACK needs no ll_callback");
+ if (!opts->callback && !opts->ll_callback)
+ BUG("OPTION_CALLBACK needs one callback");
+ if (opts->callback && opts->ll_callback)
+ BUG("OPTION_CALLBACK can't have two callbacks");
break;
case OPTION_LOWLEVEL_CALLBACK:
if (!opts->ll_callback)
diff --git a/parse-options.h b/parse-options.h
index 617bca8efe..75c953a887 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -50,7 +50,8 @@ typedef int parse_opt_cb(const struct option *, const char *arg, int unset);
struct parse_opt_ctx_t;
typedef enum parse_opt_result parse_opt_ll_cb(struct parse_opt_ctx_t *ctx,
- const struct option *opt, int unset);
+ const struct option *opt,
+ const char *arg, int unset);
/*
* `type`::
@@ -270,7 +271,7 @@ int parse_opt_commits(const struct option *, const char *, int);
int parse_opt_tertiary(const struct option *, const char *, int);
int parse_opt_string_list(const struct option *, const char *, int);
int parse_opt_noop_cb(const struct option *, const char *, int);
-int parse_opt_unknown_cb(struct parse_opt_ctx_t *ctx, const struct option *, int);
+int parse_opt_unknown_cb(struct parse_opt_ctx_t *ctx, const struct option *, const char *, int);
int parse_opt_passthru(const struct option *, const char *, int);
int parse_opt_passthru_argv(const struct option *, const char *, int);
--
2.20.0.482.g66447595a7
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 09/75] diff.h: keep forward struct declarations sorted
2018-12-27 16:25 [PATCH 00/75] Convert diff opt parser to parse_options() Nguyễn Thái Ngọc Duy
` (7 preceding siblings ...)
2018-12-27 16:25 ` [PATCH 08/75] parse-options: allow ll_callback with OPTION_CALLBACK Nguyễn Thái Ngọc Duy
@ 2018-12-27 16:25 ` Nguyễn Thái Ngọc Duy
2018-12-27 16:25 ` [PATCH 10/75] diff.h: avoid bit fields in struct diff_flags Nguyễn Thái Ngọc Duy
` (5 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-12-27 16:25 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
diff.h | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/diff.h b/diff.h
index 9e8061ca29..4b65ff739f 100644
--- a/diff.h
+++ b/diff.h
@@ -9,16 +9,16 @@
#include "object.h"
#include "oidset.h"
-struct rev_info;
+struct combine_diff_path;
+struct commit;
+struct diff_filespec;
struct diff_options;
struct diff_queue_struct;
-struct strbuf;
-struct diff_filespec;
-struct userdiff_driver;
struct oid_array;
-struct commit;
-struct combine_diff_path;
struct repository;
+struct rev_info;
+struct strbuf;
+struct userdiff_driver;
typedef int (*pathchange_fn_t)(struct diff_options *options,
struct combine_diff_path *path);
--
2.20.0.482.g66447595a7
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 10/75] diff.h: avoid bit fields in struct diff_flags
2018-12-27 16:25 [PATCH 00/75] Convert diff opt parser to parse_options() Nguyễn Thái Ngọc Duy
` (8 preceding siblings ...)
2018-12-27 16:25 ` [PATCH 09/75] diff.h: keep forward struct declarations sorted Nguyễn Thái Ngọc Duy
@ 2018-12-27 16:25 ` Nguyễn Thái Ngọc Duy
2019-01-09 20:21 ` Stefan Beller
2018-12-27 16:25 ` [PATCH 11/75] diff.c: prepare to use parse_options() for parsing Nguyễn Thái Ngọc Duy
` (4 subsequent siblings)
14 siblings, 1 reply; 18+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-12-27 16:25 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
Bitfield addresses cannot be passed around in a pointer. This makes it
hard to use parse-options to set/unset them. Turn this struct to
normal integers. This of course increases the size of this struct
multiple times, but since we only have a handful of diff_options
variables around, memory consumption is not at all a concern.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
diff.h | 66 +++++++++++++++++++++++++++++-----------------------------
1 file changed, 33 insertions(+), 33 deletions(-)
diff --git a/diff.h b/diff.h
index 4b65ff739f..b71062c2d2 100644
--- a/diff.h
+++ b/diff.h
@@ -64,39 +64,39 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)
#define DIFF_FLAGS_INIT { 0 }
struct diff_flags {
- unsigned recursive:1;
- unsigned tree_in_recursive:1;
- unsigned binary:1;
- unsigned text:1;
- unsigned full_index:1;
- unsigned silent_on_remove:1;
- unsigned find_copies_harder:1;
- unsigned follow_renames:1;
- unsigned rename_empty:1;
- unsigned has_changes:1;
- unsigned quick:1;
- unsigned no_index:1;
- unsigned allow_external:1;
- unsigned exit_with_status:1;
- unsigned reverse_diff:1;
- unsigned check_failed:1;
- unsigned relative_name:1;
- unsigned ignore_submodules:1;
- unsigned dirstat_cumulative:1;
- unsigned dirstat_by_file:1;
- unsigned allow_textconv:1;
- unsigned textconv_set_via_cmdline:1;
- unsigned diff_from_contents:1;
- unsigned dirty_submodules:1;
- unsigned ignore_untracked_in_submodules:1;
- unsigned ignore_dirty_submodules:1;
- unsigned override_submodule_config:1;
- unsigned dirstat_by_line:1;
- unsigned funccontext:1;
- unsigned default_follow_renames:1;
- unsigned stat_with_summary:1;
- unsigned suppress_diff_headers:1;
- unsigned dual_color_diffed_diffs:1;
+ unsigned recursive;
+ unsigned tree_in_recursive;
+ unsigned binary;
+ unsigned text;
+ unsigned full_index;
+ unsigned silent_on_remove;
+ unsigned find_copies_harder;
+ unsigned follow_renames;
+ unsigned rename_empty;
+ unsigned has_changes;
+ unsigned quick;
+ unsigned no_index;
+ unsigned allow_external;
+ unsigned exit_with_status;
+ unsigned reverse_diff;
+ unsigned check_failed;
+ unsigned relative_name;
+ unsigned ignore_submodules;
+ unsigned dirstat_cumulative;
+ unsigned dirstat_by_file;
+ unsigned allow_textconv;
+ unsigned textconv_set_via_cmdline;
+ unsigned diff_from_contents;
+ unsigned dirty_submodules;
+ unsigned ignore_untracked_in_submodules;
+ unsigned ignore_dirty_submodules;
+ unsigned override_submodule_config;
+ unsigned dirstat_by_line;
+ unsigned funccontext;
+ unsigned default_follow_renames;
+ unsigned stat_with_summary;
+ unsigned suppress_diff_headers;
+ unsigned dual_color_diffed_diffs;
};
static inline void diff_flags_or(struct diff_flags *a,
--
2.20.0.482.g66447595a7
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 10/75] diff.h: avoid bit fields in struct diff_flags
2018-12-27 16:25 ` [PATCH 10/75] diff.h: avoid bit fields in struct diff_flags Nguyễn Thái Ngọc Duy
@ 2019-01-09 20:21 ` Stefan Beller
0 siblings, 0 replies; 18+ messages in thread
From: Stefan Beller @ 2019-01-09 20:21 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git
On Thu, Dec 27, 2018 at 8:26 AM Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
>
> Bitfield addresses cannot be passed around in a pointer. This makes it
> hard to use parse-options to set/unset them. Turn this struct to
> normal integers. This of course increases the size of this struct
> multiple times, but since we only have a handful of diff_options
> variables around, memory consumption is not at all a concern.
Makes sense, and continues
8cc633286a (Merge branch 'bw/diff-opt-impl-to-bitfields', 2017-11-09)
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 11/75] diff.c: prepare to use parse_options() for parsing
2018-12-27 16:25 [PATCH 00/75] Convert diff opt parser to parse_options() Nguyễn Thái Ngọc Duy
` (9 preceding siblings ...)
2018-12-27 16:25 ` [PATCH 10/75] diff.h: avoid bit fields in struct diff_flags Nguyễn Thái Ngọc Duy
@ 2018-12-27 16:25 ` Nguyễn Thái Ngọc Duy
2018-12-27 16:25 ` [PATCH 12/75] diff.c: convert -u|-p|--patch Nguyễn Thái Ngọc Duy
` (3 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-12-27 16:25 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
This is a preparation step to start using parse_options() to parse
diff/revision options instead of what we have now. There are a couple
of good things from using parse_options():
- better help usage
- easier to add new options
- better completion support
- help usage generation
- better integration with main command option parser. We can just
concat the main command's option array and diffopt's together and
parse all in one go.
- detect colidding options (e.g. --reverse is used by revision code,
so diff code can't use it as long name for -R)
- consistent syntax, e.g. option that takes mandatory argument will
now accept both "--option=value" and "--option value".
The plan is migrate all diff/rev options to parse_options(). Then we
could get rid of diff_opt_parse() and expose parseopts[] directly to
the caller.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
diff.c | 27 +++++++++++++++++++++++++++
diff.h | 2 ++
2 files changed, 29 insertions(+)
diff --git a/diff.c b/diff.c
index 6954f4e39a..a866f13c76 100644
--- a/diff.c
+++ b/diff.c
@@ -23,6 +23,7 @@
#include "argv-array.h"
#include "graph.h"
#include "packfile.h"
+#include "parse-options.h"
#include "help.h"
#ifdef NO_FAST_WORKING_DIRECTORY
@@ -4424,6 +4425,8 @@ static void run_checkdiff(struct diff_filepair *p, struct diff_options *o)
builtin_checkdiff(name, other, attr_path, p->one, p->two, o);
}
+static void prep_parse_options(struct diff_options *options);
+
void repo_diff_setup(struct repository *r, struct diff_options *options)
{
memcpy(options, &default_diff_options, sizeof(*options));
@@ -4465,6 +4468,8 @@ void repo_diff_setup(struct repository *r, struct diff_options *options)
options->color_moved = diff_color_moved_default;
options->color_moved_ws_handling = diff_color_moved_ws_default;
+
+ prep_parse_options(options);
}
void diff_setup_done(struct diff_options *options)
@@ -4568,6 +4573,8 @@ void diff_setup_done(struct diff_options *options)
if (!options->use_color || external_diff())
options->color_moved = 0;
+
+ FREE_AND_NULL(options->parseopts);
}
static int opt_arg(const char *arg, int arg_short, const char *arg_long, int *val)
@@ -4858,6 +4865,16 @@ static int parse_objfind_opt(struct diff_options *opt, const char *arg)
return 1;
}
+static void prep_parse_options(struct diff_options *options)
+{
+ struct option parseopts[] = {
+ OPT_END()
+ };
+
+ ALLOC_ARRAY(options->parseopts, ARRAY_SIZE(parseopts));
+ memcpy(options->parseopts, parseopts, sizeof(parseopts));
+}
+
int diff_opt_parse(struct diff_options *options,
const char **av, int ac, const char *prefix)
{
@@ -4868,6 +4885,16 @@ int diff_opt_parse(struct diff_options *options,
if (!prefix)
prefix = "";
+ ac = parse_options(ac, av, prefix, options->parseopts, NULL,
+ PARSE_OPT_KEEP_DASHDASH |
+ PARSE_OPT_KEEP_UNKNOWN |
+ PARSE_OPT_NO_INTERNAL_HELP |
+ PARSE_OPT_ONE_SHOT |
+ PARSE_OPT_STOP_AT_NON_OPTION);
+
+ if (ac)
+ return ac;
+
/* Output format options */
if (!strcmp(arg, "-p") || !strcmp(arg, "-u") || !strcmp(arg, "--patch")
|| opt_arg(arg, 'U', "unified", &options->context))
diff --git a/diff.h b/diff.h
index b71062c2d2..8c4748ebcf 100644
--- a/diff.h
+++ b/diff.h
@@ -15,6 +15,7 @@ struct diff_filespec;
struct diff_options;
struct diff_queue_struct;
struct oid_array;
+struct option;
struct repository;
struct rev_info;
struct strbuf;
@@ -229,6 +230,7 @@ struct diff_options {
unsigned color_moved_ws_handling;
struct repository *repo;
+ struct option *parseopts;
};
void diff_emit_submodule_del(struct diff_options *o, const char *line);
--
2.20.0.482.g66447595a7
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 12/75] diff.c: convert -u|-p|--patch
2018-12-27 16:25 [PATCH 00/75] Convert diff opt parser to parse_options() Nguyễn Thái Ngọc Duy
` (10 preceding siblings ...)
2018-12-27 16:25 ` [PATCH 11/75] diff.c: prepare to use parse_options() for parsing Nguyễn Thái Ngọc Duy
@ 2018-12-27 16:25 ` Nguyễn Thái Ngọc Duy
2018-12-27 16:25 ` [PATCH 73/75] range-diff: use parse_options() instead of diff_opt_parse() Nguyễn Thái Ngọc Duy
` (2 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-12-27 16:25 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
diff.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/diff.c b/diff.c
index a866f13c76..59132ceaff 100644
--- a/diff.c
+++ b/diff.c
@@ -4868,6 +4868,13 @@ static int parse_objfind_opt(struct diff_options *opt, const char *arg)
static void prep_parse_options(struct diff_options *options)
{
struct option parseopts[] = {
+ OPT_GROUP(N_("Diff output format options")),
+ OPT_BITOP('p', "patch", &options->output_format,
+ N_("generate patch"),
+ DIFF_FORMAT_PATCH, DIFF_FORMAT_NO_OUTPUT),
+ OPT_BITOP('u', NULL, &options->output_format,
+ N_("generate patch"),
+ DIFF_FORMAT_PATCH, DIFF_FORMAT_NO_OUTPUT),
OPT_END()
};
@@ -4896,8 +4903,7 @@ int diff_opt_parse(struct diff_options *options,
return ac;
/* Output format options */
- if (!strcmp(arg, "-p") || !strcmp(arg, "-u") || !strcmp(arg, "--patch")
- || opt_arg(arg, 'U', "unified", &options->context))
+ if (opt_arg(arg, 'U', "unified", &options->context))
enable_patch_output(&options->output_format);
else if (!strcmp(arg, "--raw"))
options->output_format |= DIFF_FORMAT_RAW;
--
2.20.0.482.g66447595a7
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 73/75] range-diff: use parse_options() instead of diff_opt_parse()
2018-12-27 16:25 [PATCH 00/75] Convert diff opt parser to parse_options() Nguyễn Thái Ngọc Duy
` (11 preceding siblings ...)
2018-12-27 16:25 ` [PATCH 12/75] diff.c: convert -u|-p|--patch Nguyễn Thái Ngọc Duy
@ 2018-12-27 16:25 ` Nguyễn Thái Ngọc Duy
2018-12-27 16:25 ` [PATCH 74/75] diff --no-index: " Nguyễn Thái Ngọc Duy
2018-12-27 16:25 ` [PATCH 75/75] am: avoid diff_opt_parse() Nguyễn Thái Ngọc Duy
14 siblings, 0 replies; 18+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-12-27 16:25 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
Diff's internal option parsing is now done with 'struct option', which
makes it possible to combine all diff options to range-diff and parse
everything all at once. Parsing code becomes simpler, and we get a
looong 'git range-diff -h'
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/range-diff.c | 26 ++++++--------------------
1 file changed, 6 insertions(+), 20 deletions(-)
diff --git a/builtin/range-diff.c b/builtin/range-diff.c
index f01a0be851..784bd19321 100644
--- a/builtin/range-diff.c
+++ b/builtin/range-diff.c
@@ -16,42 +16,27 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)
int creation_factor = RANGE_DIFF_CREATION_FACTOR_DEFAULT;
struct diff_options diffopt = { NULL };
int simple_color = -1;
- struct option options[] = {
+ struct option range_diff_options[] = {
OPT_INTEGER(0, "creation-factor", &creation_factor,
N_("Percentage by which creation is weighted")),
OPT_BOOL(0, "no-dual-color", &simple_color,
N_("use simple diff colors")),
OPT_END()
};
- int i, j, res = 0;
+ struct option *options;
+ int res = 0;
struct strbuf range1 = STRBUF_INIT, range2 = STRBUF_INIT;
git_config(git_diff_ui_config, NULL);
repo_diff_setup(the_repository, &diffopt);
+ options = parse_options_concat(range_diff_options, diffopt.parseopts);
argc = parse_options(argc, argv, NULL, options,
- builtin_range_diff_usage, PARSE_OPT_KEEP_UNKNOWN |
- PARSE_OPT_KEEP_DASHDASH | PARSE_OPT_KEEP_ARGV0);
-
- for (i = j = 1; i < argc && strcmp("--", argv[i]); ) {
- int c = diff_opt_parse(&diffopt, argv + i, argc - i, prefix);
+ builtin_range_diff_usage, 0);
- if (!c)
- argv[j++] = argv[i++];
- else
- i += c;
- }
- while (i < argc)
- argv[j++] = argv[i++];
- argc = j;
diff_setup_done(&diffopt);
- /* Make sure that there are no unparsed options */
- argc = parse_options(argc, argv, NULL,
- options + ARRAY_SIZE(options) - 1, /* OPT_END */
- builtin_range_diff_usage, 0);
-
/* force color when --dual-color was used */
if (!simple_color)
diffopt.use_color = 1;
@@ -90,6 +75,7 @@ int cmd_range_diff(int argc, const char **argv, const char *prefix)
error(_("need two commit ranges"));
usage_with_options(builtin_range_diff_usage, options);
}
+ FREE_AND_NULL(options);
res = show_range_diff(range1.buf, range2.buf, creation_factor,
simple_color < 1, &diffopt);
--
2.20.0.482.g66447595a7
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 74/75] diff --no-index: use parse_options() instead of diff_opt_parse()
2018-12-27 16:25 [PATCH 00/75] Convert diff opt parser to parse_options() Nguyễn Thái Ngọc Duy
` (12 preceding siblings ...)
2018-12-27 16:25 ` [PATCH 73/75] range-diff: use parse_options() instead of diff_opt_parse() Nguyễn Thái Ngọc Duy
@ 2018-12-27 16:25 ` Nguyễn Thái Ngọc Duy
2018-12-27 20:34 ` Eric Sunshine
2018-12-27 16:25 ` [PATCH 75/75] am: avoid diff_opt_parse() Nguyễn Thái Ngọc Duy
14 siblings, 1 reply; 18+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-12-27 16:25 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
While at there, move exit() back to the caller. It's easier to see the
flow that way then burying it in diff-no-index.c
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/diff.c | 21 +++--------------
diff-no-index.c | 49 ++++++++++++++++++++++++----------------
diff.h | 3 ++-
t/t4053-diff-no-index.sh | 3 +--
4 files changed, 35 insertions(+), 41 deletions(-)
diff --git a/builtin/diff.c b/builtin/diff.c
index f0393bba23..52dc3e136f 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -320,26 +320,11 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
repo_init_revisions(the_repository, &rev, prefix);
- if (no_index && argc != i + 2) {
- if (no_index == DIFF_NO_INDEX_IMPLICIT) {
- /*
- * There was no --no-index and there were not two
- * paths. It is possible that the user intended
- * to do an inside-repository operation.
- */
- fprintf(stderr, "Not a git repository\n");
- fprintf(stderr,
- "To compare two paths outside a working tree:\n");
- }
- /* Give the usage message for non-repository usage and exit. */
- usagef("git diff %s <path> <path>",
- no_index == DIFF_NO_INDEX_EXPLICIT ?
- "--no-index" : "[--no-index]");
-
- }
if (no_index)
/* If this is a no-index diff, just run it and exit there. */
- diff_no_index(the_repository, &rev, argc, argv);
+ exit(diff_no_index(the_repository, &rev,
+ no_index == DIFF_NO_INDEX_IMPLICIT,
+ argc, argv));
/* Otherwise, we are doing the usual "git" diff */
rev.diffopt.skip_stat_unmatch = !!diff_auto_refresh_index;
diff --git a/diff-no-index.c b/diff-no-index.c
index 9414e922d1..a879f45862 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -14,6 +14,7 @@
#include "revision.h"
#include "log-tree.h"
#include "builtin.h"
+#include "parse-options.h"
#include "string-list.h"
#include "dir.h"
@@ -233,35 +234,43 @@ static void fixup_paths(const char **path, struct strbuf *replacement)
}
}
-void diff_no_index(struct repository *r,
- struct rev_info *revs,
- int argc, const char **argv)
+static const char * const diff_no_index_usage[] = {
+ N_("git diff --no-index [<options>] <path> <path>"),
+ NULL
+};
+
+int diff_no_index(struct repository *r,
+ struct rev_info *revs,
+ int implicit_no_index,
+ int argc, const char **argv)
{
- int i;
+ int i, no_index;
const char *paths[2];
struct strbuf replacement = STRBUF_INIT;
const char *prefix = revs->prefix;
+ struct option no_index_options[] = {
+ OPT_BOOL_F(0, "no-index", &no_index, "",
+ PARSE_OPT_NONEG | PARSE_OPT_HIDDEN),
+ OPT_END(),
+ };
+ struct option *options;
/*
* FIXME: --no-index should not look at index and we should be
* able to pass NULL repo. Maybe later.
*/
repo_diff_setup(r, &revs->diffopt);
- for (i = 1; i < argc - 2; ) {
- int j;
- if (!strcmp(argv[i], "--no-index"))
- i++;
- else if (!strcmp(argv[i], "--"))
- i++;
- else {
- j = diff_opt_parse(&revs->diffopt, argv + i, argc - i,
- revs->prefix);
- if (j <= 0)
- die("invalid diff option/value: %s", argv[i]);
- i += j;
- }
+ options = parse_options_concat(no_index_options,
+ revs->diffopt.parseopts);
+ argc = parse_options(argc, argv, revs->prefix, options,
+ diff_no_index_usage, 0);
+ if (argc != 2) {
+ if (implicit_no_index)
+ warning(_("Not a git repository. Use --no-index to "
+ "compare two paths outside a working tree"));
+ usage_with_options(diff_no_index_usage, options);
}
-
+ FREE_AND_NULL(options);
for (i = 0; i < 2; i++) {
const char *p = argv[argc - 2 + i];
if (!strcmp(p, "-"))
@@ -293,7 +302,7 @@ void diff_no_index(struct repository *r,
revs->diffopt.flags.exit_with_status = 1;
if (queue_diff(&revs->diffopt, paths[0], paths[1]))
- exit(1);
+ return 1;
diff_set_mnemonic_prefix(&revs->diffopt, "1/", "2/");
diffcore_std(&revs->diffopt);
diff_flush(&revs->diffopt);
@@ -304,5 +313,5 @@ void diff_no_index(struct repository *r,
* The return code for --no-index imitates diff(1):
* 0 = no changes, 1 = changes, else error
*/
- exit(diff_result_code(&revs->diffopt, 0));
+ return diff_result_code(&revs->diffopt, 0);
}
diff --git a/diff.h b/diff.h
index 8c4748ebcf..44843d8929 100644
--- a/diff.h
+++ b/diff.h
@@ -437,7 +437,8 @@ int diff_flush_patch_id(struct diff_options *, struct object_id *, int);
int diff_result_code(struct diff_options *, int);
-void diff_no_index(struct repository *, struct rev_info *, int, const char **);
+int diff_no_index(struct repository *, struct rev_info *,
+ int implicit_no_index, int, const char **);
int index_differs_from(const char *def, const struct diff_flags *flags,
int ita_invisible_in_index);
diff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh
index 6e0dd6f9e5..fb25cdb789 100755
--- a/t/t4053-diff-no-index.sh
+++ b/t/t4053-diff-no-index.sh
@@ -50,8 +50,7 @@ test_expect_success 'git diff --no-index executed outside repo gives correct err
export GIT_CEILING_DIRECTORIES &&
cd non/git &&
test_must_fail git diff --no-index a 2>actual.err &&
- echo "usage: git diff --no-index <path> <path>" >expect.err &&
- test_cmp expect.err actual.err
+ test_i18ngrep "usage: git diff --no-index" actual.err
)
'
--
2.20.0.482.g66447595a7
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 75/75] am: avoid diff_opt_parse()
2018-12-27 16:25 [PATCH 00/75] Convert diff opt parser to parse_options() Nguyễn Thái Ngọc Duy
` (13 preceding siblings ...)
2018-12-27 16:25 ` [PATCH 74/75] diff --no-index: " Nguyễn Thái Ngọc Duy
@ 2018-12-27 16:25 ` Nguyễn Thái Ngọc Duy
14 siblings, 0 replies; 18+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2018-12-27 16:25 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
diff_opt_parse() is a heavy hammer to just set diff filter. But it's
the only way because of the diff_status_letters[] mapping. Add a new
API to set diff filter and use it in git-am. diff_opt_parse()'s only
remaining call site in revision.c will be gone soon and having it here
just because of git-am does not make sense.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/am.c | 4 ++--
diff.c | 6 ++++++
diff.h | 2 ++
3 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/builtin/am.c b/builtin/am.c
index 8f27f3375b..a98e65f440 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1515,11 +1515,11 @@ static int fall_back_threeway(const struct am_state *state, const char *index_pa
* review them with extra care to spot mismerges.
*/
struct rev_info rev_info;
- const char *diff_filter_str = "--diff-filter=AM";
repo_init_revisions(the_repository, &rev_info, NULL);
rev_info.diffopt.output_format = DIFF_FORMAT_NAME_STATUS;
- diff_opt_parse(&rev_info.diffopt, &diff_filter_str, 1, rev_info.prefix);
+ rev_info.diffopt.filter |= diff_filter_bit('A');
+ rev_info.diffopt.filter |= diff_filter_bit('M');
add_pending_oid(&rev_info, "HEAD", &our_tree, 0);
diff_setup_done(&rev_info.diffopt);
run_diff_index(&rev_info, 1);
diff --git a/diff.c b/diff.c
index 43ae8d4d2c..8a7011b928 100644
--- a/diff.c
+++ b/diff.c
@@ -4756,6 +4756,12 @@ static unsigned filter_bit_tst(char status, const struct diff_options *opt)
return opt->filter & filter_bit[(int) status];
}
+unsigned diff_filter_bit(char status)
+{
+ prepare_filter_bits();
+ return filter_bit[(int) status];
+}
+
static int diff_opt_diff_filter(const struct option *option,
const char *optarg, int unset)
{
diff --git a/diff.h b/diff.h
index 44843d8929..48cfa3f0a6 100644
--- a/diff.h
+++ b/diff.h
@@ -233,6 +233,8 @@ struct diff_options {
struct option *parseopts;
};
+unsigned diff_filter_bit(char status);
+
void diff_emit_submodule_del(struct diff_options *o, const char *line);
void diff_emit_submodule_add(struct diff_options *o, const char *line);
void diff_emit_submodule_untracked(struct diff_options *o, const char *path);
--
2.20.0.482.g66447595a7
^ permalink raw reply related [flat|nested] 18+ messages in thread