Re: documentation structure

Andres Freund <andres@anarazel.de> writes:

> Hi,
>
> On 2024-04-17 12:07:24 +0100, Dagfinn Ilmari Mannsåker wrote:
>> Andres Freund <andres@anarazel.de> writes:
>> > I think the manual work for writing signatures in sgml is not insignificant,
>> > nor is the volume of sgml for them. Manually maintaining the signatures makes
>> > it impractical to significantly improve the presentation - which I don't think
>> > is all that great today.
>> 
>> And it's very inconsistent.  For example, some functions use <optional>
>> tags for optional parameters, others use square brackets, and some use
>> <literal>VARIADIC</literal> to indicate variadic parameters, others use
>> ellipses (sometimes in <optional> tags or brackets).
>
> That seems almost inevitably the outcome of many people having to manually
> infer the recommended semantics, for writing something boring but nontrivial,
> from a 30k line file.

As Corey mentioned elsethread, having a markup style guide (maybe a
comment at the top of the file?) would be nice.

>> > And the lack of argument names in the pg_proc entries is occasionally fairly
>> > annoying, because a \df+ doesn't provide enough information to use functions.
>> 
>> I was also annoyed by this the other day (specifically wrt. the boolean
>> arguments to pg_ls_dir),
>
> My bane is regexp_match et al, I have given up on remembering the argument
> order.

There's a thread elsewhere about those specifically, but I can't be
bothered to find the link right now.

>> and started whipping up a Perl script to parse func.sgml and generate
>> missing proargnames values for pg_proc.dat, which is how I discovered the
>> above.
>
> Nice.
>
>> The script currently has a pile of hacky regexes to cope with that,
>> so I'd be happy to submit a doc patch to turn it into actual markup to get
>> rid of that, if people think that's a worhtwhile use of time and won't clash
>> with any other plans for the documentation.
>
> I guess it's a bit hard to say without knowing how voluminious the changes
> would be. If we end up rewriting the whole file the tradeoff is less clear
> than if it's a dozen inconsistent entries.

It turned out to not be that many that used [] for optional parameters,
see the attached patch. 

I havent dealt with variadic yet, since the two styles are visually
different, not just markup (<optional>...</optional> renders as [...]).

The two styles for variadic are the what I call caller-style:

   concat ( val1 "any" [, val2 "any" [, ...] ] )
   format(formatstr text [, formatarg "any" [, ...] ])

which shows more clearly how you'd call it, versus definition-style:

   num_nonnulls ( VARIADIC "any" )
   jsonb_extract_path ( from_json jsonb, VARIADIC path_elems text[] )

which matches the CREATE FUNCTION statement.  I don't have a strong
opinion on which we should use, but we should be consistent.

> Greetings,
>
> Andres Freund

- ilmari

From f71e0669eb25b205bd5065f15657ba6d749261f3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org>
Date: Wed, 17 Apr 2024 16:00:52 +0100
Subject: [PATCH] func.sgml: Consistently use <optional> to indicate optional
 parameters

Some functions were using square brackets instead.
---
 doc/src/sgml/func.sgml | 54 +++++++++++++++++++++---------------------
 1 file changed, 27 insertions(+), 27 deletions(-)

diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 8dfb42ad4d..afaaf61d69 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -3036,7 +3036,7 @@
          <primary>concat</primary>
         </indexterm>
         <function>concat</function> ( <parameter>val1</parameter> <type>"any"</type>
-         [, <parameter>val2</parameter> <type>"any"</type> [, ...] ] )
+         <optional>, <parameter>val2</parameter> <type>"any"</type> [, ...] </optional> )
         <returnvalue>text</returnvalue>
        </para>
        <para>
@@ -3056,7 +3056,7 @@
         </indexterm>
         <function>concat_ws</function> ( <parameter>sep</parameter> <type>text</type>,
         <parameter>val1</parameter> <type>"any"</type>
-        [, <parameter>val2</parameter> <type>"any"</type> [, ...] ] )
+        <optional>, <parameter>val2</parameter> <type>"any"</type> [, ...] </optional> )
         <returnvalue>text</returnvalue>
        </para>
        <para>
@@ -3076,7 +3076,7 @@
          <primary>format</primary>
         </indexterm>
         <function>format</function> ( <parameter>formatstr</parameter> <type>text</type>
-        [, <parameter>formatarg</parameter> <type>"any"</type> [, ...] ] )
+        <optional>, <parameter>formatarg</parameter> <type>"any"</type> [, ...] </optional> )
         <returnvalue>text</returnvalue>
        </para>
        <para>
@@ -3170,7 +3170,7 @@
          <primary>parse_ident</primary>
         </indexterm>
         <function>parse_ident</function> ( <parameter>qualified_identifier</parameter> <type>text</type>
-        [, <parameter>strict_mode</parameter> <type>boolean</type> <literal>DEFAULT</literal> <literal>true</literal>
])
 
