git log --encoding=HTML is not supported

> Co robiłeś/-aś zanim pojawił się błąd? (Kroki, aby odtworzyć problem)
> { git log --oneline --encoding=HTML stl_function.h; }
> 
> Co powinno się stać? (Oczekiwane zachowanie)
> 828176ba490 libstdc++: Improve doxygen comments in <bits/stl_function.h>
> 
> Co stało się zamiast tego? (Rzeczywiste zachowanie)
> 828176ba490 libstdc++: Improve doxygen comments in <bits/stl_function.h>
> 
> Jaka jest różnica między tym, co powinno się stać, a tym, co się stało?
> Znak początku nazwy pliku jest interpretowany jako znak otwierający znacznik.
> 
> Inne cenne uwagi:
> Błąd u klienta:
> <URL: https://bugs.kde.org/show_bug.cgi?id=441255 >
> 
> Implementacja wtyczki:
>   "--pretty=format:
> <tr>
> <td><a href=\"rev:%h\">%h</a></td> <td>%ad</td> <td>%s</td> <td>%an</td>
> </tr>"
> 
> Podobne zgłoszenie:
> <URL: https://public-inbox.org/git/CACPiFCLzsiUjx-vm-dcd=0E8HezMWkErPyS==OQ7OhaXqR6CUA@mail.gmail.com/ >
> 
> Proponowane rozwiązanie:
> W odróżnieniu od omawianych powyżej trudności używaniem formatu wynikowego
> JSON, moim zdaniem w tym przypadku wystarczyłoby zakodować znaki [<] i [&]
> w treści w sposób odpowiedni dla HTML.
> (To rozwiązanie nie zakłada wykrywania i odrzucania znaków nieprawidłowych.)
> 
> [Informacje o systemie]
> wersja gita:
> git version 2.32.0
> cpu: x86_64
> no commit associated with this build
> sizeof-long: 8
> sizeof-size_t: 8
> shell-path: /bin/sh
> uname: Linux 5.13.12-1-default #1 SMP Wed Aug 18 08:01:38 UTC 2021 (999e604)
> x86_64
> informacje o kompilacji: gnuc: 11.1
> informacje o bibliotece libc: glibc: 2.33
> $SHELL (typically, interactive shell): /bin/bash
> 
> 
> [Włączone skrypty Gita]
> 
> 
> 

> Dnia wtorek, 24 sierpnia 2021 12:31:14 CEST Bagas Sanjaya pisze:
> 
>> Please speak English here (in other words, re-submit git-bugreport
>> without l10n).
> 
> How do I do that?

> On 24/08/21 17.33, Krzysztof Żelechowski wrote:
>> Dnia wtorek, 24 sierpnia 2021 12:31:14 CEST Bagas Sanjaya pisze:
>> 
>>> Please speak English here (in other words, re-submit git-bugreport
>>> without l10n).
>> How do I do that?
>
> You need to set locale to English when executing `git bugreport`:
>
> ```
> LANGUAGE=en_US.UTF-8 LC_ALL=en_US.UTF-8 /path/to/git bugreport
> ```

> Co robiłeś/-aś zanim pojawił się błąd? (Kroki, aby odtworzyć problem)
> { git log --oneline --encoding=HTML stl_function.h; }
> 
> Co powinno się stać? (Oczekiwane zachowanie)
> 828176ba490 libstdc++: Improve doxygen comments in <bits/stl_function.h>
> 
> Co stało się zamiast tego? (Rzeczywiste zachowanie)
> 828176ba490 libstdc++: Improve doxygen comments in <bits/stl_function.h>

> We feed the encoding "HTML" to iconv_open(), which of course has no idea
> what that is. It's unfortunate, though, that we don't even print a
> warning, and instead just quietly leave the text intact. I wonder if we
> should do something like:
>
> diff --git a/pretty.c b/pretty.c
> index 535eb97fa6..708b618cfe 100644
> --- a/pretty.c
> +++ b/pretty.c
> @@ -672,7 +672,11 @@ const char *repo_logmsg_reencode(struct repository *r,
>  	 * If the re-encoding failed, out might be NULL here; in that
>  	 * case we just return the commit message verbatim.
>  	 */
> -	return out ? out : msg;
> +	if (!out) {
> +		warning("unable to reencode commit to '%s'", output_encoding);
> +		return msg;
> +	}
> +	return out;
>  }
>  
>  static int mailmap_name(const char **email, size_t *email_len,

> Jeff King <peff@peff.net> writes:
> 
> > We feed the encoding "HTML" to iconv_open(), which of course has no idea
> > what that is. It's unfortunate, though, that we don't even print a
> > warning, and instead just quietly leave the text intact. I wonder if we
> > should do something like:
> [...]
> This addition sounds quite sensible to me.
> 
> "git log --encoding=bogus" would issue this warning for each and
> every commit and that may be a bit irritating, but being irritating
> may be a good characteristic for a warning message that is given to
> an easily correctable condition.
> 
> I originally thought that the warning would be lost to the pager,
> but apparently I forgot what I did eons ago at 61b80509 (sending
> errors to stdout under $PAGER, 2008-02-16) ;-).

