diff-filter: be more careful when looking for negative bits
The `--diff-filter=<bits>` option allows to filter the diff by certain criteria, for example `R` to only show renamed files. It also supports negating a filter via a down-cased letter, i.e. `r` to show _everything but_ renamed files. However, the code is a bit overzealous when trying to figure out whether `git diff` should start with all diff-filters turned on because the user provided a lower-case letter: if the `--diff-filter` argument starts with an upper-case letter, we must not start with all bits turned on. Even worse, it is possible to specify the diff filters in multiple, separate options, e.g. `--diff-filter=AM [...] --diff-filter=m`. Let's accumulate the include/exclude filters independently, and only special-case the "only exclude filters were specified" case after parsing the options altogether. Note: The code replaced by this commit took pains to avoid setting any unused bits of `options->filter`. That was unnecessary, though, as all accesses happen via the `filter_bit_tst()` function using specific bits, and setting the unused bits has no effect. Therefore, we can simplify the code by using `~0` (or in this instance, `~<unwanted-bit>`). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
parent
4d4d4eaa7b
commit
75408ca949
23
diff.c
23
diff.c
@ -4720,6 +4720,12 @@ void diff_setup_done(struct diff_options *options)
|
||||
if (!options->use_color || external_diff())
|
||||
options->color_moved = 0;
|
||||
|
||||
if (options->filter_not) {
|
||||
if (!options->filter)
|
||||
options->filter = ~filter_bit[DIFF_STATUS_FILTER_AON];
|
||||
options->filter &= ~options->filter_not;
|
||||
}
|
||||
|
||||
FREE_AND_NULL(options->parseopts);
|
||||
}
|
||||
|
||||
@ -4820,21 +4826,6 @@ static int diff_opt_diff_filter(const struct option *option,
|
||||
BUG_ON_OPT_NEG(unset);
|
||||
prepare_filter_bits();
|
||||
|
||||
/*
|
||||
* If there is a negation e.g. 'd' in the input, and we haven't
|
||||
* initialized the filter field with another --diff-filter, start
|
||||
* from full set of bits, except for AON.
|
||||
*/
|
||||
if (!opt->filter) {
|
||||
for (i = 0; (optch = optarg[i]) != '\0'; i++) {
|
||||
if (optch < 'a' || 'z' < optch)
|
||||
continue;
|
||||
opt->filter = (1 << (ARRAY_SIZE(diff_status_letters) - 1)) - 1;
|
||||
opt->filter &= ~filter_bit[DIFF_STATUS_FILTER_AON];
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
for (i = 0; (optch = optarg[i]) != '\0'; i++) {
|
||||
unsigned int bit;
|
||||
int negate;
|
||||
@ -4851,7 +4842,7 @@ static int diff_opt_diff_filter(const struct option *option,
|
||||
return error(_("unknown change class '%c' in --diff-filter=%s"),
|
||||
optarg[i], optarg);
|
||||
if (negate)
|
||||
opt->filter &= ~bit;
|
||||
opt->filter_not |= bit;
|
||||
else
|
||||
opt->filter |= bit;
|
||||
}
|
||||
|
2
diff.h
2
diff.h
@ -283,7 +283,7 @@ struct diff_options {
|
||||
struct diff_flags flags;
|
||||
|
||||
/* diff-filter bits */
|
||||
unsigned int filter;
|
||||
unsigned int filter, filter_not;
|
||||
|
||||
int use_color;
|
||||
|
||||
|
@ -142,6 +142,19 @@ test_expect_success 'diff-filter=R' '
|
||||
|
||||
'
|
||||
|
||||
test_expect_success 'multiple --diff-filter bits' '
|
||||
|
||||
git log -M --pretty="format:%s" --diff-filter=R HEAD >expect &&
|
||||
git log -M --pretty="format:%s" --diff-filter=Ra HEAD >actual &&
|
||||
test_cmp expect actual &&
|
||||
git log -M --pretty="format:%s" --diff-filter=aR HEAD >actual &&
|
||||
test_cmp expect actual &&
|
||||
git log -M --pretty="format:%s" \
|
||||
--diff-filter=a --diff-filter=R HEAD >actual &&
|
||||
test_cmp expect actual
|
||||
|
||||
'
|
||||
|
||||
test_expect_success 'diff-filter=C' '
|
||||
|
||||
git log -C -C --pretty="format:%s" --diff-filter=C HEAD >actual &&
|
||||
|
Loading…
Reference in New Issue
Block a user