Skip to content

Commit f427b94

Browse files
gitstergitster
committed
Merge branch 'cc/skip-to-optional-val'
Introduce a helper to simplify code to parse a common pattern that expects either "--key" or "--key=<something>". * cc/skip-to-optional-val: t4045: reindent to make helpers readable diff: add tests for --relative without optional prefix value diff: use skip_to_optional_arg_default() in parsing --relative diff: use skip_to_optional_arg_default() diff: use skip_to_optional_arg() index-pack: use skip_to_optional_arg() git-compat-util: introduce skip_to_optional_arg()
2 parents 5abbdbb + f1e4fb2 commit f427b94

5 files changed

Lines changed: 126 additions & 90 deletions

File tree

‎builtin/index-pack.c‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1660,10 +1660,7 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)
16601660
from_stdin = 1;
16611661
} else if (!strcmp(arg, "--fix-thin")) {
16621662
fix_thin_pack = 1;
1663-
} else if (!strcmp(arg, "--strict")) {
1664-
strict = 1;
1665-
do_fsck_object = 1;
1666-
} else if (skip_prefix(arg, "--strict=", &arg)) {
1663+
} else if (skip_to_optional_arg(arg, "--strict", &arg)) {
16671664
strict = 1;
16681665
do_fsck_object = 1;
16691666
fsck_set_msg_types(&fsck_options, arg);
@@ -1679,10 +1676,8 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)
16791676
verify = 1;
16801677
show_stat = 1;
16811678
stat_only = 1;
1682-
} else if (!strcmp(arg, "--keep")) {
1683-
keep_msg = "";
1684-
} else if (starts_with(arg, "--keep=")) {
1685-
keep_msg = arg + 7;
1679+
} else if (skip_to_optional_arg(arg, "--keep", &keep_msg)) {
1680+
; /* nothing to do */
16861681
} else if (starts_with(arg, "--threads=")) {
16871682
char *end;
16881683
nr_threads = strtoul(arg+10, &end, 0);

‎diff.c‎

Lines changed: 16 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -4512,17 +4512,12 @@ int diff_opt_parse(struct diff_options *options,
45124512
options->output_format |= DIFF_FORMAT_NUMSTAT;
45134513
else if (!strcmp(arg, "--shortstat"))
45144514
options->output_format |= DIFF_FORMAT_SHORTSTAT;
4515-
else if (!strcmp(arg, "-X") || !strcmp(arg, "--dirstat"))
4516-
return parse_dirstat_opt(options, "");
4517-
else if (skip_prefix(arg, "-X", &arg))
4518-
return parse_dirstat_opt(options, arg);
4519-
else if (skip_prefix(arg, "--dirstat=", &arg))
4515+
else if (skip_prefix(arg, "-X", &arg) ||
4516+
skip_to_optional_arg(arg, "--dirstat", &arg))
45204517
return parse_dirstat_opt(options, arg);
45214518
else if (!strcmp(arg, "--cumulative"))
45224519
return parse_dirstat_opt(options, "cumulative");
4523-
else if (!strcmp(arg, "--dirstat-by-file"))
4524-
return parse_dirstat_opt(options, "files");
4525-
else if (skip_prefix(arg, "--dirstat-by-file=", &arg)) {
4520+
else if (skip_to_optional_arg(arg, "--dirstat-by-file", &arg)) {
45264521
parse_dirstat_opt(options, "files");
45274522
return parse_dirstat_opt(options, arg);
45284523
}
@@ -4544,22 +4539,22 @@ int diff_opt_parse(struct diff_options *options,
45444539
return stat_opt(options, av);
45454540

45464541
/* renames options */
4547-
else if (starts_with(arg, "-B") || starts_with(arg, "--break-rewrites=") ||
4548-
!strcmp(arg, "--break-rewrites")) {
4542+
else if (starts_with(arg, "-B") ||
4543+
skip_to_optional_arg(arg, "--break-rewrites", NULL)) {
45494544
if ((options->break_opt = diff_scoreopt_parse(arg)) == -1)
45504545
return error("invalid argument to -B: %s", arg+2);
45514546
}
4552-
else if (starts_with(arg, "-M") || starts_with(arg, "--find-renames=") ||
4553-
!strcmp(arg, "--find-renames")) {
4547+
else if (starts_with(arg, "-M") ||
4548+
skip_to_optional_arg(arg, "--find-renames", NULL)) {
45544549
if ((options->rename_score = diff_scoreopt_parse(arg)) == -1)
45554550
return error("invalid argument to -M: %s", arg+2);
45564551
options->detect_rename = DIFF_DETECT_RENAME;
45574552
}
45584553
else if (!strcmp(arg, "-D") || !strcmp(arg, "--irreversible-delete")) {
45594554
options->irreversible_delete = 1;
45604555
}
4561-
else if (starts_with(arg, "-C") || starts_with(arg, "--find-copies=") ||
4562-
!strcmp(arg, "--find-copies")) {
4556+
else if (starts_with(arg, "-C") ||
4557+
skip_to_optional_arg(arg, "--find-copies", NULL)) {
45634558
if (options->detect_rename == DIFF_DETECT_COPY)
45644559
options->flags.find_copies_harder = 1;
45654560
if ((options->rename_score = diff_scoreopt_parse(arg)) == -1)
@@ -4572,11 +4567,10 @@ int diff_opt_parse(struct diff_options *options,
45724567
options->flags.rename_empty = 1;
45734568
else if (!strcmp(arg, "--no-rename-empty"))
45744569
options->flags.rename_empty = 0;
4575-
else if (!strcmp(arg, "--relative"))
4576-
options->flags.relative_name = 1;
4577-
else if (skip_prefix(arg, "--relative=", &arg)) {
4570+
else if (skip_to_optional_arg_default(arg, "--relative", &arg, NULL)) {
45784571
options->flags.relative_name = 1;
4579-
options->prefix = arg;
4572+
if (arg)
4573+
options->prefix = arg;
45804574
}
45814575

45824576
/* xdiff options */
@@ -4646,9 +4640,7 @@ int diff_opt_parse(struct diff_options *options,
46464640
else if (!strcmp(arg, "--no-follow")) {
46474641
options->flags.follow_renames = 0;
46484642
options->flags.default_follow_renames = 0;
4649-
} else if (!strcmp(arg, "--color"))
4650-
options->use_color = 1;
4651-
else if (skip_prefix(arg, "--color=", &arg)) {
4643+
} else if (skip_to_optional_arg_default(arg, "--color", &arg, "always")) {
46524644
int value = git_config_colorbool(NULL, arg);
46534645
if (value < 0)
46544646
return error("option `color' expects \"always\", \"auto\", or \"never\"");
@@ -4668,14 +4660,9 @@ int diff_opt_parse(struct diff_options *options,
46684660
if (cm < 0)
46694661
die("bad --color-moved argument: %s", arg);
46704662
options->color_moved = cm;
4671-
} else if (!strcmp(arg, "--color-words")) {
4672-
options->use_color = 1;
4673-
options->word_diff = DIFF_WORDS_COLOR;
4674-
}
4675-
else if (skip_prefix(arg, "--color-words=", &arg)) {
4663+
} else if (skip_to_optional_arg_default(arg, "--color-words", &options->word_regex, NULL)) {
46764664
options->use_color = 1;
46774665
options->word_diff = DIFF_WORDS_COLOR;
4678-
options->word_regex = arg;
46794666
}
46804667
else if (!strcmp(arg, "--word-diff")) {
46814668
if (options->word_diff == DIFF_WORDS_NONE)
@@ -4714,15 +4701,10 @@ int diff_opt_parse(struct diff_options *options,
47144701
options->flags.textconv_set_via_cmdline = 1;
47154702
} else if (!strcmp(arg, "--no-textconv"))
47164703
options->flags.allow_textconv = 0;
4717-
else if (!strcmp(arg, "--ignore-submodules")) {
4718-
options->flags.override_submodule_config = 1;
4719-
handle_ignore_submodules_arg(options, "all");
4720-
} else if (skip_prefix(arg, "--ignore-submodules=", &arg)) {
4704+
else if (skip_to_optional_arg_default(arg, "--ignore-submodules", &arg, "all")) {
47214705
options->flags.override_submodule_config = 1;
47224706
handle_ignore_submodules_arg(options, arg);
4723-
} else if (!strcmp(arg, "--submodule"))
4724-
options->submodule_format = DIFF_SUBMODULE_LOG;
4725-
else if (skip_prefix(arg, "--submodule=", &arg))
4707+
} else if (skip_to_optional_arg_default(arg, "--submodule", &arg, "log"))
47264708
return parse_submodule_opt(options, arg);
47274709
else if (skip_prefix(arg, "--ws-error-highlight=", &arg))
47284710
return parse_ws_error_highlight_opt(options, arg);

‎git-compat-util.h‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -484,6 +484,29 @@ static inline int skip_prefix(const char *str, const char *prefix,
484484
return 0;
485485
}
486486

487+
/*
488+
* If the string "str" is the same as the string in "prefix", then the "arg"
489+
* parameter is set to the "def" parameter and 1 is returned.
490+
* If the string "str" begins with the string found in "prefix" and then a
491+
* "=" sign, then the "arg" parameter is set to "str + strlen(prefix) + 1"
492+
* (i.e., to the point in the string right after the prefix and the "=" sign),
493+
* and 1 is returned.
494+
*
495+
* Otherwise, return 0 and leave "arg" untouched.
496+
*
497+
* When we accept both a "--key" and a "--key=<val>" option, this function
498+
* can be used instead of !strcmp(arg, "--key") and then
499+
* skip_prefix(arg, "--key=", &arg) to parse such an option.
500+
*/
501+
int skip_to_optional_arg_default(const char *str, const char *prefix,
502+
const char **arg, const char *def);
503+
504+
static inline int skip_to_optional_arg(const char *str, const char *prefix,
505+
const char **arg)
506+
{
507+
return skip_to_optional_arg_default(str, prefix, arg, "");
508+
}
509+
487510
/*
488511
* Like skip_prefix, but promises never to read past "len" bytes of the input
489512
* buffer, and returns the remaining number of bytes in "out" via "outlen".

‎strbuf.c‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,28 @@ int starts_with(const char *str, const char *prefix)
1111
return 0;
1212
}
1313

14+
int skip_to_optional_arg_default(const char *str, const char *prefix,
15+
const char **arg, const char *def)
16+
{
17+
const char *p;
18+
19+
if (!skip_prefix(str, prefix, &p))
20+
return 0;
21+
22+
if (!*p) {
23+
if (arg)
24+
*arg = def;
25+
return 1;
26+
}
27+
28+
if (*p != '=')
29+
return 0;
30+
31+
if (arg)
32+
*arg = p + 1;
33+
return 1;
34+
}
35+
1436
/*
1537
* Used as the default ->buf value, so that people can always assume
1638
* buf is non NULL and ->buf is NUL terminated even for a freshly

‎t/t4045-diff-relative.sh‎

Lines changed: 62 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -12,62 +12,76 @@ test_expect_success 'setup' '
1212
git commit -m one
1313
'
1414

15-
check_diff() {
16-
expect=$1; shift
17-
cat >expected <<EOF
18-
diff --git a/$expect b/$expect
19-
new file mode 100644
20-
index 0000000..25c05ef
21-
--- /dev/null
22-
+++ b/$expect
23-
@@ -0,0 +1 @@
24-
+other content
25-
EOF
26-
test_expect_success "-p $*" "
27-
git diff -p $* HEAD^ >actual &&
28-
test_cmp expected actual
29-
"
15+
check_diff () {
16+
dir=$1
17+
shift
18+
expect=$1
19+
shift
20+
cat >expected <<-EOF
21+
diff --git a/$expect b/$expect
22+
new file mode 100644
23+
index 0000000..25c05ef
24+
--- /dev/null
25+
+++ b/$expect
26+
@@ -0,0 +1 @@
27+
+other content
28+
EOF
29+
test_expect_success "-p $*" "
30+
git -C '$dir' diff -p $* HEAD^ >actual &&
31+
test_cmp expected actual
32+
"
3033
}
3134

32-
check_numstat() {
33-
expect=$1; shift
34-
cat >expected <<EOF
35-
1 0 $expect
36-
EOF
37-
test_expect_success "--numstat $*" "
38-
echo '1 0 $expect' >expected &&
39-
git diff --numstat $* HEAD^ >actual &&
40-
test_cmp expected actual
41-
"
35+
check_numstat () {
36+
dir=$1
37+
shift
38+
expect=$1
39+
shift
40+
cat >expected <<-EOF
41+
1 0 $expect
42+
EOF
43+
test_expect_success "--numstat $*" "
44+
echo '1 0 $expect' >expected &&
45+
git -C '$dir' diff --numstat $* HEAD^ >actual &&
46+
test_cmp expected actual
47+
"
4248
}
4349

44-
check_stat() {
45-
expect=$1; shift
46-
cat >expected <<EOF
47-
$expect | 1 +
48-
1 file changed, 1 insertion(+)
49-
EOF
50-
test_expect_success "--stat $*" "
51-
git diff --stat $* HEAD^ >actual &&
52-
test_i18ncmp expected actual
53-
"
50+
check_stat () {
51+
dir=$1
52+
shift
53+
expect=$1
54+
shift
55+
cat >expected <<-EOF
56+
$expect | 1 +
57+
1 file changed, 1 insertion(+)
58+
EOF
59+
test_expect_success "--stat $*" "
60+
git -C '$dir' diff --stat $* HEAD^ >actual &&
61+
test_i18ncmp expected actual
62+
"
5463
}
5564

56-
check_raw() {
57-
expect=$1; shift
58-
cat >expected <<EOF
59-
:000000 100644 0000000000000000000000000000000000000000 25c05ef3639d2d270e7fe765a67668f098092bc5 A $expect
60-
EOF
61-
test_expect_success "--raw $*" "
62-
git diff --no-abbrev --raw $* HEAD^ >actual &&
63-
test_cmp expected actual
64-
"
65+
check_raw () {
66+
dir=$1
67+
shift
68+
expect=$1
69+
shift
70+
cat >expected <<-EOF
71+
:000000 100644 0000000000000000000000000000000000000000 25c05ef3639d2d270e7fe765a67668f098092bc5 A $expect
72+
EOF
73+
test_expect_success "--raw $*" "
74+
git -C '$dir' diff --no-abbrev --raw $* HEAD^ >actual &&
75+
test_cmp expected actual
76+
"
6577
}
6678

67-
for type in diff numstat stat raw; do
68-
check_$type file2 --relative=subdir/
69-
check_$type file2 --relative=subdir
70-
check_$type dir/file2 --relative=sub
79+
for type in diff numstat stat raw
80+
do
81+
check_$type . file2 --relative=subdir/
82+
check_$type . file2 --relative=subdir
83+
check_$type subdir file2 --relative
84+
check_$type . dir/file2 --relative=sub
7185
done
7286

7387
test_done

0 commit comments

Comments
 (0)