Re: pg_basebackup --progress output for batch execution

Hello,

On Sun, Oct 01, 2017 at 04:49:17PM -0300, Martin Marques wrote:
> Updated patch with documentation of the new option.
>

I have checked the patch.
The patch is applied and compiled correctly without any errors. Tests passed.
The documentation doesn't have errors too.

I have a little suggestion. Maybe insert new line without any additional parameters? We can check that stderr is not terminal using isatty().

The code could become:

if (!isatty(fileno(stderr)))
fprintf(stderr, "\n");
else
fprintf(stderr, "\r");

Also it could be good to not insert new line after progress:

if (showprogress)
{
progress_report(PQntuples(res), NULL, true);
/* if (!batchmode) */
/* or */
if (isatty(fileno(stderr)))
fprintf(stderr, "\n"); /* Need to move to next line */
}

Otherwise we have an extra empty line:

pg_basebackup: initiating base backup, waiting for checkpoint to complete
pg_basebackup: checkpoint completed
pg_basebackup: write-ahead log start point: 0/1C000028 on timeline 1
pg_basebackup: starting background WAL receiver
pg_basebackup: created temporary replication slot "pg_basebackup_19635"
0/183345 kB (0%), 0/1 tablespace (data_repl/backup_label )
183355/183355 kB (100%), 0/1 tablespace (data_repl/global/pg_control )
183355/183355 kB (100%), 1/1 tablespace

pg_basebackup: write-ahead log end point: 0/1C0000F8
pg_basebackup: waiting for background process to finish streaming ...
pg_basebackup: base backup completed

--
Arthur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company