> On Fri, Aug 27, 2021 at 02:30:16PM -0400, Jeff King wrote:
>
>> Here it is polished into a real commit.
>> 
>> Subject: [PATCH] logmsg_reencode(): warn when iconv() fails
>
> And here's a minimal documentation I'd suggest on top. We can discuss
> going further in discussing subtleties of iconv() if we want, but IMHO
> it would work to stop here.

> On Wed, Aug 25, 2021 at 09:31:59AM -0700, Junio C Hamano wrote:
>
>> Jeff King <peff@peff.net> writes:
>> 
>> > We feed the encoding "HTML" to iconv_open(), which of course has no idea
>> > what that is. It's unfortunate, though, that we don't even print a
>> > warning, and instead just quietly leave the text intact. I wonder if we
>> > should do something like:
>> [...]
>> This addition sounds quite sensible to me.
>> 
>> "git log --encoding=bogus" would issue this warning for each and
>> every commit and that may be a bit irritating, but being irritating
>> may be a good characteristic for a warning message that is given to
>> an easily correctable condition.
>> 
>> I originally thought that the warning would be lost to the pager,
>> but apparently I forgot what I did eons ago at 61b80509 (sending
>> errors to stdout under $PAGER, 2008-02-16) ;-).
>
> Here it is polished into a real commit.
>
> -- >8 --
> Subject: [PATCH] logmsg_reencode(): warn when iconv() fails
>
> If the user asks for a pretty-printed commit to be converted (either
> explicitly with --encoding=foo, or implicitly because the commit is
> non-utf8 and we want to convert it), we pass it through iconv(). If that
> fails, we fall back to showing the input verbatim, but don't tell the
> user that the output may be bogus.
>
> Let's add a warning to do so, along with a mention in the documentation
> for --encoding. Two things to note about the implementation:
>
>   - we could produce the warning closer to the call to iconv() in
>     reencode_string_len(), which would let us relay the value of errno.
>     But this is not actually very helpful. reencode_string_len() does
>     not know we are operating on a commit, and indeed does not know that
>     the caller won't produce an error of its own. And the errno values
>     from iconv() are seldom helpful (iconv_open() only ever produces
>     EINVAL; perhaps EILSEQ from iconv() might be illuminating, but it
>     can also return EINVAL for incomplete sequences).
>
>   - if the reason for the failure is that the output charset is not
>     supported, then the user will see this warning for every commit we
>     try to display. That might be ugly and overwhelming, but on the
>     other hand it is making it clear that every one of them has not been
>     converted (and the likely outcome anyway is to re-try the command
>     with a supported output encoding).
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  Documentation/pretty-options.txt | 4 +++-
>  pretty.c                         | 6 +++++-
>  t/t4210-log-i18n.sh              | 7 +++++++
>  3 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt
> index 27ddaf84a1..42b227bc40 100644
> --- a/Documentation/pretty-options.txt
> +++ b/Documentation/pretty-options.txt
> @@ -40,7 +40,9 @@ people using 80-column terminals.
>  	defaults to UTF-8. Note that if an object claims to be encoded
>  	in `X` and we are outputting in `X`, we will output the object
>  	verbatim; this means that invalid sequences in the original
> -	commit may be copied to the output.
> +	commit may be copied to the output. Likewise, if iconv(3) fails
> +	to convert the commit, we will output the original object
> +	verbatim, along with a warning.
>  
>  --expand-tabs=<n>::
>  --expand-tabs::
> diff --git a/pretty.c b/pretty.c
> index 9631529c10..73b5ead509 100644
> --- a/pretty.c
> +++ b/pretty.c
> @@ -671,7 +671,11 @@ const char *repo_logmsg_reencode(struct repository *r,
>  	 * If the re-encoding failed, out might be NULL here; in that
>  	 * case we just return the commit message verbatim.
>  	 */
> -	return out ? out : msg;
> +	if (!out) {
> +		warning("unable to reencode commit to '%s'", output_encoding);
> +		return msg;
> +	}
> +	return out;
>  }
>  
>  static int mailmap_name(const char **email, size_t *email_len,
> diff --git a/t/t4210-log-i18n.sh b/t/t4210-log-i18n.sh
> index d2dfcf164e..0141f36e33 100755
> --- a/t/t4210-log-i18n.sh
> +++ b/t/t4210-log-i18n.sh
> @@ -131,4 +131,11 @@ do
>  	fi
>  done
>  
> +test_expect_success 'log shows warning when conversion fails' '
> +	enc=this-encoding-does-not-exist &&
> +	git log -1 --encoding=$enc 2>err &&
> +	echo "warning: unable to reencode commit to ${SQ}${enc}${SQ}" >expect &&
> +	test_cmp expect err
> +'
> +
>  test_done

> On Fri, Aug 27 2021, Jeff King wrote:
>
> $subject because I think we should really consider backing this out
> before it gets to a real release.
>
> I ran into this while testing the grep coloring patch[1] (but it's
> unrelated). Before this commit e.g.:
>
>     LC_ALL=C ~/g/git/git -P -c i18n.commitEncoding=ascii log --author=Ævar -100|wc -l
>     28333
>
> So ~3k lines for my last 100 commits, but then:
>
>     $ LC_ALL=C ~/g/git/git -P -c i18n.commitEncoding=ascii log --author=Ævar -100 2>&1|grep -c ^warning
>     299
>
> At first I thought it was spewing warnings for every failed re-encoded
> line in some cases, because I get hundreds at a time sometimes, but it's
> because stderr and stdout I/O buffering is different (a common
> case). Adding a "fflush(stderr)" "fixes" that.

> But anyway, I think we've got a lot of users who say *do* want to
> reencode something from say UTF-8 to latin1, but then might have the
> occasional non-latin1 representable data. The old behavior of silently
> falling back is going to be much better for those users, or maybe show
> one warning at the end or something, if you feel it really needs to be
> kept.

> I ran into this while testing the grep coloring patch[1] (but it's
> unrelated). Before this commit e.g.:
> 
>     LC_ALL=C ~/g/git/git -P -c i18n.commitEncoding=ascii log --author=Ævar -100|wc -l
>     28333
> 
> So ~3k lines for my last 100 commits, but then:
> 
>     $ LC_ALL=C ~/g/git/git -P -c i18n.commitEncoding=ascii log --author=Ævar -100 2>&1|grep -c ^warning
>     299
> 
> At first I thought it was spewing warnings for every failed re-encoded
> line in some cases, because I get hundreds at a time sometimes, but it's
> because stderr and stdout I/O buffering is different (a common
> case). Adding a "fflush(stderr)" "fixes" that.

> 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.

> 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;

> But in this case this seems to have been because someone tried to feed
> "HTML" to it, which is not an encoding, and something iconv_open() has
> (I daresay) always and will always error on. It returns -1 and sets
> errno=EINVAL.
> 
> So having a warning or other detection in the revision loop seems
> backwards to me, surely we want something like the below instead?
> I.e. die as close to bad option parsing as possible?

> diff --git a/environment.c b/environment.c
> index 43bb1b35ffe..c26b18f8e5c 100644
> --- a/environment.c
> +++ b/environment.c
> @@ -357,8 +357,18 @@ void set_git_dir(const char *path, int make_realpath)
>  
>  const char *get_log_output_encoding(void)
>  {
> -	return git_log_output_encoding ? git_log_output_encoding
> +	const char *encoding = git_log_output_encoding ? git_log_output_encoding
>  		: get_commit_output_encoding();
> +#ifndef NO_ICONV
> +	iconv_t conv;
> +	conv = iconv_open(encoding, "UTF-8");
> +	if (conv == (iconv_t) -1 && errno == EINVAL)
> +		die_errno("the '%s' encoding is not known to iconv", encoding);
> +#else
> +	if (strcmp(encoding, "UTF-8"))
> +		die("compiled with NO_ICONV=Y, can't re-encode to '%s'", encoding);
> +#endif
> +	return encoding;
>  }

> On Sat, Oct 09, 2021 at 03:47:16PM +0200, Ævar Arnfjörð Bjarmason wrote:
>
>> But in this case this seems to have been because someone tried to feed
>> "HTML" to it, which is not an encoding, and something iconv_open() has
>> (I daresay) always and will always error on. It returns -1 and sets
>> errno=EINVAL.
>> 
>> So having a warning or other detection in the revision loop seems
>> backwards to me, surely we want something like the below instead?
>> I.e. die as close to bad option parsing as possible?
>
> Sorry for the slow response; this got thrown on my "to think about and
> look at later" pile.
>
> Yeah, I agree that if we sanity-checked the encoding up front, that
> would cover the case we saw in practice, and goes a long way towards
> catching any practical errors.
>
> But I think this patch is tricky:
>
>> diff --git a/environment.c b/environment.c
>> index 43bb1b35ffe..c26b18f8e5c 100644
>> --- a/environment.c
>> +++ b/environment.c
>> @@ -357,8 +357,18 @@ void set_git_dir(const char *path, int make_realpath)
>>  
>>  const char *get_log_output_encoding(void)
>>  {
>> -	return git_log_output_encoding ? git_log_output_encoding
>> +	const char *encoding = git_log_output_encoding ? git_log_output_encoding
>>  		: get_commit_output_encoding();
>> +#ifndef NO_ICONV
>> +	iconv_t conv;
>> +	conv = iconv_open(encoding, "UTF-8");
>> +	if (conv == (iconv_t) -1 && errno == EINVAL)
>> +		die_errno("the '%s' encoding is not known to iconv", encoding);
>> +#else
>> +	if (strcmp(encoding, "UTF-8"))
>> +		die("compiled with NO_ICONV=Y, can't re-encode to '%s'", encoding);
>> +#endif
>> +	return encoding;
>>  }
>
> So one obvious problem here is that we call this function once per
> commit, so it's a lot of extra iconv_open() calls. But obviously we
> could use a static flag to do it once per process.

> The other issue is that it is assuming UTF-8 on one end of the
> conversion. But we aren't necessarily doing such a conversion; it
> depends on the commit's on-disk encoding, and the requested output
> encoding. In particular:
>
>   - if both of those match, we do not need to call iconv at all (see the
>     same_encoding() check in repo_logmsg_reencode()). With the patch
>     above, the NO_ICONV case would start to die() when both are say
>     iso8859-1, even though it currently works.
>
>   - likewise, even if you have iconv support, it's possible that your
>     preferred encoding is not compatible with utf8. In which case
>     iconv_open() may complain, even though the actual conversion we'd
>     ask it to do would succeed.
>
> I.e., I don't think there's a way to just ask iconv "does this encoding
> name by itself make any sense". You can only ask it about to/from
> combos.

> So I think a much better version of this is to catch the _actual_
> iconv_open() call we make. And if it fails, say "woah, this combo of
> encodings isn't supported". The reason I didn't do that in the earlier
> patch is that all of this is obscured inside reencode_string_len(),
> which does both the iconv_open() and the iconv() call. We could surface
> that error information.
>
> But I'm not sure it would make sense to die() in that case. While for
> something like "git log --encoding=nonsense" every commit is going to
> fail to re-encode, it's still possible that iconv_open() failures are
> commit-specific. I.e., you could have some garbage commit in your
> history with an unsupported encoding, and you wouldn't want to die() for
> it (it's the same case you are complaining about having a warning for,
> but much worse).
>
> I suspect the best we could do along these lines is to wait until a real
> iconv_open(to, from) fails, and then as a fallback try:
>
>   iconv_open("UTF-8", from);
>   iconv_open(to, "UTF-8");
>
> to sanity-check them individually, and guess that one of them is broken
> if it can't go to/from UTF-8. But even that feels like it's making
> assumptions about both the system iconv, and the charsets people use.
>
> To be clear, I'd expect that most people just use utf-8 in the first
> place, and even if they don't that their system has some basic utf-8
> support. But we are deep into the realm of weird corner cases here, and
> the utility of this warning / error-checking doesn't seem high enough to
> merit the possible regressions we'd get by trying to make too many
> assumptions.

> > The other issue is that it is assuming UTF-8 on one end of the
> > conversion. But we aren't necessarily doing such a conversion; it
> > depends on the commit's on-disk encoding, and the requested output
> > encoding. In particular:
> >
> >   - if both of those match, we do not need to call iconv at all (see the
> >     same_encoding() check in repo_logmsg_reencode()). With the patch
> >     above, the NO_ICONV case would start to die() when both are say
> >     iso8859-1, even though it currently works.
> >
> >   - likewise, even if you have iconv support, it's possible that your
> >     preferred encoding is not compatible with utf8. In which case
> >     iconv_open() may complain, even though the actual conversion we'd
> >     ask it to do would succeed.
> >
> > I.e., I don't think there's a way to just ask iconv "does this encoding
> > name by itself make any sense". You can only ask it about to/from
> > combos.
> 
> Yes, I'm not saying it covers the general problem, but that it covers
> the specific complained-about issue of a completely nonsensical encoding
> like "HTML". We should simply error on that on command startup, whether
> or not we have any commits to visit.

> TBH I am not even sure it is worth spending a lot of brain cells on the
> "and then consider..." part. Over all these years, we've had one report,
> and it simply misunderstand what "--encoding" was for. I thought it was
> something we could fix up easily by checking a return value, but IMHO
> doing it right is quite tricky because of iconv()'s limited interface,
> and the risk of regression outweighs the potential benefit.

> Jeff King <peff@peff.net> writes:
>
>> TBH I am not even sure it is worth spending a lot of brain cells on the
>> "and then consider..." part. Over all these years, we've had one report,
>> and it simply misunderstand what "--encoding" was for. I thought it was
>> something we could fix up easily by checking a return value, but IMHO
>> doing it right is quite tricky because of iconv()'s limited interface,
>> and the risk of regression outweighs the potential benefit.
>
> I tend to agree with the above.  Let's not over-engineer things.

> > I tend to agree with the above.  Let's not over-engineer things.
> 
> ----- >8 --------- >8 --------- >8 --------- >8 --------- >8 -----
> From: Junio C Hamano <gitster@pobox.com>
> Date: Fri, 29 Oct 2021 13:48:58 -0700
> Subject: [PATCH] Revert "logmsg_reencode(): warn when iconv() fails"
> 
> This reverts commit fd680bc5 (logmsg_reencode(): warn when iconv()
> fails, 2021-08-27).  Throwing a warning for each and every commit
> that gets reencoded, without allowing a way to squelch, would make
> it unpleasant for folks who have to deal with an ancient part of the
> history in an old project that used wrong encoding in the commits.

> 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;

> On Sat, Oct 09, 2021 at 02:58:10AM +0200, Ævar Arnfjörð Bjarmason wrote:
> 
>> I ran into this while testing the grep coloring patch[1] (but it's
>> unrelated). Before this commit e.g.:
>>
>>     LC_ALL=C ~/g/git/git -P -c i18n.commitEncoding=ascii log --author=Ævar -100|wc -l
>>     28333
>>
>> So ~3k lines for my last 100 commits, but then:
>>
>>     $ LC_ALL=C ~/g/git/git -P -c i18n.commitEncoding=ascii log --author=Ævar -100 2>&1|grep -c ^warning
>>     299
>>
>> At first I thought it was spewing warnings for every failed re-encoded
>> line in some cases, because I get hundreds at a time sometimes, but it's
>> because stderr and stdout I/O buffering is different (a common
>> case). Adding a "fflush(stderr)" "fixes" that.
> 
> I don't think the buffering is the issue. By default stderr flushes on
> lines, and we flush commits after showing them. If you take away "-P"
> (or look at the combined 2>&1 output in order), you'll see that they are
> grouped.
> 
> Now one thing you might notice is that there may be multiple warnings
> between output commits. But that's because we really are re-encoding
> each of those intermediate commits to do your --author grep. And if that
> re-encoding fails, we may well be producing the wrong output, because
> the matching won't be correct (in your case, presumably the correct
> output should be _nothing_, because Æ is not an ascii character).

