Horton

Thread

Commits

  1. Document that --sync-method takes an argument.

  1. --sync-method isn't documented to take an argument

    Robert Haas <robertmhaas@gmail.com> — 2023-10-04T13:08:57Z

    The various command-line utilities that have recently acquired a
--sync-method option document it like this:

<term><option>--sync-method</option></term>

But that is not how we document options which take an argument. We do
it like this:

<term><option>--pgdata=<replaceable>directory</replaceable></option></term>
<term><option>--filenode=<replaceable>filenode</replaceable></option></term>

etc.

This one should be something like this:

<term><option>--sync-method=<replaceable>method</replaceable></option></term>

-- 
Robert Haas
EDB: http://www.enterprisedb.com

  2. Re: --sync-method isn't documented to take an argument

    Daniel Gustafsson <daniel@yesql.se> — 2023-10-04T13:15:37Z

    > On 4 Oct 2023, at 15:08, Robert Haas <robertmhaas@gmail.com> wrote:

> This one should be something like this:
> 
> <term><option>--sync-method=<replaceable>method</replaceable></option></term>

Shouldn't it be <replaceable class="parameter">method</replaceable> ?

--
Daniel Gustafsson

  3. Re: --sync-method isn't documented to take an argument

    Robert Haas <robertmhaas@gmail.com> — 2023-10-04T13:22:14Z

    On Wed, Oct 4, 2023 at 9:15 AM Daniel Gustafsson <daniel@yesql.se> wrote:
> > On 4 Oct 2023, at 15:08, Robert Haas <robertmhaas@gmail.com> wrote:
> > This one should be something like this:
> >
> > <term><option>--sync-method=<replaceable>method</replaceable></option></term>
>
> Shouldn't it be <replaceable class="parameter">method</replaceable> ?

Hmm, I think you're probably right. But look at this:

      <term><option>-S <replaceable>slotname</replaceable></option></term>
      <term><option>--slot=<replaceable
class="parameter">slotname</replaceable></option></term>

But then in the very same file:

      <term><option>-r <replaceable
class="parameter">rate</replaceable></option></term>
      <term><option>--max-rate=<replaceable
class="parameter">rate</replaceable></option></term>

It doesn't look to me like we're entirely consistent about this. I
also found this in vacuumlo.sgml, and there seem to be various other
examples:

<term><option>-U <replaceable>username</replaceable></option></term>

-- 
Robert Haas
EDB: http://www.enterprisedb.com

  4. Re: --sync-method isn't documented to take an argument

    Daniel Gustafsson <daniel@yesql.se> — 2023-10-04T13:34:49Z

    > On 4 Oct 2023, at 15:22, Robert Haas <robertmhaas@gmail.com> wrote:
> 
> On Wed, Oct 4, 2023 at 9:15 AM Daniel Gustafsson <daniel@yesql.se> wrote:
>>> On 4 Oct 2023, at 15:08, Robert Haas <robertmhaas@gmail.com> wrote:
>>> This one should be something like this:
>>> 
>>> <term><option>--sync-method=<replaceable>method</replaceable></option></term>
>> 
>> Shouldn't it be <replaceable class="parameter">method</replaceable> ?
> 
> Hmm, I think you're probably right. But look at this:
> 
>      <term><option>-S <replaceable>slotname</replaceable></option></term>
>      <term><option>--slot=<replaceable
> class="parameter">slotname</replaceable></option></term>
> 
> But then in the very same file:
> 
>      <term><option>-r <replaceable
> class="parameter">rate</replaceable></option></term>
>      <term><option>--max-rate=<replaceable
> class="parameter">rate</replaceable></option></term>

Hmm.. that's a bit unfortunate.

> It doesn't look to me like we're entirely consistent about this.

That (sadly) applies to a fair chunk of the docs.

I can take a stab at tidying this up during breaks at the conference.  It might
not be the most important bit of markup, but for anyone building the docs who
might want to use this it seems consistency will help.

--
Daniel Gustafsson

  5. Re: --sync-method isn't documented to take an argument

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2023-10-04T14:35:32Z

    On 2023-Oct-04, Daniel Gustafsson wrote:

> I can take a stab at tidying this up during breaks at the conference.  It might
> not be the most important bit of markup, but for anyone building the docs who
> might want to use this it seems consistency will help.

So for HTML, the result of the pg_basebackup lines are these two lines:

</p></dd><dt><span class="term"><code class="option">-S <em class="replaceable"><code>slotname</code></em></code><br /></span><span class="term"><code class="option">--slot=<em class="replaceable"><code>slotname</code></em></code></span></dt><dd><p>

and 

</p></dd><dt><span class="term"><code class="option">-r <em class="replaceable"><code>rate</code></em></code><br /></span><span class="term"><code class="option">--max-rate=<em class="replaceable"><code>rate</code></em></code></span></dt><dd><p>

So I'm not sure that specifying the class="parameter" bit does anything in
reality, or that changing lines to add or remove it will have any effect.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/

  6. Re: --sync-method isn't documented to take an argument

    Robert Haas <robertmhaas@gmail.com> — 2023-10-04T14:39:19Z

    On Wed, Oct 4, 2023 at 10:35 AM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> So I'm not sure that specifying the class="parameter" bit does anything in
