Re: [PATCH v6 1/8] fetch: lowercase error messages

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 16, 2021, 05:19 UTC
Message-ID: <xmqqczn0d6er.fsf@gitster.g>
In-Reply-To: <20211113033358.2179376-2-andersk@mit.edu>

Anders Kaseorg <andersk@mit.edu> writes:

> Documentation/CodingGuidelines says “do not end error messages with a
> full stop” and “do not capitalize the first word”.  Reviewers requested
> updating the existing messages to comply with these guidelines prior to
> the following patches.

Thanks. Whether reviewers requested or you thought of it on your own, separating such a preliminary clean-up into its own patch would be a good idea, especially if the later patches need to update (some of) them.

Show 16 quoted lines

> @@ -1062,13 +1062,13 @@ static void close_fetch_head(struct fetch_head *fetch_head)
>  }
>  
>  static const char warn_show_forced_updates[] =
> -N_("Fetch normally indicates which branches had a forced update,\n"
> -   "but that check has been disabled. To re-enable, use '--show-forced-updates'\n"
> -   "flag or run 'git config fetch.showForcedUpdates true'.");
> +N_("fetch normally indicates which branches had a forced update,\n"
> +   "but that check has been disabled; to re-enable, use '--show-forced-updates'\n"
> +   "flag or run 'git config fetch.showForcedUpdates true'");
>  static const char warn_time_show_forced_updates[] =
> -N_("It took %.2f seconds to check forced updates. You can use\n"
> +N_("it took %.2f seconds to check forced updates; you can use\n"
>     "'--no-show-forced-updates' or run 'git config fetch.showForcedUpdates false'\n"
> -   " to avoid this check.\n");
> +   " to avoid this check\n");

The two guidelines cited in the proposed log message are primarily to prefer

    fatal: unrecognized argument: --no-such-option

over

    fatal: Unrecognized argument: --no-such-option.

and does not say much what to do to a multi-sentence message. In this part (and other parts) of the patch, I can see that you thought this one through when preparing this patch. I very much appreciate it.

The approach chosen (consistently) in this patch is to

 (1) turn them into a (semi) single sentence, concatenated with ';'

 (2) as a side effect of not being a free-standing sentence anymore,
     the second and subsequent sentences in the original, that are
     now just pieces in a single sentence separated with ';', do not
     get capitalized, and

 (3) the sentence as a whole lacks the full-stop, just like a single
     sentence message.

I think we are fine with these rules, especially given that these multi-sentence messages are not the main part of this topic touches and are not the primary focus of this topic anyway.

I am highlighting this part of the change, just in case others think of a better set of rules to follow. Existing multi-sentence messages follow different ad-hoc patterns, it seems (e.g. "git show 00000000").

Thanks.