public inbox for [email protected]  

help / color / mirror / Atom feed

From: Chao Li <[email protected]>
To: Álvaro Herrera <[email protected]>
Cc: Pg Hackers <[email protected]>
Subject: Re: splitting pg_resetwal output strings
Date: Mon, 2 Feb 2026 11:58:10 +0800
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>



> On Jan 31, 2026, at 23:08, Álvaro Herrera <[email protected]> wrote:
> 
> On 2026-Jan-31, Álvaro Herrera wrote:
> 
>> This is not complete.  It modifies PrintControlValues(), which is easy
>> because I just print each item in the entries.h file in the same order
>> they appear there.   But for PrintNewControlValues() I'll need to add a
>> symbolic identifier to each string, that the code can use to scan the
>> array and print just those elements.  I'll do that if there are no
>> objections to this idea here.
> 
> It looks more or less like this.  Patch 0001 is the same as before, and
> 0002 adds the symbolic names, which is used to create an enum and then
> to search for the correct lines to print in PrintNewControlValues.
> 
> I decided to reuse simple_oid_list to store the integers values, which
> is kinda icky, so getting to something committable I think would have me
> add simple_int_list.  Also, there's a few blocks that are duplicate
> cases of the same line measuring and printing logic; I suppose I should
> have a routine to simplify.
> 

Hi Alvaro,

I don't think we necessarily need to add a simple_int_list. Since OIDs are not globally unique across all object types, we can reasonably treat ControlData items as "objects" and assign them OIDs within this context.

If we take that approach, the enum could be named ControlDataOid:
```
/* Define the string enums */
#define CONTROLDATA_LINE(symbol, description, fmt, ...) \
symbol,
enum {
#include "entries.h”
} ControlDataOid;
#undef CONTROLDATA_LINE
```

This makes reusing simple_oid_list feel appropriate. If you aren't fond of that idea, I’d suggest naming the enum ControlDataSymbol rather than ControlDataStrings, as the former better reflects what the members actually are.

Regarding patch 0002:
```
+#define CONTROLDATA_LINE(symbol, description, fmt, ...)			\
+	if (simple_oid_list_member(&toprint, symbol))				\
+	{															\
+	thislen = internal_wcswidth(_(description),					\
+								strlen(_(description)),			\
+								encoding);						\
+	if (thislen > maxlen)										\
+		maxlen = thislen;										\
 	}
+#include "entries.h"
+#undef CONTROLDATA_LINE
```
The code block inside the first if statement is missing an indentation level.

Otherwise, the patch looks good to me. I've tested it and confirmed that calculating the padding at runtime generates better-looking output.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: splitting pg_resetwal output strings
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox