Re: format-patch base-commit: moving to above the patch?

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Josh Triplett <josh@xxxxxxxxxxxxxxxx> writes:

> Currently, format-patch puts base-commit and prerequisite-patch-id
> information below the patch, and below the email signature.  Most mail
> clients automatically trim everything below the signature marker as
> unimportant when quoting a mail for a reply, which would make it
> difficult for someone to reply, quote the base-commit, and say something
> like "I don't have this commit, where did it come from?" or "Can you
> please rebase this on ...".
>
> Might it make sense to move this information adjacent to the diffstat,
> instead?  Or, at least, above the email signature?

I personally feel that it would be annoying to have them near
diffstat, especially given that unbounded many prereq patches can be
listed.  It would not be too bad to flip the order between the call
to print_signature() and print_bases(), though.  The extent of the
change needed to (note: not even compile-tested) does does not look
too bad, either.

I did not carefully think what the right adjustment for the MIME
case is, though.

I would expect some tests that expect the current order of the tail
end of the output to break, which you would need to adjust.  And if
there is no such test right now, you should add one, as your inquiry
and this patch _sets_ a concrete expectation as to what should come
before the signature line, which future updates should not break.


 builtin/log.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index 92dc34d..d69d5e6 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1042,7 +1042,6 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
 	diff_flush(&opts);
 
 	fprintf(rev->diffopt.file, "\n");
-	print_signature(rev->diffopt.file);
 }
 
 static const char *clean_message_id(const char *msg_id)
@@ -1720,6 +1719,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		make_cover_letter(&rev, use_stdout,
 				  origin, nr, list, branch_name, quiet);
 		print_bases(&bases, rev.diffopt.file);
+		print_signature(rev.diffopt.file);
 		total++;
 		start_number--;
 	}
@@ -1779,13 +1779,13 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		if (!use_stdout)
 			rev.shown_one = 0;
 		if (shown) {
+			print_bases(&bases, rev.diffopt.file);
 			if (rev.mime_boundary)
 				fprintf(rev.diffopt.file, "\n--%s%s--\n\n\n",
 				       mime_boundary_leader,
 				       rev.mime_boundary);
 			else
 				print_signature(rev.diffopt.file);
-			print_bases(&bases, rev.diffopt.file);
 		}
 		if (!use_stdout)
 			fclose(rev.diffopt.file);



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]