Subject: Re: [PATCH 1/1] clean: further clean-up of implementation around "--force"

From: Junio C Hamano <[email-masked]>
Date: Sun, 03 Mar 2024 14:18:38 -0800

Junio C Hamano <[email-masked]> writes:

> @@ -950,13 +950,8 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
>  	argc = parse_options(argc, argv, prefix, options, builtin_clean_usage,
>  			     0);
>  
> -	/* Dry run won't remove anything, so requiring force makes no sense */
> -	if (dry_run)
> -		require_force = 0;
> -
> -	if (require_force != 0 && !force && !interactive)
> -		die(_("clean.requireForce is true and neither -f nor -i given:"
> -				  " refusing to clean"));
> +	if (require_force != 0 && !force && !interactive && !dry_run)
> +		die(_("clean.requireForce is true and -f not given: refusing to clean"));
>  
>  	if (force > 1)
>  		rm_flags = 0;

An obvious alternative way to clean-up the logic is to do this
instead:

	if (dry_run || interactive))
		require_force = 0;
 	if (require_force != 0 && !force)
		die(_("clean.requireForce is true and ..."));

But as I wrote, the most important improvement done by Sergey's
patch was to remove the dual meaning of the "force" variable so that
it indicates if the "--force" option was given and nothing else,
while the "require_force" variable indicates if clean.requireForce
was given and nothing else.  From that point of view, the
conditional tweaking done to require_force in the above alternative
makes the code worse, relative to Sergey's patch, and certainly to
its follow up, my patch about "--interactive".