On Fri, Oct 08 2021, Jeff King wrote:
> On Fri, Oct 08, 2021 at 10:36:02PM -0400, Jeff King wrote:
>
>> If that were coupled with, say, an advise() call to explain that output
>> and matching might be inaccurate (and show that _once_), that might
>> might it more clear what's going on.
>>
>> Now I am sympathetic to flooding the user with too many messages, and
>> maybe reducing this to a single instance of "some commit messages could
>> not be re-encoded; output and matching might be inaccurate" is the right
>> thing. But in a sense, it's also working as designed: what you asked for
>> is producing wrong output over and over, and Git is saying so.
>
> The single-output version would perhaps be something like this:
>
> diff --git a/pretty.c b/pretty.c
> index 708b618cfe..c86f41bae7 100644
> --- a/pretty.c
> +++ b/pretty.c
> @@ -606,6 +606,21 @@ static char *replace_encoding_header(char *buf, const char *encoding)
> return strbuf_detach(&tmp, NULL);
> }
>
> +static void show_encoding_warning(const char *output_encoding)
> +{
> + static int seen_warning;
> +
> + if (seen_warning)
> + return;
> +
> + seen_warning = 1;
> + warning("one or more commits could not be re-encoded to '%s'",
> + output_encoding);
> + advise("When re-encoding fails, some output may be in an unexpected\n"
> + "encoding, and pattern matches against commit data may be\n"
> + "inaccurate.");
> +}
> +
> const char *repo_logmsg_reencode(struct repository *r,
> const struct commit *commit,
> char **commit_encoding,
> @@ -673,7 +688,7 @@ const char *repo_logmsg_reencode(struct repository *r,
> * case we just return the commit message verbatim.
> */
> if (!out) {
> - warning("unable to reencode commit to '%s'", output_encoding);
> + show_encoding_warning(output_encoding);
> return msg;
> }
> return out;
*Poke* about this. We're getting pretty close to release. I think the
WIP hunk I posted in
https://lore.kernel.org/git/871r4umfnm.fsf@xxxxxxxxxxxxxxxxxxx/ presents
a good way forward with this.
I.e. aside from how wise it is to warn about this in general, I think
there's a pretty bad bug in your implementation where what should
effectively be a parse_options() or git_config()-time one-off warning is
being fired off for every single commit in "git log" in some cases.
