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

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 20, 2021, 20:17 UTC
Message-ID: <xmqqmtpbub07.fsf@gitster.g>
In-Reply-To: <20210820004604.9948-2-tbperrotta@gmail.com>

Thiago Perrotta <tbperrotta@gmail.com> 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.

Show 16 quoted lines

> Signed-off-by: Thiago Perrotta <tbperrotta@gmail.com>
> ---
>  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);
>  }