> Am 09.10.21 um 04:36 schrieb Jeff King:
>> On Sat, Oct 09, 2021 at 02:58:10AM +0200, Ævar Arnfjörð Bjarmason wrote:
>> 
>>> I ran into this while testing the grep coloring patch[1] (but it's
>>> unrelated). Before this commit e.g.:
>>>
>>>     LC_ALL=C ~/g/git/git -P -c i18n.commitEncoding=ascii log --author=Ævar -100|wc -l
>>>     28333
>>>
>>> So ~3k lines for my last 100 commits, but then:
>>>
>>>     $ LC_ALL=C ~/g/git/git -P -c i18n.commitEncoding=ascii log --author=Ævar -100 2>&1|grep -c ^warning
>>>     299
>>>
>>> At first I thought it was spewing warnings for every failed re-encoded
>>> line in some cases, because I get hundreds at a time sometimes, but it's
>>> because stderr and stdout I/O buffering is different (a common
>>> case). Adding a "fflush(stderr)" "fixes" that.
>> 
>> I don't think the buffering is the issue. By default stderr flushes on
>> lines, and we flush commits after showing them. If you take away "-P"
>> (or look at the combined 2>&1 output in order), you'll see that they are
>> grouped.
>> 
>> Now one thing you might notice is that there may be multiple warnings
>> between output commits. But that's because we really are re-encoding
>> each of those intermediate commits to do your --author grep. And if that
>> re-encoding fails, we may well be producing the wrong output, because
>> the matching won't be correct (in your case, presumably the correct
>> output should be _nothing_, because Æ is not an ascii character).
>
> I don't understand why i18n.commitEncoding plays a role here. Isn't it
> an instruction "when you make a commit, mark the commit message having
> this encoding". But grep does not make a commit.
>
> If this were i18n.logOuputEncoding it would make much more sense.
>
> Have I misunderstood the meaning of the two options?

