Skip to content

Commit 54b469b

Browse files
gitstergitster
committed
Merge branch 'nd/diff-parseopt'
The diff machinery, one of the oldest parts of the system, which long predates the parse-options API, uses fairly long and complex handcrafted option parser. This is being rewritten to use the parse-options API. * nd/diff-parseopt: diff.c: convert --raw diff.c: convert -W|--[no-]function-context diff.c: convert -U|--unified diff.c: convert -u|-p|--patch diff.c: prepare to use parse_options() for parsing diff.h: avoid bit fields in struct diff_flags diff.h: keep forward struct declarations sorted parse-options: allow ll_callback with OPTION_CALLBACK parse-options: avoid magic return codes parse-options: stop abusing 'callback' for lowlevel callbacks parse-options: add OPT_BITOP() parse-options: disable option abbreviation with PARSE_OPT_KEEP_UNKNOWN parse-options: add one-shot mode parse-options.h: remove extern on function prototypes
2 parents 7d0c1f4 + ed88148 commit 54b469b

File tree

10 files changed

+318
-158
lines changed

10 files changed

+318
-158
lines changed

‎Documentation/diff-options.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ endif::git-format-patch[]
3636
-U<n>::
3737
--unified=<n>::
3838
Generate diffs with <n> lines of context instead of
39-
the usual three.
39+
the usual three. Implies `--patch`.
4040
ifndef::git-format-patch[]
4141
Implies `-p`.
4242
endif::git-format-patch[]

‎builtin/blame.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -814,7 +814,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
814814
* and are only included here to get included in the "-h"
815815
* output:
816816
*/
817-
{ OPTION_LOWLEVEL_CALLBACK, 0, "indent-heuristic", NULL, NULL, N_("Use an experimental heuristic to improve diffs"), PARSE_OPT_NOARG, parse_opt_unknown_cb },
817+
{ 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 },
818818

819819
OPT_BIT(0, "minimal", &xdl_opts, N_("Spend extra cycles to find better match"), XDF_NEED_MINIMAL),
820820
OPT_STRING('S', NULL, &revs_file, N_("file"), N_("Use revisions from <file> instead of calling git-rev-list")),

‎builtin/merge.c‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -113,12 +113,15 @@ static int option_parse_message(const struct option *opt,
113113
return 0;
114114
}
115115

116-
static int option_read_message(struct parse_opt_ctx_t *ctx,
117-
const struct option *opt, int unset)
116+
static enum parse_opt_result option_read_message(struct parse_opt_ctx_t *ctx,
117+
const struct option *opt,
118+
const char *arg_not_used,
119+
int unset)
118120
{
119121
struct strbuf *buf = opt->value;
120122
const char *arg;
121123

124+
BUG_ON_OPT_ARG(arg_not_used);
122125
if (unset)
123126
BUG("-F cannot be negated");
124127

@@ -262,7 +265,7 @@ static struct option builtin_merge_options[] = {
262265
option_parse_message),
263266
{ OPTION_LOWLEVEL_CALLBACK, 'F', "file", &merge_msg, N_("path"),
264267
N_("read message from file"), PARSE_OPT_NONEG,
265-
(parse_opt_cb *) option_read_message },
268+
NULL, 0, option_read_message },
266269
OPT__VERBOSITY(&verbosity),
267270
OPT_BOOL(0, "abort", &abort_current_merge,
268271
N_("abort the current in-progress merge")),

‎builtin/update-index.c‎

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -848,14 +848,16 @@ static int parse_new_style_cacheinfo(const char *arg,
848848
return 0;
849849
}
850850