+        <optional>, <parameter>strict_mode</parameter> <type>boolean</type> <literal>DEFAULT</literal>
<literal>true</literal></optional> )
 
         <returnvalue>text[]</returnvalue>
        </para>
        <para>
@@ -3309,8 +3309,8 @@
          <primary>regexp_count</primary>
         </indexterm>
         <function>regexp_count</function> ( <parameter>string</parameter> <type>text</type>,
<parameter>pattern</parameter><type>text</type>
 
-         [, <parameter>start</parameter> <type>integer</type>
-         [, <parameter>flags</parameter> <type>text</type> ] ] )
+         <optional>, <parameter>start</parameter> <type>integer</type>
+         <optional>, <parameter>flags</parameter> <type>text</type> </optional> </optional> )
         <returnvalue>integer</returnvalue>
        </para>
        <para>
@@ -3331,11 +3331,11 @@
          <primary>regexp_instr</primary>
         </indexterm>
         <function>regexp_instr</function> ( <parameter>string</parameter> <type>text</type>,
<parameter>pattern</parameter><type>text</type>
 
-         [, <parameter>start</parameter> <type>integer</type>
-         [, <parameter>N</parameter> <type>integer</type>
-         [, <parameter>endoption</parameter> <type>integer</type>
-         [, <parameter>flags</parameter> <type>text</type>
-         [, <parameter>subexpr</parameter> <type>integer</type> ] ] ] ] ] )
+         <optional>, <parameter>start</parameter> <type>integer</type>
+         <optional>, <parameter>N</parameter> <type>integer</type>
+         <optional>, <parameter>endoption</parameter> <type>integer</type>
+         <optional>, <parameter>flags</parameter> <type>text</type>
+         <optional>, <parameter>subexpr</parameter> <type>integer</type> </optional> </optional> </optional>
</optional></optional> )
 
         <returnvalue>integer</returnvalue>
        </para>
        <para>
@@ -3360,7 +3360,7 @@
          <primary>regexp_like</primary>
         </indexterm>
         <function>regexp_like</function> ( <parameter>string</parameter> <type>text</type>,
<parameter>pattern</parameter><type>text</type>
 
-         [, <parameter>flags</parameter> <type>text</type> ] )
+         <optional>, <parameter>flags</parameter> <type>text</type> </optional> )
         <returnvalue>boolean</returnvalue>
        </para>
        <para>
@@ -3380,7 +3380,7 @@
         <indexterm>
          <primary>regexp_match</primary>
         </indexterm>
-        <function>regexp_match</function> ( <parameter>string</parameter> <type>text</type>,
<parameter>pattern</parameter><type>text</type> [, <parameter>flags</parameter> <type>text</type> ] )
 
+        <function>regexp_match</function> ( <parameter>string</parameter> <type>text</type>,
<parameter>pattern</parameter><type>text</type> <optional>, <parameter>flags</parameter> <type>text</type> </optional>
)
         <returnvalue>text[]</returnvalue>
        </para>
        <para>
@@ -3400,7 +3400,7 @@
         <indexterm>
          <primary>regexp_matches</primary>
         </indexterm>
-        <function>regexp_matches</function> ( <parameter>string</parameter> <type>text</type>,
<parameter>pattern</parameter><type>text</type> [, <parameter>flags</parameter> <type>text</type> ] )
 
+        <function>regexp_matches</function> ( <parameter>string</parameter> <type>text</type>,
<parameter>pattern</parameter><type>text</type> <optional>, <parameter>flags</parameter> <type>text</type> </optional>
)
         <returnvalue>setof text[]</returnvalue>
        </para>
        <para>
@@ -3426,8 +3426,8 @@
          <primary>regexp_replace</primary>
         </indexterm>
         <function>regexp_replace</function> ( <parameter>string</parameter> <type>text</type>,
<parameter>pattern</parameter><type>text</type>, <parameter>replacement</parameter> <type>text</type>
 
-         [, <parameter>start</parameter> <type>integer</type> ]
-         [, <parameter>flags</parameter> <type>text</type> ] )
+         <optional>, <parameter>start</parameter> <type>integer</type> </optional>
+         <optional>, <parameter>flags</parameter> <type>text</type> </optional> )
         <returnvalue>text</returnvalue>
        </para>
        <para>
@@ -3447,7 +3447,7 @@
         <function>regexp_replace</function> ( <parameter>string</parameter> <type>text</type>,
<parameter>pattern</parameter><type>text</type>, <parameter>replacement</parameter> <type>text</type>,
 
          <parameter>start</parameter> <type>integer</type>,
          <parameter>N</parameter> <type>integer</type>
-         [, <parameter>flags</parameter> <type>text</type> ] )
+         <optional>, <parameter>flags</parameter> <type>text</type> </optional> )
         <returnvalue>text</returnvalue>
        </para>
        <para>
@@ -3467,7 +3467,7 @@
         <indexterm>
          <primary>regexp_split_to_array</primary>
         </indexterm>