> reality, or that changing lines to add or remove it will have any effect.

Interesting. I wondered whether that might be the case.

The original issue I reported does make a real difference, though. :-)

-- 
Robert Haas
EDB: http://www.enterprisedb.com

  7. Re: --sync-method isn't documented to take an argument

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-10-04T14:39:50Z

    Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> So I'm not sure that specifying the class="parameter" bit does anything in
> reality, or that changing lines to add or remove it will have any effect.

I concluded a long time ago that it does nothing.  We have a good
mix of places that write <replaceable> with or without that, and
I've never detected any difference in the output.  So I tend to
leave it out, or at most make new entries look like the adjacent
ones.

			regards, tom lane

  8. Re: --sync-method isn't documented to take an argument

    Daniel Gustafsson <daniel@yesql.se> — 2023-10-04T14:49:30Z

    > On 4 Oct 2023, at 16:39, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 
> Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
>> So I'm not sure that specifying the class="parameter" bit does anything in
>> reality, or that changing lines to add or remove it will have any effect.
> 
> I concluded a long time ago that it does nothing.

It does nothing in our current doc rendering, but if someone would like to
render docs with another style where it does make a difference it seems
unhelpful to not be consistent.

--
Daniel Gustafsson

  9. Re: --sync-method isn't documented to take an argument

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-10-04T14:51:59Z

    Daniel Gustafsson <daniel@yesql.se> writes:
> On 4 Oct 2023, at 16:39, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I concluded a long time ago that it does nothing.

> It does nothing in our current doc rendering, but if someone would like to
> render docs with another style where it does make a difference it seems
> unhelpful to not be consistent.

To do that, we'd need some sort of agreement on what the possible
"class" values are and when to use each one.  I've never seen any
documentation about that.

			regards, tom lane

  10. Re: --sync-method isn't documented to take an argument

    Nathan Bossart <nathandbossart@gmail.com> — 2023-10-04T14:56:35Z

    On Wed, Oct 04, 2023 at 09:08:57AM -0400, Robert Haas wrote:
> The various command-line utilities that have recently acquired a
> --sync-method option document it like this:
>
> <term><option>--sync-method</option></term>
>
> But that is not how we document options which take an argument. We do
> it like this:
>
> <term><option>--pgdata=<replaceable>directory</replaceable></option></term>
> <term><option>--filenode=<replaceable>filenode</replaceable></option></term>
>
> etc.
>
> This one should be something like this:
>
> <term><option>--sync-method=<replaceable>method</replaceable></option></term>

Whoops.  Thanks for pointing this out.  I'll get this fixed.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

  11. Re: --sync-method isn't documented to take an argument

    Daniel Gustafsson <daniel@yesql.se> — 2023-10-04T14:56:54Z

    > On 4 Oct 2023, at 16:51, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 
> Daniel Gustafsson <daniel@yesql.se> writes:
>> On 4 Oct 2023, at 16:39, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> I concluded a long time ago that it does nothing.
> 
>> It does nothing in our current doc rendering, but if someone would like to
>> render docs with another style where it does make a difference it seems
>> unhelpful to not be consistent.
> 
> To do that, we'd need some sort of agreement on what the possible
> "class" values are and when to use each one.  I've never seen any
> documentation about that.

Thats fair.  The 4.5 docbook guide isn't terribly helpful on what the
attributes mean and how they should be used:

	https://tdg.docbook.org/tdg/4.5/replaceable

In the 5.2 version the text is slightly expanded but not by all that much:

	https://tdg.docbook.org/tdg/5.2/replaceable

--
Daniel Gustafsson

  12. Re: --sync-method isn't documented to take an argument

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2023-10-04T15:00:26Z

    On 2023-Oct-04, Robert Haas wrote:

> The original issue I reported does make a real difference, though. :-)

Yes, absolutely, and I agree that it'd be better to get it fixed.

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
"Learn about compilers. Then everything looks like either a compiler or
a database, and now you have two problems but one of them is fun."
            https://twitter.com/thingskatedid/status/1456027786158776329

  13. Re: --sync-method isn't documented to take an argument

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-10-04T15:07:07Z

    Daniel Gustafsson <daniel@yesql.se> writes:
> On 4 Oct 2023, at 16:51, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> To do that, we'd need some sort of agreement on what the possible
>> "class" values are and when to use each one.  I've never seen any
>> documentation about that.

> Thats fair.  The 4.5 docbook guide isn't terribly helpful on what the
> attributes mean and how they should be used:
> 	https://tdg.docbook.org/tdg/4.5/replaceable
> In the 5.2 version the text is slightly expanded but not by all that much:
> 	https://tdg.docbook.org/tdg/5.2/replaceable

OK, so now I know what the possible values of "class" are, but
I'm still not seeing a reason why we shouldn't just assume that
"parameter" is the only one of interest.  We don't really have
places where "command" or "function" would be appropriate AFAIR.
As for "option", what's the distinction between that and
"parameter"?  And why wouldn't I use "<option>" instead?

