Re: [PATCH 2/2] diff-filter: be more careful when looking for negative bits

[Taylor Blau <me@ttaylorr.com>]

On Tue, Jan 25, 2022 at 10:29:19PM +0000, Johannes Schindelin via GitGitGadget wrote:
> diff --git a/diff.c b/diff.c
> index c862771a589..fc1151b9c73 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -4821,17 +4821,15 @@ static int diff_opt_diff_filter(const struct option *option,
>  	prepare_filter_bits();
>
>  	/*
> -	 * If there is a negation e.g. 'd' in the input, and we haven't
> +	 * If the input starts with a negation e.g. 'd', 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;
> +		optch = optarg[0];
> +		if (optch >= 'a' && 'z' >= optch) {
>  			opt->filter = (1 << (ARRAY_SIZE(diff_status_letters) - 1)) - 1;
>  			opt->filter &= ~filter_bit[DIFF_STATUS_FILTER_AON];
> -			break;
>  		}
>  	}

Thinking through how this would have worked before with
`--diff-filter=Dr`, I think it goes something like:

  1. We set all bits (except the all-or-none bit) on via the first loop.
  2. Then we OR in the bit for deletions, which does not change the
     overall filter (since it was already set by the previous step).
  3. Then we unset the bit corresponding to renames.

That leaves us with all bits on except two: DIFF_STATUS_RENAMED and
DIFF_STATUS_FILTER_AON.

As far as I can understand, the AON "filter" shows all files as long as
at least one of them matches the filter, otherwise it shows nothing at
all.

But that doesn't save us, since we have many more bits on than we should
have, meaning that `--diff-filter=Dr` doesn't work at all (assuming you
expected it to show just deletions, like `--diff-filter=D` does).

It's possible that I don't understand what the all-or-nothing bit is
supposed to be doing, though.

Thanks,
Taylor