Re: [PATCH v2 2/4] git-send-email: refactor duplicate $? checks into a function

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 5, 2021, 19:11 UTC
Message-ID: <xmqqmtuc5yx7.fsf@gitster.g>
In-Reply-To: <patch-2.5-f236f083e36-20210404T091649Z-avarab@gmail.com>

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

Show 12 quoted lines

> @@ -1938,6 +1945,12 @@ sub unique_email_list {
>  	return @emails;
>  }
>  
> +sub validate_patch_error {
> +	my ($fn, $error) = @_;
> +	die sprintf(__("fatal: %s: %s\nwarning: no patches were sent\n"),
> +		    $fn, $error);
> +}
> +
>  sub validate_patch {
>  	my ($fn, $xfer_encoding) = @_;

I like the overall direction of this series, but this change will soon be reverted back to have the die/sprintf in the two callsite in 4/4 anyway, so this hunk looks more like "I thought this would be a good way, but in the end I had to change my mind".

Show 25 quoted lines

> @@ -1952,11 +1965,12 @@ sub validate_patch {
>  			chdir($repo->wc_path() or $repo->repo_path())
>  				or die("chdir: $!");
>  			local $ENV{"GIT_DIR"} = $repo->repo_path();
> -			$hook_error = "rejected by sendemail-validate hook"
> -				if system($validate_hook, $target);
> +			if (my $msg = system_or_msg([$validate_hook, $target])) {
> +				$hook_error = __("rejected by sendemail-validate hook");
> +			}
>  			chdir($cwd_save) or die("chdir: $!");
>  		}
> -		return $hook_error if $hook_error;
> +		validate_patch_error($fn, $hook_error) if $hook_error;
>  	}
>  
>  	# Any long lines will be automatically fixed if we use a suitable transfer
> @@ -1966,7 +1980,7 @@ sub validate_patch {
>  			or die sprintf(__("unable to open %s: %s\n"), $fn, $!);
>  		while (my $line = <$fh>) {
>  			if (length($line) > 998) {
> -				return sprintf(__("%s: patch contains a line longer than 998 characters"), $.);
> +				validate_patch_error($fn, sprintf(__("%s: patch contains a line longer than 998 characters"), $.));
>  			}
>  		}
>  	}