Thread: --sync-method isn't documented to take an argument
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
> 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
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
> 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
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><spanclass="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><spanclass="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/
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
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
> 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
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
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
> 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
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
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
> 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
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
Attachment
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
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
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
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