Date: Fri, 08 Nov 2024 11:47:13 +0900 (JST)
Message-Id: <20241108.114713.484828673471024549.ishii@postgresql.org>
To: masao.fujii@oss.nttdata.com
Cc: btogiwarayuushi@oss.nttdata.com, pgsql-hackers@postgresql.org
Subject: Re: Fix for Extra Parenthesis in pgbench progress message
From: Tatsuo Ishii <ishii@postgresql.org>
In-Reply-To: <85fe426d-248a-47f7-8784-22af811ee572@oss.nttdata.com>
References: <0f7ac0fa-f26b-49dc-b9a0-d35c29ace3a5@oss.nttdata.com>
	<20241107.104143.1498084100859794666.ishii@postgresql.org>
	<85fe426d-248a-47f7-8784-22af811ee572@oss.nttdata.com>
Mime-Version: 1.0
Content-Type: Text/Plain; charset=iso-2022-jp
Content-Transfer-Encoding: 7bit
Archived-At: <https://www.postgresql.org/message-id/20241108.114713.484828673471024549.ishii%40postgresql.org>
Precedence: bulk

>> Maybe. But I am also worried about the case when we would want to
>> change the log line format in the future. We might introduce this kind
>> of bug again.  By dynamically calculating the number of necessary
>> spaces, we don't need to think about the concern.
> 
> +1
> 
> +			if (previous_chars != 0)
> +			{
> + int n_spaces = chars - previous_chars;
> +				fprintf(stderr, "%*s", n_spaces, "");
> +				chars += n_spaces;
> +			}
> 
> Currently, this is added only when use_quiet is false, but shouldn’t
> it also apply when use_quiet is true?

Yes. The patch just showed my idea and I thought I needed to apply the
case when use_quiet is true.  But the fix should be applied to after
"else if (use_quiet && (j % 100 == 0))" as shown in your patch.

> Also, what happens if chars is smaller than previous_chars?

My oversight. Better to check if chars is smaller than previous_chars.

> I’m unsure why chars needs to be incremented by n_spaces.

Yeah, it's not necessary.

> I’ve created an updated patch based on your idea―could you take a
> look?

Sure.

I think you need to adjust

		fprintf(stderr, "%*c\r", chars - 1, ' ');	/* Clear the current line */

to:

		fprintf(stderr, "%*c\r", chars, ' ');	/* Clear the current line */

since now chars does not consider the EOL char. By clearing chars - 1,
the closing parenthesis will be left on the screen.

Best reagards,
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp