Re: describe special values in GUC descriptions more consistently

Thank you for the reviews.

On Mon, Feb 10, 2025 at 05:25:42PM -0700, David G. Johnston wrote:
> On Mon, Feb 10, 2025 at 4:53 PM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
>> {"shared_memory_size_in_huge_pages", PGC_INTERNAL, PRESET_OPTIONS,
>> gettext_noop("Shows the number of huge pages needed for the main
>> shared memory area."),
>> - gettext_noop("-1 indicates that the value could not be determined."),
>> + gettext_noop("-1 means the value could not be determined."),
>>
>> I didn't know what that meant. And the docs seem worded differently. More
>> like:
>> "-1 means huge pages are not supported."
>>
>
> Agreed

Fixed.

>> - gettext_noop("An empty string indicates that \"archive_command\"
>> should be used.")
>> + gettext_noop("An empty string means \"archive_command\" should be used.")
>>
>> Should that be worded more like other ones that delegate to other GUCs:
>>
>> "An empty string means use \"archive_command\"."
>>
>
> Agreed

Fixed.

>> What is the difference between "system setting" and "system default",
>> or should those be made the same?
>>
>>
> Looking at the documentation it seems to be communicating the difference
> between a PostgreSQL default (system default) and a value taken from the
> operating environment (system setting). Maybe making that more clear by
> saying "operating system setting" is warranted.

I've used "operating system setting" in v3.

> Minor typo, trailing whitespace in message:
>
> "lost before a connection is considered dead. "

This is intentional so that there is a space between the sentences in the
long description.

> And then these two don't seem worded symmetrically enough:
>
> "An empty string means don't load CRL file unless \"ssl_crl_dir\" is set."
> "An empty string means don't use CRLs unless \"ssl_crl_file\" is set."
>
> The first one also needs to be "the CRL file".
>
> But I'm thinking something more like:
>
> "An empty string disables the directory-based CRL auto-load mechanism."
> (ssl_crl_dir)
> "An empty string disables the fixed-file CRL reload mechanism."
> (ssl_crl_file)
>
> I don't see much benefit trying to shoe-horn a cross-reference to the other
> setting in these brief usage messages. Though if we wanted to a simple
> (see also ssr_crl_<file|dir>) would suffice. It seems unworthy of the
> limited space to try and communicate the fact that if both are empty
> strings no CRL files will be loaded (or that both can be used at the same
> time). Even trying to fit in the "auto-load versus reload" dynamic of
> these two choices seems awkward.

I thought about this one a bit, and I actually came to the opposite
conclusion. IMHO it's reasonably obvious that an empty string means that
the file isn't loaded, so there's not much point in stating it in the GUC
description. Instead, I think we should follow the
archive_command/archive_library example and use this space _only_ as a
cross-reference to each other. There's certainly some nuances missed with
this strategy, but that's not unique to this GUC.

--
nathan