[Justin Pryzby <pryzby@telsasoft.com>]

On Sat, Jun 20, 2020 at 01:55:33PM +0300, Alexander Korotkov wrote:
> On Thu, Jun 18, 2020 at 8:06 PM Alexander Korotkov <a.korotkov@postgrespro.ru> wrote:
> > On Wed, Jun 17, 2020 at 2:00 PM Alexander Korotkov <a.korotkov@postgrespro.ru> wrote:
> > > On Wed, Jun 17, 2020 at 2:50 AM Peter Geoghegan <pg@bowt.ie> wrote:
> > > > On Tue, Jun 16, 2020 at 4:24 AM Alexander Korotkov <a.korotkov@postgrespro.ru> wrote:
> > > > > Thank you for patience.  The documentation patch is attached.  I think
> > > > > it requires review by native english speaker.
...
> > > Fixed, thanks!
> > >
> > > > It's very hard to write documentation like this, even for native
> > > > English speakers. I think that it's important to have something in
> > > > place, though. The GiST example helps a lot.
...
> > I'm going to push this patch if there are no objections.  I'm almost
> > sure that documentation of opclass options will require further
> > adjustments.  However, I think the current patch makes it better, not
> > worse.
> 
> So, pushed!

Find attached some language review of user-facing docs.

inline patch 1: doc/src/sgml/brin.sgml, doc/src/sgml/btree.sgml (+0/-0 lines) download inline-1.patchdiff --git a/doc/src/sgml/brin.sgml b/doc/src/sgml/brin.sgml
index d7f1af7819..4c5eeb875f 100644
--- a/doc/src/sgml/brin.sgml
+++ b/doc/src/sgml/brin.sgml
@@ -562,7 +562,7 @@ typedef struct BrinOpcInfo
   </varlistentry>
  </variablelist>

  [-Optionally, an-]{+An+} operator class for <acronym>BRIN</acronym> can [-supply-]{+optionally specify+} the
  following method:

  <variablelist>
@@ -570,22 +570,22 @@ typedef struct BrinOpcInfo
     <term><function>void options(local_relopts *relopts)</function></term>
     <listitem>
      <para>
       Defines {+a+} set of user-visible parameters that control operator class
       behavior.
      </para>

      <para>
       The <function>options</function> function [-has given-]{+is passed a+} pointer to {+a+}
       <replaceable>local_relopts</replaceable> struct, which needs to be
       filled with a set of operator class specific options.  The options
       can be accessed from other support functions using {+the+}
       <literal>PG_HAS_OPCLASS_OPTIONS()</literal> and
       <literal>PG_GET_OPCLASS_OPTIONS()</literal> macros.
      </para>

      <para>
       Since both key extraction [-for-]{+of+} indexed [-value-]{+values+} and representation of the
       key in <acronym>GIN</acronym> are flexible, [-it-]{+they+} may [-depends-]{+depend+} on
       user-specified parameters.
      </para>
     </listitem>
diff --git a/doc/src/sgml/btree.sgml b/doc/src/sgml/btree.sgml
index 2c4dd48ea3..b17b166e84 100644
--- a/doc/src/sgml/btree.sgml
+++ b/doc/src/sgml/btree.sgml
@@ -557,7 +557,7 @@ equalimage(<replaceable>opcintype</replaceable> <type>oid</type>) returns bool
     Optionally, a B-tree operator family may provide
     <function>options</function> (<quote>operator class specific
     options</quote>) support functions, registered under support
     function number 5.  These functions define {+a+} set of user-visible
     parameters that control operator class behavior.
    </para>
    <para>
@@ -566,19 +566,19 @@ equalimage(<replaceable>opcintype</replaceable> <type>oid</type>) returns bool
<synopsis>
options(<replaceable>relopts</replaceable> <type>local_relopts *</type>) returns void
</synopsis>
     The function [-has given-]{+is passed a+} pointer to {+a+} <replaceable>local_relopts</replaceable>
     struct, which needs to be filled with a set of operator class
     specific options.  The options can be accessed from other support
     functions using {+the+} <literal>PG_HAS_OPCLASS_OPTIONS()</literal> and
     <literal>PG_GET_OPCLASS_OPTIONS()</literal> macros.
    </para>
    <para>
     Currently, no B-Tree operator class has {+an+} <function>options</function>
     support function.  B-tree doesn't allow flexible representation of keys
     like GiST, SP-GiST, GIN and BRIN do.  So, <function>options</function>
     probably doesn't have much [-usage-]{+application+} in {+the+} current[-shape of-] B-tree index
     access method.  Nevertheless, this support function was added to B-tree
     for uniformity, and[-probably it-] will [-found its usage-]{+probably find uses+} during further
     evolution of B-tree in <productname>PostgreSQL</productname>.
    </para>
   </listitem>