-        <function>regexp_split_to_array</function> ( <parameter>string</parameter> <type>text</type>,
<parameter>pattern</parameter><type>text</type> [, <parameter>flags</parameter> <type>text</type> ] )
 
+        <function>regexp_split_to_array</function> ( <parameter>string</parameter> <type>text</type>,
<parameter>pattern</parameter><type>text</type> <optional>, <parameter>flags</parameter> <type>text</type> </optional>
)
         <returnvalue>text[]</returnvalue>
        </para>
        <para>
@@ -3486,7 +3486,7 @@
         <indexterm>
          <primary>regexp_split_to_table</primary>
         </indexterm>
-        <function>regexp_split_to_table</function> ( <parameter>string</parameter> <type>text</type>,
<parameter>pattern</parameter><type>text</type> [, <parameter>flags</parameter> <type>text</type> ] )
 
+        <function>regexp_split_to_table</function> ( <parameter>string</parameter> <type>text</type>,
<parameter>pattern</parameter><type>text</type> <optional>, <parameter>flags</parameter> <type>text</type> </optional>
)
         <returnvalue>setof text</returnvalue>
        </para>
        <para>
@@ -3510,10 +3510,10 @@
          <primary>regexp_substr</primary>
         </indexterm>
         <function>regexp_substr</function> ( <parameter>string</parameter> <type>text</type>,
<parameter>pattern</parameter><type>text</type>
 
-         [, <parameter>start</parameter> <type>integer</type>
-         [, <parameter>N</parameter> <type>integer</type>
-         [, <parameter>flags</parameter> <type>text</type>
-         [, <parameter>subexpr</parameter> <type>integer</type> ] ] ] ] )
+         <optional>, <parameter>start</parameter> <type>integer</type>
+         <optional>, <parameter>N</parameter> <type>integer</type>
+         <optional>, <parameter>flags</parameter> <type>text</type>
+         <optional>, <parameter>subexpr</parameter> <type>integer</type> </optional> </optional> </optional>
</optional>)
 
         <returnvalue>text</returnvalue>
        </para>
        <para>
@@ -3980,7 +3980,7 @@
 
     <para>
 <synopsis>
-<function>format</function>(<parameter>formatstr</parameter> <type>text</type> [, <parameter>formatarg</parameter>
<type>"any"</type>[, ...] ])
 
+<function>format</function>(<parameter>formatstr</parameter> <type>text</type> <optional>,
<parameter>formatarg</parameter><type>"any"</type> [, ...] </optional>)
 
 </synopsis>
      <parameter>formatstr</parameter> is a format string that specifies how the
      result should be formatted.  Text in the format string is copied
@@ -10568,7 +10568,7 @@
 
    <para>
 <synopsis>
-date_trunc(<replaceable>field</replaceable>, <replaceable>source</replaceable> [, <replaceable>time_zone</replaceable>
])
+date_trunc(<replaceable>field</replaceable>, <replaceable>source</replaceable> <optional>,
<replaceable>time_zone</replaceable></optional>)
 
 </synopsis>
     <replaceable>source</replaceable> is a value expression of type
     <type>timestamp</type>, <type>timestamp with time zone</type>,
@@ -29308,11 +29308,11 @@
         <indexterm>
          <primary>pg_logical_emit_message</primary>
         </indexterm>
-        <function>pg_logical_emit_message</function> ( <parameter>transactional</parameter> <type>boolean</type>,
<parameter>prefix</parameter><type>text</type>, <parameter>content</parameter> <type>text</type> [,
<parameter>flush</parameter><type>boolean</type> <literal>DEFAULT</literal> <literal>false</literal>] )
 
+        <function>pg_logical_emit_message</function> ( <parameter>transactional</parameter> <type>boolean</type>,
<parameter>prefix</parameter><type>text</type>, <parameter>content</parameter> <type>text</type> <optional>,
<parameter>flush</parameter><type>boolean</type> <literal>DEFAULT</literal> <literal>false</literal></optional> )
 
         <returnvalue>pg_lsn</returnvalue>
        </para>
        <para role="func_signature">
-        <function>pg_logical_emit_message</function> ( <parameter>transactional</parameter> <type>boolean</type>,
<parameter>prefix</parameter><type>text</type>, <parameter>content</parameter> <type>bytea</type> [,
<parameter>flush</parameter><type>boolean</type> <literal>DEFAULT</literal> <literal>false</literal>] )
 
+        <function>pg_logical_emit_message</function> ( <parameter>transactional</parameter> <type>boolean</type>,
<parameter>prefix</parameter><type>text</type>, <parameter>content</parameter> <type>bytea</type> <optional>,
<parameter>flush</parameter><type>boolean</type> <literal>DEFAULT</literal> <literal>false</literal></optional> )
 
         <returnvalue>pg_lsn</returnvalue>
        </para>
        <para>
-- 
2.39.2