diff.c: flush stdout before printing rename warnings
PATCH 1/1 diff.c: flush stdout before printing rename warnings
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.
--- diff.c | 1 + 1 file changed, 1 insertion(+)
diff.c +1 -0
@@ -5454,6 +5454,7 @@ N_("you may want to set your %s variable to at least "void diff_warn_rename_limit (const char *varname,int needed,int degraded_cc) { +fflush (stdout);if (degraded_cc)warning (_ (degrade_cc_to_c_warning));else if (needed)
Signature · 1 line
2.15.1.600.g899a5f85c6
Quoted 12 lines · Nguyễn Thái Ngọc Duy
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.
Quoted 6 lines
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. It also would be conceptually nicer to flush stderr as well even though it is by default not fully buffered.
- Also this function can take two bools and gives a warning that potentially cause the issue three out of four combinations, so one out of four cases we would be unnecessarily flushing.
Quoted 17 lines
Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>--- diff.c | 1 + 1 file changed, 1 insertion(+)diff.c +1
-0@@ -5454,6 +5454,7 @@ N_("you may want to set your %s variable to at least "void diff_warn_rename_limit (const char *varname,int needed,int degraded_cc) { +fflush (stdout);if (degraded_cc)warning (_ (degrade_cc_to_c_warning));else if (needed)
Quoted 21 lines · Junio C Hamano
Quoted 12 lines · Nguyễn Thái Ngọc Duy
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.
Quoted 6 lines
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.
Quoted 6 lines
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)
Quoted 3 lines
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.
Signature · 1 line
Duy