inline patch 2: doc/src/sgml/gin.sgml, doc/src/sgml/gist.sgml (+0/-0 lines) download inline-2.patchdiff --git a/doc/src/sgml/gin.sgml b/doc/src/sgml/gin.sgml
index d85e7c8796..7a8c18a449 100644
--- a/doc/src/sgml/gin.sgml
+++ b/doc/src/sgml/gin.sgml
@@ -411,17 +411,17 @@
      </para>

      <para>
       The <function>options</function> function [-has given-]{+is passed a+} pointer to {+a+}
       <replaceable>local_relopts</replaceable> struct, which needs to be
       filled with [-s-]{+a+} set of operator class specific options.  The options
       can be accessed from other support functions using {+the+}
       <literal>PG_HAS_OPCLASS_OPTIONS()</literal> and
       <literal>PG_GET_OPCLASS_OPTIONS()</literal> macros.
      </para>

      <para>
       Since both key extraction [-for-]{+of+} indexed [-value-]{+values+} and representation of the
       key in <acronym>GIN</acronym> are flexible, [-it-]{+they+} may [-depends-]{+depend+} on
       user-specified parameters.
      </para>
     </listitem>
diff --git a/doc/src/sgml/gist.sgml b/doc/src/sgml/gist.sgml
index 31c28fdb61..5d970ee9f2 100644
--- a/doc/src/sgml/gist.sgml
+++ b/doc/src/sgml/gist.sgml
@@ -946,7 +946,7 @@ my_fetch(PG_FUNCTION_ARGS)
     <term><function>options</function></term>
     <listitem>
      <para>
       Allows [-defintion-]{+definition+} of user-visible parameters that control operator
       class behavior.
      </para>

@@ -962,16 +962,16 @@ LANGUAGE C STRICT;
      </para>

      <para>
       The function [-has given-]{+is passed a+} pointer to {+a+} <replaceable>local_relopts</replaceable>
       struct, which needs to be filled with a set of operator class
       specific options.  The options can be accessed from other support
       functions using {+the+} <literal>PG_HAS_OPCLASS_OPTIONS()</literal> and
       <literal>PG_GET_OPCLASS_OPTIONS()</literal> macros.
      </para>

       <para>
        [-The sample-]{+An example+} implementation of [-my_option()-]{+my_options()+} and parameters [-usage-]
[-        in the another-]{+use+}
{+        from other+} support [-function-]{+functions+} are given below:

<programlisting>
typedef enum MyEnumType
@@ -990,7 +990,7 @@ typedef struct
    int     str_param;  /* string parameter */
} MyOptionsStruct;

