Re: [PATCH v8 2/2] send-email: expose header information to git-send-email's sendemail-validate hook

> On Wed, Jan 18 2023, Michael Strawbridge wrote:
>
>> To allow further flexibility in the Git hook, the SMTP header
>> information of the email which git-send-email intends to send, is now
>> passed as the 2nd argument to the sendemail-validate hook.
>>
>> As an example, this can be useful for acting upon keywords in the
>> subject or specific email addresses.
>>
>> As a consequence of needing all the header data, validation has been
>> moved later in the sequence to right before sending the emails instead
>> of at the beginning.
> Ah, I see. I tested this (i.e. moving it back to the previous behavior)
> and you did this change because you don't have the $sender variable yet.
>
> I tried this quickly on top, which seems to work, i.e. now we do this in
> the same order as before, but we just move the $sender code earlier:
> 	
> 	diff --git a/git-send-email.perl b/git-send-email.perl
> 	index d123dfd33d5..7e7681116bb 100755
> 	--- a/git-send-email.perl
> 	+++ b/git-send-email.perl
> 	@@ -787,6 +787,28 @@ sub is_format_patch_arg {
> 	 
> 	 @files = handle_backup_files(@files);
> 	 
> 	+if (defined $sender) {
> 	+	$sender =~ s/^\s+|\s+$//g;
> 	+	($sender) = expand_aliases($sender);
> 	+} else {
> 	+	$sender = $repoauthor->() || $repocommitter->() || '';
> 	+}
> 	+
> 	+# $sender could be an already sanitized address
> 	+# (e.g. sendemail.from could be manually sanitized by user).
> 	+# But it's a no-op to run sanitize_address on an already sanitized address.
> 	+$sender = sanitize_address($sender);
> 	+
> 	+if ($validate) {
> 	+	foreach my $f (@files) {
> 	+		unless (-p $f) {
> 	+		        pre_process_file($f, 1);
> 	+
> 	+			validate_patch($f, $target_xfer_encoding);
> 	+		}
> 	+	}
> 	+}
> 	+
> 	 if (@files) {
> 	 	unless ($quiet) {
> 	 		print $_,"\n" for (@files);
> 	@@ -1035,18 +1057,6 @@ sub file_declares_8bit_cte {
> 	 	}
> 	 }
> 	 
> 	-if (defined $sender) {
> 	-	$sender =~ s/^\s+|\s+$//g;
> 	-	($sender) = expand_aliases($sender);
> 	-} else {
> 	-	$sender = $repoauthor->() || $repocommitter->() || '';
> 	-}
> 	-
> 	-# $sender could be an already sanitized address
> 	-# (e.g. sendemail.from could be manually sanitized by user).
> 	-# But it's a no-op to run sanitize_address on an already sanitized address.
> 	-$sender = sanitize_address($sender);
> 	-
> 	 my $to_whom = __("To whom should the emails be sent (if anyone)?");
> 	 my $prompting = 0;
> 	 if (!@initial_to && !defined $to_cmd) {
> 	@@ -1120,16 +1130,6 @@ sub expand_one_alias {
> 	 
> 	 $time = time - scalar $#files;
> 	 
> 	-if ($validate) {
> 	-	foreach my $f (@files) {
> 	-		unless (-p $f) {
> 	-		        pre_process_file($f, 1);
> 	-
> 	-			validate_patch($f, $target_xfer_encoding);
> 	-		}
> 	-	}
> 	-}
> 	-
> 	 $in_reply_to = $initial_in_reply_to;
> 	 $references = $initial_in_reply_to || '';
> 	 $message_num = 0;
>
> All tests pass with that, which is less good than it sounds, because
> shouldn't your tests be checking whether we have this non--quiet
> print-out of the files as expected before or after the validation hook
> runs?

>> +
>> +			my ($recipients_ref, $to, $date, $gitversion, $cc, $ccline, $header) = gen_header();
>> +
>> +			require File::Temp;
>> +			my ($header_filehandle, $header_filename) = File::Temp::tempfile(
>> +                            ".gitsendemail.header.XXXXXX", DIR => $repo->repo_path());
>> +			print $header_filehandle $header;
>> +
>>  			my @cmd = ("git", "hook", "run", "--ignore-missing",
>>  				    $hook_name, "--");
>> -			my @cmd_msg = (@cmd, "<patch>");
>> -			my @cmd_run = (@cmd, $target);
>> +			my @cmd_msg = (@cmd, "<patch>", "<header>");
>> +			my @cmd_run = (@cmd, $target, $header_filename);
>>  			$hook_error = system_or_msg(\@cmd_run, undef, "@cmd_msg");
>> +			unlink($header_filehandle);
>>  			chdir($cwd_save) or die("chdir: $!");
> I'm still curious about the "stdin" question I asked in the last round.