> Dnia środa, 25 sierpnia 2021 02:57:47 CEST Jeff King pisze:
> > As far as what you're trying to accomplish, HTML-escaping isn't
> > something Git supports. You'll have to run the output through an
> > external escaping mechanism.
> 
> Have you looked at the format?  It is a HTML fragment with placeholders to be 
> filled by git log.  I cannot run the output through an external escaping 
> mechanism because it will kill the markup that is already there.

>
> Dnia środa, 25 sierpnia 2021 02:57:47 CEST Jeff King pisze:
> > diff --git a/pretty.c b/pretty.c
>
> Please fix the manual for git log.  It should say what encoding is recognised
> (namely if supported by iconv(1), except that POSIX character maps of
> iconv(1p) are not supported), and that an unrecognised encoding is ignored.
>
> I would also like to see the HTML encoding supported independently of iconv,
> which seems like a pretty easy thing to do.  Dream on, I guess?

> On Wed, Aug 25, 2021 at 4:29 PM Krzysztof Żelechowski
> <giecrilj@stegny.2a.pl> wrote:
>>
>> Dnia środa, 25 sierpnia 2021 02:57:47 CEST Jeff King pisze:
>> > diff --git a/pretty.c b/pretty.c
>>
>> Please fix the manual for git log.  It should say what encoding is recognised
>> (namely if supported by iconv(1), except that POSIX character maps of
>> iconv(1p) are not supported), and that an unrecognised encoding is ignored.
>>
>> I would also like to see the HTML encoding supported independently of iconv,
>> which seems like a pretty easy thing to do.  Dream on, I guess?
>
> I suspect the answer is less "Dream on" and more "Patches welcome."

