Re: Should the --encoding argument to log/show commands make any guarantees about their output?

On 2015-06-16 11.38, Jan-Philip Gehrcke wrote:

On 15.06.2015 18:21, Torsten Bögershausen wrote:

quoted

On 2015-06-15 10.50, Jan-Philip Gehrcke wrote:

quoted

Let me describe what I think it currently does:

The program attempts to re-code a log message, so it follows the chain

     raw input -> unicode -> raw output

Not sure what "raw input/output" means.
But there is only one reencode step involved, e.g.
input(8859) -> output(UTF-8)

I probably need to correct myself:
pretty.c
void format_commit_message(const struct commit *commit,

The the message is converted from the commit encoding into UTF-8.
When the log encoding is different from UTF-8,
There is a second conversio (as you said).
(But not in your case, here the second conversion is skipped)

[snip]

I'd be willing to contribute, but of course there must be a discussion and an
agreement before that, if there is need to change something at all, and what
exactly.

[]

What do you think?

See commit.c:

/*
 * This verifies that the buffer is in proper utf8 format.
 *
 * If it isn't, it assumes any non-utf8 characters are Latin1,
 * and does the conversion.
 */
static int verify_utf8(struct strbuf *buf)

I think the --encoding option would have ideal semantics for described behavior.

However, I guess maintaining backwards compatibility is an issue here. On the
other hand, I realize that the --encoding option undergoes changes: the docs for
git log in release 2.4.3 do not even list the --encoding option anymore. Why is
that? I haven't found a corresponding changelog/release notes entry.

It still seems to work:
git log --encoding ISO-8859-1 | xxd | grep gersha | less
includes this line:
7465 6e20 42f6 6765 7273 6861 7573 656e  ten B.gershausen


So my suggestion (in short):
move verify_utf8() from commit.c into utf8.c,
make it non-static, and add a prototype in utf8.h

Use that function in pretty.c (and commit.c), test it.
Make a patch out of it and send it to the list.

In the ideal world the patch will include a test case,
but I don't know how easy it is to create such a commit.