> 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 via email to