851-
static int cacheinfo_callback(struct parse_opt_ctx_t *ctx,
852-
const struct option *opt, int unset)
851+
static enum parse_opt_result cacheinfo_callback(
852+
struct parse_opt_ctx_t *ctx, const struct option *opt,
853+
const char *arg, int unset)
853854
{
854855
struct object_id oid;
855856
unsigned int mode;
856857
const char *path;
857858

858859
BUG_ON_OPT_NEG(unset);
860+
BUG_ON_OPT_ARG(arg);
859861

860862
if (!parse_new_style_cacheinfo(ctx->argv[1], &mode, &oid, &path)) {
861863
if (add_cacheinfo(mode, &oid, path, 0))
@@ -874,12 +876,14 @@ static int cacheinfo_callback(struct parse_opt_ctx_t *ctx,
874876
return 0;
875877
}
876878

877-
static int stdin_cacheinfo_callback(struct parse_opt_ctx_t *ctx,
878-
const struct option *opt, int unset)
879+
static enum parse_opt_result stdin_cacheinfo_callback(
880+
struct parse_opt_ctx_t *ctx, const struct option *opt,
881+
const char *arg, int unset)
879882
{
880883
int *nul_term_line = opt->value;
881884

882885
BUG_ON_OPT_NEG(unset);
886+
BUG_ON_OPT_ARG(arg);
883887

884888
if (ctx->argc != 1)
885889
return error("option '%s' must be the last argument", opt->long_name);
@@ -888,26 +892,30 @@ static int stdin_cacheinfo_callback(struct parse_opt_ctx_t *ctx,
888892
return 0;
889893
}
890894

891-
static int stdin_callback(struct parse_opt_ctx_t *ctx,
892-
const struct option *opt, int unset)
895+
static enum parse_opt_result stdin_callback(
896+
struct parse_opt_ctx_t *ctx, const struct option *opt,
897+
const char *arg, int unset)
893898
{
894899
int *read_from_stdin = opt->value;
895900

896901
BUG_ON_OPT_NEG(unset);
902+
BUG_ON_OPT_ARG(arg);
897903

898904
if (ctx->argc != 1)
899905
return error("option '%s' must be the last argument", opt->long_name);
900906
*read_from_stdin = 1;
901907
return 0;
902908
}
903909

904-
static int unresolve_callback(struct parse_opt_ctx_t *ctx,
905-
const struct option *opt, int unset)
910+
static enum parse_opt_result unresolve_callback(
911+
struct parse_opt_ctx_t *ctx, const struct option *opt,
912+
const char *arg, int unset)
906913
{
907914
int *has_errors = opt->value;
908915
const char *prefix = startup_info->prefix;
909916

910917
BUG_ON_OPT_NEG(unset);
918+
BUG_ON_OPT_ARG(arg);
911919

912920
/* consume remaining arguments. */
913921
*has_errors = do_unresolve(ctx->argc, ctx->argv,
@@ -920,13 +928,15 @@ static int unresolve_callback(struct parse_opt_ctx_t *ctx,
920928
return 0;
921929
}
922930

923-
static int reupdate_callback(struct parse_opt_ctx_t *ctx,
924-
const struct option *opt, int unset)
931+
static enum parse_opt_result reupdate_callback(
932+
struct parse_opt_ctx_t *ctx, const struct option *opt,
933+
const char *arg, int unset)
925934
{
926935
int *has_errors = opt->value;
927936
const char *prefix = startup_info->prefix;
928937

929938
BUG_ON_OPT_NEG(unset);
939+
BUG_ON_OPT_ARG(arg);
930940

931941
/* consume remaining arguments. */
932942
setup_work_tree();
@@ -986,7 +996,8 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
986996
N_("add the specified entry to the index"),
987997
PARSE_OPT_NOARG | /* disallow --cacheinfo=<mode> form */
988998
PARSE_OPT_NONEG | PARSE_OPT_LITERAL_ARGHELP,
989-
(parse_opt_cb *) cacheinfo_callback},
999+
NULL, 0,
1000+
cacheinfo_callback},
9901001
{OPTION_CALLBACK, 0, "chmod", &set_executable_bit, "(+|-)x",
9911002
N_("override the executable bit of the listed files"),
9921003
PARSE_OPT_NONEG,
@@ -1012,19 +1023,19 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
10121023
{OPTION_LOWLEVEL_CALLBACK, 0, "stdin", &read_from_stdin, NULL,
10131024
N_("read list of paths to be updated from standard input"),
10141025
PARSE_OPT_NONEG | PARSE_OPT_NOARG,
1015-
(parse_opt_cb *) stdin_callback},
1026+
NULL, 0, stdin_callback},
10161027
{OPTION_LOWLEVEL_CALLBACK, 0, "index-info", &nul_term_line, NULL,
10171028
N_("add entries from standard input to the index"),
10181029
PARSE_OPT_NONEG | PARSE_OPT_NOARG,
1019-
(parse_opt_cb *) stdin_cacheinfo_callback},
1030+
NULL, 0, stdin_cacheinfo_callback},
10201031
{OPTION_LOWLEVEL_CALLBACK, 0, "unresolve", &has_errors, NULL,
10211032
N_("repopulate stages #2 and #3 for the listed paths"),
10221033
PARSE_OPT_NONEG | PARSE_OPT_NOARG,
1023-
(parse_opt_cb *) unresolve_callback},
1034+
NULL, 0, unresolve_callback},
10241035
{OPTION_LOWLEVEL_CALLBACK, 'g', "again", &has_errors, NULL,
10251036
N_("only update entries that differ from HEAD"),
10261037
PARSE_OPT_NONEG | PARSE_OPT_NOARG,
1027-
(parse_opt_cb *) reupdate_callback},
1038+
NULL, 0, reupdate_callback},
10281039
OPT_BIT(0, "ignore-missing", &refresh_args.flags,
10291040
N_("ignore files missing from worktree"),
10301041
REFRESH_IGNORE_MISSING),

‎diff.c‎

Lines changed: 59 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include "argv-array.h"
2424
#include "graph.h"
2525
#include "packfile.h"
26+
#include "parse-options.h"
2627
#include "help.h"
2728

2829
#ifdef NO_FAST_WORKING_DIRECTORY
@@ -4491,6 +4492,8 @@ static void run_checkdiff(struct diff_filepair *p, struct diff_options *o)
44914492
builtin_checkdiff(name, other, attr_path, p->one, p->two, o);
44924493
}
44934494

4495+
static void prep_parse_options(struct diff_options *options);
4496+
44944497
void repo_diff_setup(struct repository *r, struct diff_options *options)
44954498
{
44964499
memcpy(options, &default_diff_options, sizeof(*options));
@@ -4532,6 +4535,8 @@ void repo_diff_setup(struct repository *r, struct diff_options *options)
45324535

45334536
options->color_moved = diff_color_moved_default;
45344537
options->color_moved_ws_handling = diff_color_moved_ws_default;
4538+
4539+
prep_parse_options(options);
45354540
}
45364541

45374542
void diff_setup_done(struct diff_options *options)
@@ -4635,6 +4640,8 @@ void diff_setup_done(struct diff_options *options)
46354640

46364641
if (!options->use_color || external_diff())
46374642
options->color_moved = 0;
4643+
4644+
FREE_AND_NULL(options->parseopts);
46384645
}
46394646

46404647
static int opt_arg(const char *arg, int arg_short, const char *arg_long, int *val)
@@ -4926,6 +4933,47 @@ static int parse_objfind_opt(struct diff_options *opt, const char *arg)
49264933
return 1;
49274934
}
49284935

4936+
static int diff_opt_unified(const struct option *opt,
4937+
const char *arg, int unset)
4938+
{
4939+
struct diff_options *options = opt->value;
4940+
char *s;
4941+
4942+
BUG_ON_OPT_NEG(unset);
4943+
4944+
options->context = strtol(arg, &s, 10);
4945+
if (*s)
4946+
return error(_("%s expects a numerical value"), "--unified");
4947+
enable_patch_output(&options->output_format);
4948+
4949+
return 0;
4950+
}
4951+
4952+
static void prep_parse_options(struct diff_options *options)
4953+
{
4954+
struct option parseopts[] = {
4955+
OPT_GROUP(N_("Diff output format options")),
4956+
OPT_BITOP('p', "patch", &options->output_format,
4957+
N_("generate patch"),
4958+
DIFF_FORMAT_PATCH, DIFF_FORMAT_NO_OUTPUT),
4959+
OPT_BITOP('u', NULL, &options->output_format,
4960+
N_("generate patch"),
4961+
DIFF_FORMAT_PATCH, DIFF_FORMAT_NO_OUTPUT),
4962+
OPT_CALLBACK_F('U', "unified", options, N_("<n>"),
4963+
N_("generate diffs with <n> lines context"),
4964+
PARSE_OPT_NONEG, diff_opt_unified),
4965+
OPT_BOOL('W', "function-context", &options->flags.funccontext,
4966+
N_("generate diffs with <n> lines context")),
4967+
OPT_BIT_F(0, "raw", &options->output_format,
4968+
N_("generate the diff in raw format"),
4969+
DIFF_FORMAT_RAW, PARSE_OPT_NONEG),
4970+
OPT_END()
4971+
};
4972+
4973+
ALLOC_ARRAY(options->parseopts, ARRAY_SIZE(parseopts));
4974+
memcpy(options->parseopts, parseopts, sizeof(parseopts));
4975+
}
4976+
49294977
int diff_opt_parse(struct diff_options *options,
49304978
const char **av, int ac, const char *prefix)
49314979
{
@@ -4936,13 +4984,18 @@ int diff_opt_parse(struct diff_options *options,
49364984
if (!prefix)
49374985
prefix = "";
49384986

4987+
ac = parse_options(ac, av, prefix, options->parseopts, NULL,
4988+
PARSE_OPT_KEEP_DASHDASH |
4989+
PARSE_OPT_KEEP_UNKNOWN |
4990+
PARSE_OPT_NO_INTERNAL_HELP |
4991+
PARSE_OPT_ONE_SHOT |
4992+
PARSE_OPT_STOP_AT_NON_OPTION);
4993+
4994+
if (ac)
4995+
return ac;
4996+
49394997
/* Output format options */
4940-
if (!strcmp(arg, "-p") || !strcmp(arg, "-u") || !strcmp(arg, "--patch")
4941-
|| opt_arg(arg, 'U', "unified", &options->context))
4942-
enable_patch_output(&options->output_format);
4943-
else if (!strcmp(arg, "--raw"))
4944-
options->output_format |= DIFF_FORMAT_RAW;
4945-
else if (!strcmp(arg, "--patch-with-raw")) {
4998+
if (!strcmp(arg, "--patch-with-raw")) {
49464999
enable_patch_output(&options->output_format);
49475000
options->output_format |= DIFF_FORMAT_RAW;
49485001
} else if (!strcmp(arg, "--numstat"))
@@ -5230,12 +5283,6 @@ int diff_opt_parse(struct diff_options *options,
52305283
else if (opt_arg(arg, '\0', "inter-hunk-context",
52315284
&options->interhunkcontext))
52325285
;
5233-
else if (!strcmp(arg, "-W"))
5234-
options->flags.funccontext = 1;
5235-
else if (!strcmp(arg, "--function-context"))
5236-
options->flags.funccontext = 1;
5237-
else if (!strcmp(arg, "--no-function-context"))
5238-
options->flags.funccontext = 0;
52395286
else if ((argcount = parse_long_opt("output", av, &optarg))) {
52405287
char *path = prefix_filename(prefix, optarg);
52415288
options->file = xfopen(path, "w");

‎diff.h‎

Lines changed: 41 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -9,16 +9,17 @@
99
#include "object.h"
1010
#include "oidset.h"
1111

12-
struct rev_info;
12+
struct combine_diff_path;
13+
struct commit;
14+
struct diff_filespec;
1315
struct diff_options;
1416
struct diff_queue_struct;
15-
struct strbuf;
16-
struct diff_filespec;
17-
struct userdiff_driver;
1817
struct oid_array;
19-
struct commit;
20-
struct combine_diff_path;
18+
struct option;
2119
struct repository;
20+
struct rev_info;
21+
struct strbuf;
22+
struct userdiff_driver;
2223

2324
typedef int (*pathchange_fn_t)(struct diff_options *options,
2425
struct combine_diff_path *path);
@@ -64,39 +65,39 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)
6465

6566
#define DIFF_FLAGS_INIT { 0 }
6667
struct diff_flags {
67-
unsigned recursive:1;
68-
unsigned tree_in_recursive:1;
69-
unsigned binary:1;
70-
unsigned text:1;
71-
unsigned full_index:1;
72-
unsigned silent_on_remove:1;
73-
unsigned find_copies_harder:1;
74-
unsigned follow_renames:1;
75-
unsigned rename_empty:1;
76-
unsigned has_changes:1;
77-
unsigned quick:1;
78-
unsigned no_index:1;
79-
unsigned allow_external:1;
80-
unsigned exit_with_status:1;
81-
unsigned reverse_diff:1;
82-
unsigned check_failed:1;
83-
unsigned relative_name:1;
84-
unsigned ignore_submodules:1;
85-
unsigned dirstat_cumulative:1;
86-
unsigned dirstat_by_file:1;
87-
unsigned allow_textconv:1;
88-
unsigned textconv_set_via_cmdline:1;
89-
unsigned diff_from_contents:1;
90-
unsigned dirty_submodules:1;
91-
unsigned ignore_untracked_in_submodules:1;
92-
unsigned ignore_dirty_submodules:1;
93-
unsigned override_submodule_config:1;
94-
unsigned dirstat_by_line:1;
95-
unsigned funccontext:1;
96-
unsigned default_follow_renames:1;
97-
unsigned stat_with_summary:1;
98-
unsigned suppress_diff_headers:1;
99-
unsigned dual_color_diffed_diffs:1;
68+
unsigned recursive;
69+
unsigned tree_in_recursive;
70+
unsigned binary;
71+
unsigned text;
72+
unsigned full_index;
73+
unsigned silent_on_remove;
74+
unsigned find_copies_harder;
75+
unsigned follow_renames;
76+
unsigned rename_empty;
77+
unsigned has_changes;
78+
unsigned quick;
79+
unsigned no_index;
80+
unsigned allow_external;
81+
unsigned exit_with_status;
82+
unsigned reverse_diff;
83+
unsigned check_failed;
84+
unsigned relative_name;
85+
unsigned ignore_submodules;
86+
unsigned dirstat_cumulative;
87+
unsigned dirstat_by_file;
88+
unsigned allow_textconv;
89+
unsigned textconv_set_via_cmdline;
90+
unsigned diff_from_contents;
91+
unsigned dirty_submodules;
92+
unsigned ignore_untracked_in_submodules;
93+
unsigned ignore_dirty_submodules;
94+
unsigned override_submodule_config;
95+
unsigned dirstat_by_line;
96+
unsigned funccontext;
97+
unsigned default_follow_renames;
98+
unsigned stat_with_summary;
99+
unsigned suppress_diff_headers;
100+
unsigned dual_color_diffed_diffs;
100101
};
101102

102103
static inline void diff_flags_or(struct diff_flags *a,
@@ -229,6 +230,7 @@ struct diff_options {
229230
unsigned color_moved_ws_handling;
230231

231232
struct repository *repo;
233+
struct option *parseopts;
232234
};
233235

234236
void diff_emit_submodule_del(struct diff_options *o, const char *line);

0 commit comments

Comments
 (0)