Even if we did have uses for the other class values, I'm skeptical
that we'd ever use them consistently enough that there'd be
value in rendering them differently.

			regards, tom lane

  14. Re: --sync-method isn't documented to take an argument

    Daniel Gustafsson <daniel@yesql.se> — 2023-10-04T15:16:48Z

    > On 4 Oct 2023, at 17:07, Tom Lane <tgl@sss.pgh.pa.us> wrote:

> OK, so now I know what the possible values of "class" are, but
> I'm still not seeing a reason why we shouldn't just assume that
> "parameter" is the only one of interest.

I think "parameter" is the only one of interest for this usecase (params to
application options), but it might not necessarily be applicable to other uses
of <replaceable /> we have in the tree, like for example in bki.sgml:

	<replaceable>oprname(lefttype,righttype)</replaceable>

But since we don't know if anyone is rendering our docs with a custom style, or
ever will, this is all very theoretical.  Maybe skipping the class attribute is
the right choice for consistency?

--
Daniel Gustafsson

  15. Re: --sync-method isn't documented to take an argument

    Nathan Bossart <nathandbossart@gmail.com> — 2023-10-04T15:27:50Z

    On Wed, Oct 04, 2023 at 05:00:26PM +0200, Alvaro Herrera wrote:
> On 2023-Oct-04, Robert Haas wrote:
> 
>> The original issue I reported does make a real difference, though. :-)
> 
> Yes, absolutely, and I agree that it'd be better to get it fixed.

Here's a patch.  I didn't address the class="parameter" stuff at all.  I
figured it would be best to handle that separately.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

  16. Re: --sync-method isn't documented to take an argument

    Robert Haas <robertmhaas@gmail.com> — 2023-10-04T15:51:32Z

    On Wed, Oct 4, 2023 at 11:27 AM Nathan Bossart <nathandbossart@gmail.com> wrote:
> Here's a patch.  I didn't address the class="parameter" stuff at all.  I
> figured it would be best to handle that separately.

I guess I'll vote for including class=parameter in this addition for
now, as that appears to be the majority position in the documentation
today. If we get a consensus to change something, so be it. But also,
if you don't want to do that, so be it.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

  17. Re: --sync-method isn't documented to take an argument

    Nathan Bossart <nathandbossart@gmail.com> — 2023-10-04T16:12:14Z

    On Wed, Oct 04, 2023 at 11:51:32AM -0400, Robert Haas wrote:
> On Wed, Oct 4, 2023 at 11:27 AM Nathan Bossart <nathandbossart@gmail.com> wrote:
>> Here's a patch.  I didn't address the class="parameter" stuff at all.  I
>> figured it would be best to handle that separately.
> 
> I guess I'll vote for including class=parameter in this addition for
> now, as that appears to be the majority position in the documentation
> today. If we get a consensus to change something, so be it. But also,
> if you don't want to do that, so be it.

WFM.  I'll add it before committing, which I plan to do later today.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

  18. Re: --sync-method isn't documented to take an argument

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-10-04T16:24:36Z

    Robert Haas <robertmhaas@gmail.com> writes:
> On Wed, Oct 4, 2023 at 11:27 AM Nathan Bossart <nathandbossart@gmail.com> wrote:
>> Here's a patch.  I didn't address the class="parameter" stuff at all.  I
>> figured it would be best to handle that separately.

> I guess I'll vote for including class=parameter in this addition for
> now, as that appears to be the majority position in the documentation
> today. If we get a consensus to change something, so be it. But also,
> if you don't want to do that, so be it.

FWIW, I just did a little sed hacking to count the instances of the
different cases in the docs as of today.  I found

   4038 <replaceable>
      3 <replaceable class="command">
   4017 <replaceable class="parameter">

The three with "command" are all in plpgsql.sgml, and are marking up
query strings in the synopses of EXECUTE and variants.  I'm inclined
to argue that those three are actually wrong, on the grounds that

(1) From the perspective of EXECUTE, you could equally well say that
the string to be executed is a parameter;

(2) Our general convention elsewhere is that "command" refers to a
command type such as SELECT or UPDATE, not to a complete query string.

In any case, trying to standardize this looks like it would be a
huge amount of churn for very little gain.  I'd recommend making
your markup look similar to what's immediately adjacent, if possible,
and not sweating too much otherwise.

			regards, tom lane

  19. Re: --sync-method isn't documented to take an argument

    Nathan Bossart <nathandbossart@gmail.com> — 2023-10-04T19:50:59Z

    On Wed, Oct 04, 2023 at 12:24:36PM -0400, Tom Lane wrote:
> In any case, trying to standardize this looks like it would be a
> huge amount of churn for very little gain.  I'd recommend making
> your markup look similar to what's immediately adjacent, if possible,
> and not sweating too much otherwise.

I matched the adjacent options as you suggested.  Perhaps unsurprisingly,
the inclusion of class="parameter" is not the only inconsistency.  I also
found that pg_upgrade adds the </option> before the argument name!

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com