On Wed, Feb 23 2022, Junio C Hamano wrote:
> Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes:
>
>> - printf("\n%s\n", _(desc));
>> + putchar('\n');
>> + puts(_(desc));
>
> This is sort of "Meh". Even the justification that says "we'll do
> the same thing in future patches" is not really a justification, as
> it is entirely fine to add more of the "line-break plus %\n" printf()
> in the later steps in the same series.
Yes, I agree that justification wouldn't make sense, "let's change this
for consistency with code that doesn't exist yet and I'm about to
introduce" is a non-starter as an argument.
But that's not what I mentioned in the commit message or why I changed
this, as noted it's for doing the same "as other existing code in the
file does".
I.e. you'll see that adjacent and related code if you run this on
master:
git grep -W '\\n' -- help.c
Now, I fully agree that's not a *strong* reason to change this, it could
just be left in place.
I just thought post-series that skimming through those related functions
made for marginally easier reading if they all used the same pattern to
accomplish the same thing.
In any case, I don't at all feel strongly about including this change,
so I can drop it if you'd like. I just wanted to clarify why I changed
it.
>> print_command_list(cmds, mask, longest);
>> }
>> free(cmds);
>> @@ -317,7 +318,7 @@ void list_commands(struct cmdnames *main_cmds, struct cmdnames *other_cmds)
>> }
>>
>> if (other_cmds->cnt) {
>> - printf_ln(_("git commands available from elsewhere on your $PATH"));
>> + puts(_("git commands available from elsewhere on your $PATH"));
>
> This *IS* an improvement, as the first parameter to printf_ln() is
> supposed to be a format string, and should have been
>
> printf_ln("%s", _("git commands ..."));
>
>> - printf_ln(_("See 'git help <command>' to read about a specific subcommand"));
>> + puts(_("See 'git help <command>' to read about a specific subcommand"));
>
> Ditto.
