Hi Fabien, On Fri, Nov 29, 2019 at 10:13 PM Fabien COELHO <coe...@cri.ensmp.fr> wrote: > > I wonder why we don't use the same style for $subject as pg_basebackup > > --progress, that is, use a carriage return instead of a newline after > > each line reporting the number of tuples copied? > > Patch applies cleanly, compiles, and works for me.
Thanks a lot for the quick review. > My 0.02€: > > fprintf -> fputs or fputc to avoid a format parsing, or maybe use %c in > the formats. > > As the format is not constant, ISTM that vfprintf should be called, not > fprintf (even if in practice fprintf does call vfprintf internally). > > I'm not sure what the compilers does with isatty(fileno(stderr)), maybe > the eol could be precomputed: > > char eol = isatty(...) ? '\r' : '\n'; > > and reused afterwards in the loop: > > fprintf(stderr, ".... %c", ..., eol); > > that would remove the added in-loop printing. I have updated the patch based on these observations. Attached v2. Thanks, Amit
compactify-pgbench-init-progress-output_v2.patch
Description: Binary data