> Dnia czwartek, 26 sierpnia 2021 17:37:40 CEST Junio C Hamano pisze:
>>         git log --html \
>>                 --format='<tr><td>%h</td><td>%s</td>...</tr>'
>
> I would like to be able to say:
>  { git config i18n.logOutputEscape HTML; }
>
> What do you think?

> Krzysztof Żelechowski <giecrilj@stegny.2a.pl> writes:
> 
> > Dnia czwartek, 26 sierpnia 2021 17:37:40 CEST Junio C Hamano pisze:
> >>         git log --html \
> >>                 --format='<tr><td>%h</td><td>%s</td>...</tr>'
> >
> > I would like to be able to say:
> >  { git config i18n.logOutputEscape HTML; }
> >
> > What do you think?
> 
> It depends on what it does.
> 
> If the configuration means that "git log" output (with any supported
> options, like "-p") will be given with '<' written to '^lt;' etc. so
> that it becomes safe to dump it in HTML, it fails to interest me at
> all.

> I wouldn't be opposed to an option for the pretty formatter to have
> encodings. Something like:
>
>   git log --format='%(authorname:quote=html)'
>
> I'd probably put off implementing that until we actually unify the
> for-each-ref and pretty formats, though (we do not even have
> %(authorname) at this point!). The latter already has a quoting
> mechanism for shell/perl/python/tcl (though it is not per-atom, and I
> wouldn't be opposed to a --format-quote option that quoted all pretty.c
> placeholders).