/* String [-representations for-]{+representation of+} enum values */
static relopt_enum_elt_def myEnumValues[] =
{
    {"on", MY_ENUM_ON},
@@ -1002,7 +1002,7 @@ static relopt_enum_elt_def myEnumValues[] =
static char *str_param_default = "default";

/*
 * Sample [-validatior:-]{+validator:+} checks that string is not longer than 8 bytes.
 */
static void 
validate_my_string_relopt(const char *value)
@@ -1090,8 +1090,8 @@ my_compress(PG_FUNCTION_ARGS)

      <para>
       Since the representation of the key in <acronym>GiST</acronym> is
       flexible, it may [-depends-]{+depend+} on user-specified parameters.  For [-instace,-]{+instance,+}
       the length of key signature may be [-such parameter.-]{+specified.+}  See
       <literal>gtsvector_options()</literal> for example.
      </para>
     </listitem>
inline patch 3: doc/src/sgml/spgist.sgml, doc/src/sgml/xindex.sgml (+0/-0 lines) download inline-3.patchdiff --git a/doc/src/sgml/spgist.sgml b/doc/src/sgml/spgist.sgml
index 03f914735b..1395dbaf88 100644
--- a/doc/src/sgml/spgist.sgml
+++ b/doc/src/sgml/spgist.sgml
@@ -895,16 +895,16 @@ LANGUAGE C STRICT;
      </para>

      <para>
       The function [-has given-]{+is passed a+} pointer to {+a+} <replaceable>local_relopts</replaceable>
       struct, which needs to be filled with a set of operator class
       specific options.  The options can be accessed from other support
       functions using {+the+} <literal>PG_HAS_OPCLASS_OPTIONS()</literal> and
       <literal>PG_GET_OPCLASS_OPTIONS()</literal> macros.
      </para>

      <para>
       Since the representation of the key in <acronym>SP-GiST</acronym> is
       flexible, it may [-depends-]{+depend+} on user-specified parameters.
      </para>
     </listitem>
    </varlistentry>
diff --git a/doc/src/sgml/xindex.sgml b/doc/src/sgml/xindex.sgml
index 0e4587a81b..2cfd71b5b7 100644
--- a/doc/src/sgml/xindex.sgml
+++ b/doc/src/sgml/xindex.sgml
@@ -410,9 +410,9 @@
  </para>

  <para>
   Additionally, some opclasses allow [-user-]{+users+} to [-set specific parameters,-]{+specify parameters+} which
   [-controls its-]{+control their+} behavior.  Each builtin index access method [-have-]{+has an+} optional
   <function>options</function> support function, which defines {+a+} set of
   opclass-specific parameters.
  </para>

@@ -459,7 +459,7 @@
      </row>
      <row>
       <entry>
        Defines {+a+} set of options that are specific [-for-]{+to+} this operator class
        (optional)
       </entry>
       <entry>5</entry>
@@ -501,7 +501,7 @@
      </row>
      <row>
       <entry>
        Defines {+a+} set of options that are specific [-for-]{+to+} this operator class
        (optional)
       </entry>
       <entry>3</entry>
@@ -584,7 +584,7 @@
      <row>
       <entry><function>options</function></entry>
       <entry>
        Defines {+a+} set of options that are specific [-for-]{+to+} this operator class
        (optional)
       </entry>
       <entry>10</entry>
@@ -643,7 +643,7 @@
      <row>
       <entry><function>options</function></entry>
       <entry>
        Defines {+a+} set of options that are specific [-for-]{+to+} this operator class
        (optional)
       </entry>
       <entry>6</entry>
@@ -720,7 +720,7 @@
      <row>
       <entry><function>options</function></entry>
       <entry>
        Defines {+a+} set of options that are specific [-for-]{+to+} this operator class
        (optional)
       </entry>
       <entry>7</entry>
@@ -778,7 +778,7 @@
      <row>
       <entry><function>options</function></entry>
       <entry>
        Defines {+a+} set of options that are specific [-for-]{+to+} this operator class
        (optional)
       </entry>
       <entry>5</entry>

Patches and attachments: download all (4 files, .tar.gz)

v1-0001-Language-fixen-for-15cb2bd27009f73a84a35c2ba60fdd.patch (text/x-diff, 11.4K) download
From c590277f8ae3f77be420f93aeaece5cefdfd15a1 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Sat, 20 Jun 2020 16:37:51 -0500
Subject: [PATCH v1] Language fixen for
 15cb2bd27009f73a84a35c2ba60fdd105b4bf263

---
 doc/src/sgml/brin.sgml   | 12 ++++++------
 doc/src/sgml/btree.sgml  | 12 ++++++------
 doc/src/sgml/gin.sgml    | 10 +++++-----
 doc/src/sgml/gist.sgml   | 18 +++++++++---------
 doc/src/sgml/spgist.sgml |  6 +++---
 doc/src/sgml/xindex.sgml | 18 +++++++++---------
 6 files changed, 38 insertions(+), 38 deletions(-)

diff --git a/doc/src/sgml/brin.sgml b/doc/src/sgml/brin.sgml
index d7f1af7819..4c5eeb875f 100644
--- a/doc/src/sgml/brin.sgml
+++ b/doc/src/sgml/brin.sgml
@@ -562,7 +562,7 @@ typedef struct BrinOpcInfo
    </varlistentry>
   </variablelist>
 
-  Optionally, an operator class for <acronym>BRIN</acronym> can supply the
+  An operator class for <acronym>BRIN</acronym> can optionally specify the
   following method:
 
   <variablelist>
@@ -570,22 +570,22 @@ typedef struct BrinOpcInfo
      <term><function>void options(local_relopts *relopts)</function></term>
      <listitem>
       <para>
-       Defines set of user-visible parameters that control operator class
+       Defines a set of user-visible parameters that control operator class
        behavior.
       </para>
 
       <para>
-       The <function>options</function> function has given pointer to
+       The <function>options</function> function is passed a pointer to a
        <replaceable>local_relopts</replaceable> struct, which needs to be
        filled with a set of operator class specific options.  The options
-       can be accessed from other support functions using
+       can be accessed from other support functions using the
        <literal>PG_HAS_OPCLASS_OPTIONS()</literal> and
        <literal>PG_GET_OPCLASS_OPTIONS()</literal> macros.
       </para>
 
       <para>
-       Since both key extraction for indexed value and representation of the
-       key in <acronym>GIN</acronym> are flexible, it may depends on
+       Since both key extraction of indexed values and representation of the
+       key in <acronym>GIN</acronym> are flexible, they may depend on
        user-specified parameters.
       </para>
      </listitem>
diff --git a/doc/src/sgml/btree.sgml b/doc/src/sgml/btree.sgml
index 2c4dd48ea3..b17b166e84 100644
--- a/doc/src/sgml/btree.sgml
+++ b/doc/src/sgml/btree.sgml
@@ -557,7 +557,7 @@ equalimage(<replaceable>opcintype</replaceable> <type>oid</type>) returns bool
      Optionally, a B-tree operator family may provide
      <function>options</function> (<quote>operator class specific
      options</quote>) support functions, registered under support
-     function number 5.  These functions define set of user-visible
+     function number 5.  These functions define a set of user-visible
      parameters that control operator class behavior.
     </para>
     <para>
@@ -566,19 +566,19 @@ equalimage(<replaceable>opcintype</replaceable> <type>oid</type>) returns bool
 <synopsis>
 options(<replaceable>relopts</replaceable> <type>local_relopts *</type>) returns void
 </synopsis>
-     The function has given pointer to <replaceable>local_relopts</replaceable>
+     The function is passed a pointer to a <replaceable>local_relopts</replaceable>
      struct, which needs to be filled with a set of operator class
      specific options.  The options can be accessed from other support
-     functions using <literal>PG_HAS_OPCLASS_OPTIONS()</literal> and
+     functions using the <literal>PG_HAS_OPCLASS_OPTIONS()</literal> and
      <literal>PG_GET_OPCLASS_OPTIONS()</literal> macros.
     </para>
     <para>
-     Currently, no B-Tree operator class has <function>options</function>
+     Currently, no B-Tree operator class has an <function>options</function>
      support function.  B-tree doesn't allow flexible representation of keys
      like GiST, SP-GiST, GIN and BRIN do.  So, <function>options</function>
-     probably doesn't have much usage in current shape of B-tree index
+     probably doesn't have much application in the current B-tree index
      access method.  Nevertheless, this support function was added to B-tree
-     for uniformity, and probably it will found its usage during further
+     for uniformity, and will probably find uses during further
      evolution of B-tree in <productname>PostgreSQL</productname>.
     </para>
    </listitem>
diff --git a/doc/src/sgml/gin.sgml b/doc/src/sgml/gin.sgml
index d85e7c8796..7a8c18a449 100644
--- a/doc/src/sgml/gin.sgml
+++ b/doc/src/sgml/gin.sgml
@@ -411,17 +411,17 @@
       </para>
 
       <para>
-       The <function>options</function> function has given pointer to
+       The <function>options</function> function is passed a pointer to a
        <replaceable>local_relopts</replaceable> struct, which needs to be
-       filled with s set of operator class specific options.  The options
-       can be accessed from other support functions using
+       filled with a set of operator class specific options.  The options
+       can be accessed from other support functions using the
        <literal>PG_HAS_OPCLASS_OPTIONS()</literal> and
        <literal>PG_GET_OPCLASS_OPTIONS()</literal> macros.
       </para>
 
       <para>
-       Since both key extraction for indexed value and representation of the
-       key in <acronym>GIN</acronym> are flexible, it may depends on
+       Since both key extraction of indexed values and representation of the
+       key in <acronym>GIN</acronym> are flexible, they may depend on
        user-specified parameters.
       </para>
      </listitem>
diff --git a/doc/src/sgml/gist.sgml b/doc/src/sgml/gist.sgml
index 31c28fdb61..5d970ee9f2 100644
--- a/doc/src/sgml/gist.sgml
+++ b/doc/src/sgml/gist.sgml
@@ -946,7 +946,7 @@ my_fetch(PG_FUNCTION_ARGS)
      <term><function>options</function></term>
      <listitem>
       <para>
-       Allows defintion of user-visible parameters that control operator
+       Allows definition of user-visible parameters that control operator
        class behavior.
       </para>
 
@@ -962,16 +962,16 @@ LANGUAGE C STRICT;
       </para>
 
       <para>
-       The function has given pointer to <replaceable>local_relopts</replaceable>
+       The function is passed a pointer to a <replaceable>local_relopts</replaceable>
        struct, which needs to be filled with a set of operator class
        specific options.  The options can be accessed from other support
-       functions using <literal>PG_HAS_OPCLASS_OPTIONS()</literal> and
+       functions using the <literal>PG_HAS_OPCLASS_OPTIONS()</literal> and
        <literal>PG_GET_OPCLASS_OPTIONS()</literal> macros.
       </para>
 
        <para>
-        The sample implementation of my_option() and parameters usage
-        in the another support function are given below:
+        An example implementation of my_options() and parameters use
+        from other support functions are given below:
 
 <programlisting>
 typedef enum MyEnumType
@@ -990,7 +990,7 @@ typedef struct
     int     str_param;  /* string parameter */
 } MyOptionsStruct;
 
