Re: [PATCH] check-ignore: --non-matching without --verbose

From: Derrick Stolee <derrickstolee@github.com>
To: Carl Smedstad via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org
Cc: Carl Smedstad <carl.smedstad@protonmail.com>
Subject: Re: [PATCH] check-ignore: --non-matching without --verbose
Date: Mon, 23 May 2022 16:44:55 -0400	[thread overview]
Message-ID: <362128ff-690d-8dc1-88fd-620f52b14d54@github.com> (raw)
In-Reply-To: <pull.1236.git.1653310466062.gitgitgadget@gmail.com>

On 5/23/2022 8:54 AM, Carl Smedstad via GitGitGadget wrote:
> From: Carl Smedstad <carl.smedstad@protonmail.com>

>  -n, --non-matching::
> -	Show given paths which don't match any pattern.	 This only
> -	makes sense when `--verbose` is enabled, otherwise it would
> -	not be possible to distinguish between paths which match a
> -	pattern and those which don't.
> +	Only show given paths which don't match any pattern. If `--verbose` is
> +	enabled, show both paths that match a pattern and those which don't.

This may be better to indicate the behavior as predicated on the
existence of --verbose:

	If `--verbose` is enabled, then all paths are listed along
	with an indicator (`::`) that no matching pattern was found.
	Without `--verbose`, list only the paths that do not match
	any pattern.

> -		if (!quiet && (pattern || show_non_matching))
> -			output_pattern(pathspec.items[i].original, pattern);
> +		if (!quiet) {
> +			if (verbose) {
> +				if (show_non_matching || pattern)
> +					output_pattern(pathspec.items[i].original, pattern);
> +			} else {
> +				if (show_non_matching && !pattern)
> +					output_pattern(pathspec.items[i].original, pattern);
> +				if (!show_non_matching && pattern)
> +					output_pattern(pathspec.items[i].original, pattern);

These three blocks all call the same code line. So really you want
to avoid a single case:

	if (!quiet &&
	    ((verbose && (show_non_matching || pattern)) ||
	     (!verbose && !!show_non_matching != !!pattern)))

This is the most direct way to write what you had above. However,
we could do this more simply:

	/* If --non-matching, then show if verbose or the pattern is missing. */
	if (!quiet && show_non_matching && (verbose || !pattern))
		output_pattern(...);

	/* If not --non-matching, then show if the pattern exists. */
	if (!quiet && !show_non_matching && pattern)
		output_pattern(...);

Hopefully that's a bit easier to parse. I believe it is
equivalent.

Thanks,
-Stolee