Thread (44 messages) flat view 44 messages, 5 authors, 2021-10-26

Re: [PATCH v2 1/3] send-email: print newline for --git-completion-helper

From: Junio C Hamano <hidden>
Date: 2021-08-20 20:17:15

Possibly related (same subject, not in this thread)

Thiago Perrotta [off-list ref] writes:
Rationale: Currently all git built-in commands print a newline upon upon git
<cmd> --git-completion-helper. Therefore git-send-email should follow suit for
consistency.
"upon upon".

You do not need to start with "Rationale", because one of the things
you need to have in any proposed log message is a justification for
the change.  You also do not need "Currently" in our log message, as
the convention here is to state the status quo in the present tense,
point out what's wrong there (or leave it unsaid and implied by the
description of the current state, if it is obvious), state the
approach to correct what's wrong, and finally give an order to the
codebase to "become like so".

	Unlike other Git subcommands, "git send-email" leaves its
	output an incomplete line when "--git-completion-helper" is
	asked.  Be consistent by terminating the message with LF
	here.

or something like that.

I do not know which style is preferred among

 (1)	print something;
	print "\n";

 (2)	print something, "\n";

 (3)	print something . "\n";

but other than that, the goal and the implementation both sound
sensible (the second one is what I'd be writing if I were doing this
change myself, FWIW).

Thanks.
quoted hunk
Signed-off-by: Thiago Perrotta <redacted>
---
 git-send-email.perl | 1 +
 1 file changed, 1 insertion(+)
diff --git a/git-send-email.perl b/git-send-email.perl
index e65d969d0b..e991bf333d 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -115,6 +115,7 @@ sub usage {
 
 sub completion_helper {
     print Git::command('format-patch', '--git-completion-helper');
+    print "\n";
     exit(0);
 }
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help