-/* String representations for enum values */
+/* String representation of enum values */
 static relopt_enum_elt_def myEnumValues[] =
 {
     {"on", MY_ENUM_ON},
@@ -1002,7 +1002,7 @@ static relopt_enum_elt_def myEnumValues[] =
 static char *str_param_default = "default";
 
 /*
- * Sample validatior: checks that string is not longer than 8 bytes.
+ * Sample validator: checks that string is not longer than 8 bytes.
  */
 static void 
 validate_my_string_relopt(const char *value)
@@ -1090,8 +1090,8 @@ my_compress(PG_FUNCTION_ARGS)
 
       <para>
        Since the representation of the key in <acronym>GiST</acronym> is
-       flexible, it may depends on user-specified parameters.  For instace,
-       the length of key signature may be such parameter.  See
+       flexible, it may depend on user-specified parameters.  For instance,
+       the length of key signature may be specified.  See
        <literal>gtsvector_options()</literal> for example.
       </para>
      </listitem>
diff --git a/doc/src/sgml/spgist.sgml b/doc/src/sgml/spgist.sgml
index 03f914735b..1395dbaf88 100644
--- a/doc/src/sgml/spgist.sgml
+++ b/doc/src/sgml/spgist.sgml
@@ -895,16 +895,16 @@ LANGUAGE C STRICT;
       </para>
 
       <para>
-       The function has given pointer to <replaceable>local_relopts</replaceable>
+       The function is passed a pointer to a <replaceable>local_relopts</replaceable>
        struct, which needs to be filled with a set of operator class
        specific options.  The options can be accessed from other support
-       functions using <literal>PG_HAS_OPCLASS_OPTIONS()</literal> and
+       functions using the <literal>PG_HAS_OPCLASS_OPTIONS()</literal> and
        <literal>PG_GET_OPCLASS_OPTIONS()</literal> macros.
       </para>
 
       <para>
        Since the representation of the key in <acronym>SP-GiST</acronym> is
-       flexible, it may depends on user-specified parameters.
+       flexible, it may depend on user-specified parameters.
       </para>
      </listitem>
     </varlistentry>
diff --git a/doc/src/sgml/xindex.sgml b/doc/src/sgml/xindex.sgml
index 0e4587a81b..2cfd71b5b7 100644
--- a/doc/src/sgml/xindex.sgml
+++ b/doc/src/sgml/xindex.sgml
@@ -410,9 +410,9 @@
   </para>
 
   <para>
-   Additionally, some opclasses allow user to set specific parameters, which
-   controls its behavior.  Each builtin index access method have optional
-   <function>options</function> support function, which defines set of
+   Additionally, some opclasses allow users to specify parameters which
+   control their behavior.  Each builtin index access method has an optional
+   <function>options</function> support function, which defines a set of
    opclass-specific parameters.
   </para>
 
@@ -459,7 +459,7 @@
       </row>
       <row>
        <entry>
-        Defines set of options that are specific for this operator class
+        Defines a set of options that are specific to this operator class
         (optional)
        </entry>
        <entry>5</entry>
@@ -501,7 +501,7 @@
       </row>
       <row>
        <entry>
-        Defines set of options that are specific for this operator class
+        Defines a set of options that are specific to this operator class
         (optional)
        </entry>
        <entry>3</entry>
@@ -584,7 +584,7 @@
       <row>
        <entry><function>options</function></entry>
        <entry>
-        Defines set of options that are specific for this operator class
+        Defines a set of options that are specific to this operator class
         (optional)
        </entry>
        <entry>10</entry>
@@ -643,7 +643,7 @@
       <row>
        <entry><function>options</function></entry>
        <entry>
-        Defines set of options that are specific for this operator class
+        Defines a set of options that are specific to this operator class
         (optional)
        </entry>
        <entry>6</entry>
@@ -720,7 +720,7 @@
       <row>
        <entry><function>options</function></entry>
        <entry>
-        Defines set of options that are specific for this operator class
+        Defines a set of options that are specific to this operator class
         (optional)
        </entry>
        <entry>7</entry>
@@ -778,7 +778,7 @@
       <row>
        <entry><function>options</function></entry>
        <entry>
-        Defines set of options that are specific for this operator class
+        Defines a set of options that are specific to this operator class
         (optional)
        </entry>
        <entry>5</entry>
-- 
2.17.0