Re: [PATCH] diff.c: flush stdout before printing rename warnings

[Duy Nguyen <pclouds@gmail.com>]

On Wed, Jan 17, 2018 at 2:06 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
>> The diff output is buffered in a FILE object and could still be
>> partially buffered when we print these warnings (directly to fd 2).
>> The output is messed up like this
>>
>>  worktree.c                                   |   138 +-
>>  worktree.h        warning: inexact rename detection was skipped due to too many files.
>>                            |    12 +-
>>  wrapper.c                                    |    83 +-
>>
>> It gets worse if the warning is printed after color codes for the graph
>> part are already printed. You'll get a warning in green or red.
>>
>> Flush stdout first, so we can get something like this instead:
>>
>>  xdiff/xutils.c                               |    42 +-
>>  xdiff/xutils.h                               |     4 +-
>>  1033 files changed, 150824 insertions(+), 69395 deletions(-)
>> warning: inexact rename detection was skipped due to too many files.
>
> The patch sort-of makes sense, and I am not sure if any of the
> issues that show rooms for potential improvements I'll mention are
> worth doing.
>
>  - This matters only when the standard output and the starndard error
>    are going to the same place.

Which is usually true when pager is involved because we redirect
stderr to the pager too 61b80509e3 (sending errors to stdout under
$PAGER - 2008-02-16)

>    It also would be conceptually nicer to
>    flush stderr as well even though it is by default not fully
>    buffered.

There's more than that. I briefly considered if the same thing could
happen elsewhere, this is not the only place we use buffered stdio.
But since it's just a minor annoyance, I stuck to minimal fixes and
the "fix as we see" approach instead of scanning the whole code base
and preventing similar cases.
-- 
Duy