Re: [PATCH] pretty: user format ignores i18n.logOutputEncoding setting

[Junio C Hamano <gitster@pobox.com>]

Alexey Shumkin <alex.crezoff@gmail.com> writes:

> git log --graph --oneline
> and
> git log --graph --pretty=format:'%C(yellow)%h %Creset%s'
> give different outputs on Linux UTF-8 console
> when commit messages contain non-UTF-8 characters

(content) "different" is not specific enough to describe a problem (in
some cases, difference in command line arguments is the way to obtain
different results), especially if you do not state that they "should" be
the same to begin with, and preferrably under what condition they should
be the same.

(content) Do you really need all of the above options to observe the
breakage, or can you drop --graph?  If so, drop it from the proposed
commit log message, as it is distracting without merit.

(content) Do you really mean SP before %Creset, not after?

(style) Somehow I find this hard to read. Perhaps make the two "sample
commands" stand out more from the surrounding text?  E.g.

-- >8 --
The following two ought to give the same output to a terminal:

	$ git log --oneline
        $ git log --pretty=format:'%C(yellow)%h %creset%s'

However, the former pays attention to i18n.logOutputEncoding
configuration, while the latter does not when it format "%s". A log
messages written in an encoding i18n.commitEncoding that is not UTF-8
is shown corrupted with the latter even when i18n.logOutputEncoding is set
to convert to UTF-8.

Signed-off-by: ...
-- 8< --

(question) Does this change affect other commands, most notably
format-patch, and if so how?

> diff --git a/log-tree.c b/log-tree.c
> index e945701..39913d7 100644
> --- a/log-tree.c
> +++ b/log-tree.c
> @@ -488,6 +488,7 @@ void show_log(struct rev_info *opt)
>  	ctx.preserve_subject = opt->preserve_subject;
>  	ctx.reflog_info = opt->reflog_info;
>  	ctx.fmt = opt->commit_format;
> +	ctx.output_encoding = get_log_output_encoding();
>  	pretty_print_commit(&ctx, commit, &msgbuf);
>  
>  	if (opt->add_signoff)
> diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
> index 2ae9faa..24e787a 100755
> --- a/t/t4205-log-pretty-formats.sh
> +++ b/t/t4205-log-pretty-formats.sh
> @@ -11,10 +11,12 @@ test_expect_success 'set up basic repos' '
>  	>bar &&
>  	git add foo &&
>  	test_tick &&
> -	git commit -m initial &&
> +	git config i18n.commitEncoding cp1251 &&
> +	printf "initial \320\272\320\276\320\274\320\274\320\270\321\202" | iconv -t cp1251 -f utf-8 | xargs -I{} git commit -m "{}" &&
>  	git add bar &&
>  	test_tick &&
> -	git commit -m "add bar"
> +	git commit -m "add bar" &&
> +	git config --unset i18n.commitEncoding
>  '
>  
>  test_expect_success 'alias builtin format' '
> @@ -38,6 +40,20 @@ test_expect_success 'alias user-defined format' '
>  	test_cmp expected actual
>  '
>  
> +test_expect_success 'alias user-defined tformat with %s (native encoding)' '
> +	git config i18n.logOutputEncoding cp1251 &&
> +	git log --oneline >expected-s &&
> +	git log --pretty="tformat:%h %s" >actual-s &&
> +	git config --unset i18n.logOutputEncoding &&
> +	test_cmp expected-s actual-s
> +'
> +
> +test_expect_success 'alias user-defined tformat with %s (utf-8 encoding)' '
> +	git log --oneline >expected-s &&
> +	git log --pretty="tformat:%h %s" >actual-s &&
> +	test_cmp expected-s actual-s
> +'
> +
>  test_expect_success 'alias user-defined tformat' '
>  	git log --pretty="tformat:%h" >expected &&
>  	git config pretty.test-alias "tformat:%h" &&