Horton

Thread

Commits

  1. doc: Make HTML ids discoverable

  2. Add missing XML ID attribute

  3. postgres_fdw: Add support for parallel abort.

  4. Add missing XML ID attributes

  5. Add XML ID attributes to create_subscription.sgml.

  6. Stop recommending auto-download of DTD files, and indeed disable it.

  7. Doc: add XML ID attributes to <sectN> and <varlistentry> tags.

  8. Allow left join removals and unique joins on partitioned tables

  9. Improve GIN cost estimation

  1. doc: add missing "id" attributes to extension packaging page

    Ian Lawrence Barwick <barwick@gmail.com> — 2022-12-05T01:50:16Z

    Hi

On this page:

  https://www.postgresql.org/docs/current/extend-extensions.html

three of the <sect2> sections are missing an "id" attribute; patch adds
these. Noticed when trying to create a stable link to one of the affected
sections.

Regards

Ian Barwick

  2. Re: doc: add missing "id" attributes to extension packaging page

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2022-12-05T09:56:31Z

    On 2022-Dec-05, Ian Lawrence Barwick wrote:

> On this page:
> 
>   https://www.postgresql.org/docs/current/extend-extensions.html
> 
> three of the <sect2> sections are missing an "id" attribute; patch adds
> these. Noticed when trying to create a stable link to one of the affected
> sections.

Hm, I was reminded of this patch here that adds IDs in a lot of places
https://postgr.es/m/3bac458c-b121-1b20-8dea-0665986faa40@gmx.de
and this other one 
https://postgr.es/m/76287ac6-f415-8562-fdaa-5876380c05f3@gmx.de
which adds XSL stuff for adding selectable anchors next to each
id-carrying item.

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/

  3. Re: doc: add missing "id" attributes to extension packaging page

    Ian Lawrence Barwick <barwick@gmail.com> — 2022-12-06T00:55:18Z

    2022年12月5日(月) 18:56 Alvaro Herrera <alvherre@alvh.no-ip.org>:
>
> On 2022-Dec-05, Ian Lawrence Barwick wrote:
>
> > On this page:
> >
> >   https://www.postgresql.org/docs/current/extend-extensions.html
> >
> > three of the <sect2> sections are missing an "id" attribute; patch adds
> > these. Noticed when trying to create a stable link to one of the affected
> > sections.
>
> Hm, I was reminded of this patch here that adds IDs in a lot of places
> https://postgr.es/m/3bac458c-b121-1b20-8dea-0665986faa40@gmx.de
> and this other one
> https://postgr.es/m/76287ac6-f415-8562-fdaa-5876380c05f3@gmx.de
> which adds XSL stuff for adding selectable anchors next to each
> id-carrying item.

Oh, now you mention it, I vaguely recall seeing those. However the thread
stalled back in March and the patches don't seem to have made it to a
CommitFest entry. Brar, would you like to add an entry so they don't get
lost? See: https://commitfest.postgresql.org/41/

The items in my patch are covered by the above so disregard that.

Regards

Ian Barwick

  4. Re: doc: add missing "id" attributes to extension packaging page

    Brar Piening <brar@gmx.de> — 2022-12-06T08:25:34Z

    On 06.12.2022 at 01:55, Ian Lawrence Barwick wrote:

> Oh, now you mention it, I vaguely recall seeing those. However the thread
> stalled back in March and the patches don't seem to have made it to a
> CommitFest entry.

Yes, my patches added quite a few ids and also some xsl/css logic to
make them more discoverable in the browser but I had gotten the
impression that nobody besides me cares about this, so I didn't push it
any further.

> Brar, would you like to add an entry so they don't get
> lost? See: https://commitfest.postgresql.org/41/

Yes. I can certainly add them to the commitfest although I'm not sure if
they still apply cleanly.

I can also rebase or extend them if somebody cares.

Regards,

Brar

  5. Re: doc: add missing "id" attributes to extension packaging page

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2022-12-06T08:38:09Z

    On 2022-Dec-06, Brar Piening wrote:

> On 06.12.2022 at 01:55, Ian Lawrence Barwick wrote:
> 
> > Oh, now you mention it, I vaguely recall seeing those. However the thread
> > stalled back in March and the patches don't seem to have made it to a
> > CommitFest entry.
> 
> Yes, my patches added quite a few ids and also some xsl/css logic to
> make them more discoverable in the browser but I had gotten the
> impression that nobody besides me cares about this, so I didn't push it
> any further.

I care.  The problem last time is that we were in the middle of the last
commitfest, so we were (or at least I was) distracted by other stuff.

Looking at the resulting psql page,
https://pgdocs.piening.info/app-psql.html#APP-PSQL-OPTIONS-EXPANDED
I note that the ID for the -x option is called "options-blah".  I
understand where does this come from: it's the "expanded" bit in the
"options" section.  However, put together it's a bit silly to have
"options" in plural there; it would make more sense to have it be
https://pgdocs.piening.info/app-psql.html#APP-PSQL-OPTION-EXPANDED
(where you can read more naturally "the expanded option for psql").
How laborious would it be to make it so?

> Yes. I can certainly add them to the commitfest although I'm not sure if
> they still apply cleanly.

It'll probably have some conflicts, yeah.

> I can also rebase or extend them if somebody cares.

I would welcome separate patches: one to add the IDs, another for the
XSL/CSS stuff.  That allows us to discuss them separately.

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

  6. Re: doc: add missing "id" attributes to extension packaging page

    Brar Piening <brar@gmx.de> — 2022-12-06T17:59:59Z

    On 06.12.2022 at 09:38, Alvaro Herrera wrote:
> I care.  The problem last time is that we were in the middle of the last
> commitfest, so we were (or at least I was) distracted by other stuff.

Ok, thanks. That's appreciated and understood.


> Looking at the resulting psql page,
> https://pgdocs.piening.info/app-psql.html#APP-PSQL-OPTIONS-EXPANDED
> I note that the ID for the -x option is called "options-blah".  I
> understand where does this come from: it's the "expanded" bit in the
> "options" section.  However, put together it's a bit silly to have
> "options" in plural there; it would make more sense to have it be
> https://pgdocs.piening.info/app-psql.html#APP-PSQL-OPTION-EXPANDED
> (where you can read more naturally "the expanded option for psql").
> How laborious would it be to make it so?

No problem. I've already done it. Your second link should work now.


> It'll probably have some conflicts, yeah.

I've updated  and rebased my branch on current master now.


> I would welcome separate patches: one to add the IDs, another for the
> XSL/CSS stuff.  That allows us to discuss them separately.

I'll send two patches in two separate e-mails in a moment.

Regards,

Brar

  7. Re: doc: add missing "id" attributes to extension packaging page

    Brar Piening <brar@gmx.de> — 2022-12-06T18:11:57Z

    On 06.12.2022 at 18:59, Brar Piening wrote:
> On 06.12.2022 at 09:38, Alvaro Herrera wrote:
>> I would welcome separate patches: one to add the IDs, another for the
>> XSL/CSS stuff.  That allows us to discuss them separately.
>
> I'll send two patches in two separate e-mails in a moment.

This is patch no 1 that adds ids to various elements without making them
visible on the HTML surface.
It is an updated and rebased version of the work discussed in the
following thread:

https://www.postgresql.org/message-id/f900c5e1-a18a-84cc-6536-e85ec655c7d7%40gmx.de

The current statistics for docbook elements with/without ids after
applying the patch are the following:

name | with_id | without_id | id_coverage | min_id_len | max_id_len
---------------+---------+------------+-------------+------------+------------
sect2 | 870 | 0 | 100.00 | 7 | 57 sect1 | 739 | 0 | 100.00 | 4 | 46
refentry | 307 | 0 | 100.00 | 8 | 37 sect3 | 206 | 0 | 100.00 | 11 | 57
chapter | 77 | 0 | 100.00 | 5 | 24 sect4 | 28 | 0 | 100.00 | 16 | 47
biblioentry | 23 | 0 | 100.00 | 6 | 15 simplesect | 20 | 0 | 100.00 | 24
| 39 appendix | 15 | 0 | 100.00 | 7 | 23 part | 8 | 0 | 100.00 | 5 | 20
co | 4 | 0 | 100.00 | 18 | 30 figure | 3 | 0 | 100.00 | 13 | 28
reference | 3 | 0 | 100.00 | 14 | 18 anchor | 1 | 0 | 100.00 | 21 | 21
bibliography | 1 | 0 | 100.00 | 8 | 8 book | 1 | 0 | 100.00 | 10 | 10
index | 1 | 0 | 100.00 | 11 | 11 legalnotice | 1 | 0 | 100.00 | 13 | 13
preface | 1 | 0 | 100.00 | 9 | 9 glossentry | 119 | 14 | 89.47 | 13 | 32
table | 285 | 162 | 63.76 | 12 | 56 example | 27 | 16 | 62.79 | 12 | 42
refsect3 | 5 | 3 | 62.50 | 19 | 24 refsect2 | 41 | 56 | 42.27 | 10 | 36
varlistentry | 1701 | 3172 | 34.91 | 9 | 64 footnote | 5 | 18 | 21.74 |
17 | 32 step | 28 | 130 | 17.72 | 7 | 28 refsect1 | 151 | 1333 | 10.18 |
15 | 40 informaltable | 1 | 15 | 6.25 | 25 | 25 phrase | 2 | 94 | 2.08 |
20 | 26 indexterm | 5 | 3262 | 0.15 | 17 | 26 variablelist | 1 | 813 |
0.12 | 21 | 21 function | 4 | 4011 | 0.10 | 12 | 28 entry | 11 | 17740 |
0.06 | 21 | 40 para | 3 | 25734 | 0.01 | 21 | 27

Regards,

Brar

  8. Re: doc: add missing "id" attributes to extension packaging page

    Brar Piening <brar@gmx.de> — 2022-12-06T18:16:51Z

    On 06.12.2022 at 18:59, Brar Piening wrote:
> On 06.12.2022 at 09:38, Alvaro Herrera wrote:
>> I would welcome separate patches: one to add the IDs, another for the
>> XSL/CSS stuff.  That allows us to discuss them separately.
>
> I'll send two patches in two separate e-mails in a moment.

This is patch no 2 that adds links to html elements with ids to make
them visible on the HTML surface when hovering the element.

Regards,

Brar

  9. Re: doc: add missing "id" attributes to extension packaging page

    Brar Piening <brar@gmx.de> — 2022-12-06T18:35:01Z

    On 06.12.2022 at 19:11, Brar Piening wrote:
> The current statistics for docbook elements with/without ids after 
> applying the patch are the following: 

Somehow my e-mail client destroyed the table. That's how it was supposed 
to look like:

      name      | with_id | without_id | id_coverage | min_id_len | 
max_id_len
---------------+---------+------------+-------------+------------+------------
  sect2         |     870 |          0 |      100.00 |          7 
|         57
  sect1         |     739 |          0 |      100.00 |          4 
|         46
  refentry      |     307 |          0 |      100.00 |          8 
|         37
  sect3         |     206 |          0 |      100.00 |         11 
|         57
  chapter       |      77 |          0 |      100.00 |          5 
|         24
  sect4         |      28 |          0 |      100.00 |         16 
|         47
  biblioentry   |      23 |          0 |      100.00 |          6 
|         15
  simplesect    |      20 |          0 |      100.00 |         24 
|         39
  appendix      |      15 |          0 |      100.00 |          7 
|         23
  part          |       8 |          0 |      100.00 |          5 
|         20
  co            |       4 |          0 |      100.00 |         18 
|         30
  figure        |       3 |          0 |      100.00 |         13 
|         28
  reference     |       3 |          0 |      100.00 |         14 
|         18
  anchor        |       1 |          0 |      100.00 |         21 
|         21
  bibliography  |       1 |          0 |      100.00 |          8 
|          8
  book          |       1 |          0 |      100.00 |         10 
|         10
  index         |       1 |          0 |      100.00 |         11 
|         11
  legalnotice   |       1 |          0 |      100.00 |         13 
|         13
  preface       |       1 |          0 |      100.00 |          9 
|          9
  glossentry    |     119 |         14 |       89.47 |         13 
|         32
  table         |     285 |        162 |       63.76 |         12 
|         56
  example       |      27 |         16 |       62.79 |         12 
|         42
  refsect3      |       5 |          3 |       62.50 |         19 
|         24
  refsect2      |      41 |         56 |       42.27 |         10 
|         36
  varlistentry  |    1701 |       3172 |       34.91 |          9 
|         64
  footnote      |       5 |         18 |       21.74 |         17 
|         32
  step          |      28 |        130 |       17.72 |          7 
|         28
  refsect1      |     151 |       1333 |       10.18 |         15 
|         40
  informaltable |       1 |         15 |        6.25 |         25 
|         25
  phrase        |       2 |         94 |        2.08 |         20 
|         26
  indexterm     |       5 |       3262 |        0.15 |         17 
|         26
  variablelist  |       1 |        813 |        0.12 |         21 
|         21
  function      |       4 |       4011 |        0.10 |         12 
|         28
  entry         |      11 |      17740 |        0.06 |         21 
|         40
  para          |       3 |      25734 |        0.01 |         21 
|         27

  10. Re: doc: add missing "id" attributes to extension packaging page

    Karl O. Pinc <kop@karlpinc.com> — 2023-01-02T20:53:54Z

    Hello,

Attached is a new patch (add_html_ids_v2.patch), almost identical to
the old but modified so that it applies.  There were 2 changes (made
to sgml/plpgsql.stml and sgml/ddl.sgml) that prevented the patch from
applying.  (In ddl.sgml the VACUUM and ANALYZE privileges were
combined into MAINTAIN.  In plpgsql.sgml an id attribute was added.)

If the author will look over my version of the patch I believe it can
be approved and sent on to the committers.


What follows is my notes for the committers:

The ids look reasonably consistent, with "nesting" so that ids of
sub-sections mostly have (at least some of) the id of the parent
section as a prefix.  There are a few inconsistencies.  A sect3 has
id="collation-managing-standard" and sect4 has
id="collation-managing-predefined".  There is a slight possibility of
conflict, as in this case sect4 ids omit the last word of the section
3 ids it is possible to have conflicts with the ids of the sect4s in
other sect3s of the same file.  I don't have a problem with this.

(I see establishing strict standards for id values as excessive.)

The above was the only case I noticed.  I also tried counting words,
"-" delimited, in ids and found no cases with fewer words than the
number of section levels.   Here's the hack:

egrep '^\+ *<sect' /tmp/add_html_ids.patch \
  | gawk '{if (int(substr($2, length($2), 1)) < split($2, dummy, "-"))
               print $0;}'

As far as I know the ids are consistent with the rest of the
documentation.  They are not entirely consistent in construction.
Mostly they copy the section title, but sometimes words are omitted.
E.g in sgml/charset.sgml where sect2 is "Managing Collations" with
id="collation-managing" and sect3 is "Standard Collations" with
id="collation-managing-standard". Also there is at least one
abbreviation in the id of a word in the title.
(id="installation-notes-aix-mem-management" v.s a title of "Memory
Management") All this seems fine to me.

The ids are sometimes very long.  This also seems ok.

I did not do a particularly close look at the id values for
varlistentrys. Scanning the patch they seem fine.

I can confirm that all the patch does is add ids.

Regards,

Karl <kop@karlpinc.com>
Free Software:  "You don't pay back, you pay forward."
                 -- Robert A. Heinlein

  11. Re: doc: add missing "id" attributes to extension packaging page

    Brar Piening <brar@gmx.de> — 2023-01-03T20:35:09Z

    On 02.01.2023 at 21:53, Karl O. Pinc wrote:
> If the author will look over my version of the patch I believe it can
> be approved and sent on to the committers.

LGTM.

Thanks!

Brar

  12. Re: doc: add missing "id" attributes to extension packaging page

    Karl O. Pinc <kop@karlpinc.com> — 2023-01-03T22:43:34Z

    On Tue, 3 Jan 2023 21:35:09 +0100
Brar Piening <brar@gmx.de> wrote:

> On 02.01.2023 at 21:53, Karl O. Pinc wrote:
> > If the author will look over my version of the patch I believe it
> > can be approved and sent on to the committers.  
> 
> LGTM.

Approved for committer!

Regards,

Karl <kop@karlpinc.com>
Free Software:  "You don't pay back, you pay forward."
                 -- Robert A. Heinlein

  13. Re: doc: add missing "id" attributes to extension packaging page

    vignesh C <vignesh21@gmail.com> — 2023-01-09T02:31:37Z

    On Wed, 4 Jan 2023 at 04:13, Karl O. Pinc <kop@karlpinc.com> wrote:
>
> On Tue, 3 Jan 2023 21:35:09 +0100
> Brar Piening <brar@gmx.de> wrote:
>
> > On 02.01.2023 at 21:53, Karl O. Pinc wrote:
> > > If the author will look over my version of the patch I believe it
> > > can be approved and sent on to the committers.
> >
> > LGTM.
>
> Approved for committer!

The patch does not apply on top of HEAD as in [1], please post a rebased patch:
=== Applying patches on top of PostgreSQL commit ID
cd9479af2af25d7fa9bfd24dd4dcf976b360f077 ===
=== applying patch ./add_html_ids_v2.patch
....
patching file doc/src/sgml/ref/pgbench.sgml
patching file doc/src/sgml/ref/psql-ref.sgml
Hunk #73 FAILED at 1824.
....
2 out of 208 hunks FAILED -- saving rejects to file
doc/src/sgml/ref/psql-ref.sgml.rej

[1] - http://cfbot.cputube.org/patch_41_4041.log

Regards,
Vignesh

  14. Re: doc: add missing "id" attributes to extension packaging page

    vignesh C <vignesh21@gmail.com> — 2023-01-09T02:38:22Z

    On Mon, 9 Jan 2023 at 08:01, vignesh C <vignesh21@gmail.com> wrote:
>
> On Wed, 4 Jan 2023 at 04:13, Karl O. Pinc <kop@karlpinc.com> wrote:
> >
> > On Tue, 3 Jan 2023 21:35:09 +0100
> > Brar Piening <brar@gmx.de> wrote:
> >
> > > On 02.01.2023 at 21:53, Karl O. Pinc wrote:
> > > > If the author will look over my version of the patch I believe it
> > > > can be approved and sent on to the committers.
> > >
> > > LGTM.
> >
> > Approved for committer!
>
> The patch does not apply on top of HEAD as in [1], please post a rebased patch:
> === Applying patches on top of PostgreSQL commit ID
> cd9479af2af25d7fa9bfd24dd4dcf976b360f077 ===
> === applying patch ./add_html_ids_v2.patch
> ....
> patching file doc/src/sgml/ref/pgbench.sgml
> patching file doc/src/sgml/ref/psql-ref.sgml
> Hunk #73 FAILED at 1824.
> ....
> 2 out of 208 hunks FAILED -- saving rejects to file
> doc/src/sgml/ref/psql-ref.sgml.rej
>
> [1] - http://cfbot.cputube.org/patch_41_4041.log

There are couple of commitfest entries for this:
https://commitfest.postgresql.org/41/4041/
https://commitfest.postgresql.org/41/4042/

Can one of them be closed?

Regards,
Vignesh

  15. Re: doc: add missing "id" attributes to extension packaging page

    Brar Piening <brar@gmx.de> — 2023-01-09T07:09:02Z

    On 09.01.2023 at 03:31, vignesh C wrote:
> The patch does not apply on top of HEAD as in [1], please post a rebased patch:

Voilà

This one applies on top of 3c569049b7b502bb4952483d19ce622ff0af5fd6 and
the documentation build succeeds. Beyond rebasing I've added a few more
ids (to make the other patch (make_html_ids_discoverable.patch) build
without warnings again) but nothing that would justify another review.

We probably have to move quickly with this patch since it touches pretty
much any file in the documentation and will be outdated in a minute.

Regards,

Brar

  16. Re: doc: add missing "id" attributes to extension packaging page

    Brar Piening <brar@gmx.de> — 2023-01-09T07:31:22Z

    On 09.01.2023 at 03:38, vignesh C wrote:
> There are couple of commitfest entries for this:
> https://commitfest.postgresql.org/41/4041/
> https://commitfest.postgresql.org/41/4042/ Can one of them be closed?
I've split the initial patch into two parts upon Álvaro's request in [1]
so that we can discuss them separately

https://commitfest.postgresql.org/41/4041/ is tracking the patch you've
been trying to apply and that I've just sent a rebased version for. It
only adds (invisible) ids to the HTML documentation and can be closed
once you've applied the patch.
https://commitfest.postgresql.org/41/4042/ is tracking a different patch
that makes the ids and the corresponding links discoverable at the HTML
surface. Hover one of the psql options in [2] to see the behavior. This
one still needs reviewing and there is no discussion around it yet.

Regards,
Brar

[1]
https://www.postgresql.org/message-id/20221206083809.3kaygnh2xswoxslj%40alvherre.pgsql
[2] https://pgdocs.piening.info/app-psql.html#APP-PSQL-OPTION-PORT

  17. Re: doc: add missing "id" attributes to extension packaging page

    Karl O. Pinc <kop@karlpinc.com> — 2023-01-09T15:53:38Z

    On Mon, 9 Jan 2023 08:09:02 +0100
Brar Piening <brar@gmx.de> wrote:

> On 09.01.2023 at 03:31, vignesh C wrote:
> > The patch does not apply on top of HEAD as in [1], please post a
> > rebased patch:  

> This one applies on top of 3c569049b7b502bb4952483d19ce622ff0af5fd6
> and the documentation build succeeds. Beyond rebasing I've added a
> few more ids (to make the other patch
> (make_html_ids_discoverable.patch) build without warnings again) but
> nothing that would justify another review.

Agreed.  I believe that as long as your system has xmllint installed
and the documentation builds there's not a lot that can go wrong.
This patch only adds lots-of-id attributes.

> We probably have to move quickly with this patch since it touches
> pretty much any file in the documentation and will be outdated in a
> minute.

+1

Regards,

Karl <kop@karlpinc.com>
Free Software:  "You don't pay back, you pay forward."
                 -- Robert A. Heinlein

  18. Re: doc: add missing "id" attributes to extension packaging page

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-01-09T20:18:18Z

    Brar Piening <brar@gmx.de> writes:
> On 09.01.2023 at 03:38, vignesh C wrote:
>> There are couple of commitfest entries for this:
>> https://commitfest.postgresql.org/41/4041/
>> https://commitfest.postgresql.org/41/4042/ Can one of them be closed?

> I've split the initial patch into two parts upon Álvaro's request in [1]
> so that we can discuss them separately

It's not great to have multiple CF entries pointing at the same email
thread --- it confuses both people and bots.  Next time please split
off a thread for each distinct patch.

I pushed the ID-addition patch, with a few fixes:

* AFAIK our practice is to use "-" never "_" in XML ID attributes.
You weren't very consistent about that even within this patch, and
the overall effect would have been to have no standard about that
at all, which doesn't seem great.  I changed them all to "-".

* I got rid of a couple of "-et-al" additions, because it did not
seem like a good precedent.  That would tempt people to modify
existing ID tags when adding variables to an entry, which'd defeat
the purpose I think.

* I fixed a couple of things that looked like typos or unnecessary
inconsistencies.  I have to admit that my eyes glazed over after
awhile, so there might be remaining infelicities.

It's probably going to be necessary to have follow-on patches,
because I'm sure there is stuff in the pipeline that adds more
ID-less tags.  Or do we have a way to create warnings about that?

I'm unqualified to review CSS stuff, so you'll need to get somebody
else to review that patch.  But I'd suggest reposting it, else
the cfbot is going to start whining that the patch-of-record in
this thread no longer applies.

			regards, tom lane

  19. Re: doc: add missing "id" attributes to extension packaging page

    Karl O. Pinc <kop@karlpinc.com> — 2023-01-09T20:45:57Z

    On Mon, 09 Jan 2023 15:18:18 -0500
Tom Lane <tgl@sss.pgh.pa.us> wrote:

> I pushed the ID-addition patch, with a few fixes:
> 
> * AFAIK our practice is to use "-" never "_" in XML ID attributes.
> You weren't very consistent about that even within this patch, and
> the overall effect would have been to have no standard about that
> at all, which doesn't seem great.  I changed them all to "-".

Apologies for not catching this.

Regards,

Karl <kop@karlpinc.com>
Free Software:  "You don't pay back, you pay forward."
                 -- Robert A. Heinlein

  20. Re: doc: add missing "id" attributes to extension packaging page

    Karl O. Pinc <kop@karlpinc.com> — 2023-01-09T22:28:21Z

    On Mon, 09 Jan 2023 15:18:18 -0500
Tom Lane <tgl@sss.pgh.pa.us> wrote:

> Brar Piening <brar@gmx.de> writes:
> > On 09.01.2023 at 03:38, vignesh C wrote:  
> >> There are couple of commitfest entries for this:
> >> https://commitfest.postgresql.org/41/4041/
> >> https://commitfest.postgresql.org/41/4042/ Can one of them be
> >> closed?  
> 
> > I've split the initial patch into two parts upon Álvaro's request
> > in [1] so that we can discuss them separately  

> I pushed the ID-addition patch, with a few fixes:

> It's probably going to be necessary to have follow-on patches,
> because I'm sure there is stuff in the pipeline that adds more
> ID-less tags.  Or do we have a way to create warnings about that?

I am unclear on how to make warnings with xslt.  You can make
a listing of problems, but who would read it if the build
completed successfully?  You can make errors and abort.

But my xslt and docbook and pg-docs-fu are a bit stale.

I think there's more to comment on regards the xslt in the
other patch, but I'll wait for the new thread for that patch.
That might be where there should be warnings raised, etc.

Regards,

Karl <kop@karlpinc.com>
Free Software:  "You don't pay back, you pay forward."
                 -- Robert A. Heinlein

  21. Re: doc: add missing "id" attributes to extension packaging page

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-01-09T22:30:59Z

    "Karl O. Pinc" <kop@karlpinc.com> writes:
> I think there's more to comment on regards the xslt in the
> other patch, but I'll wait for the new thread for that patch.
> That might be where there should be warnings raised, etc.

We can continue using this thread, now that the other commit is in.

			regards, tom lane

  22. Re: doc: add missing "id" attributes to extension packaging page

    Brar Piening <brar@gmx.de> — 2023-01-10T05:12:44Z

    On 09.01.2023 at 21:18, Tom Lane wrote:
> It's not great to have multiple CF entries pointing at the same email
> thread --- it confuses both people and bots.  Next time please split
> off a thread for each distinct patch.

I agree. I had overestimated the cfbot's ability to handle branched
threads. I'll create separate threads next time.


> * AFAIK our practice is to use "-" never "_" in XML ID attributes.
> You weren't very consistent about that even within this patch, and
> the overall effect would have been to have no standard about that
> at all, which doesn't seem great.  I changed them all to "-".

Noted. Maybe it's worth to write a short paragraph about Ids and their
style somewhere in the docs (e.g.  Appendix J.5).


> * I got rid of a couple of "-et-al" additions, because it did not
> seem like a good precedent.  That would tempt people to modify
> existing ID tags when adding variables to an entry, which'd defeat
> the purpose I think.

I tried to use it sparsely, mostly where a varlistentry had multiple
child items and I had arbitrarily pick one of them. It's not important,
though. I'm curious how you solved this.


> * I fixed a couple of things that looked like typos or unnecessary
> inconsistencies.  I have to admit that my eyes glazed over after
> awhile, so there might be remaining infelicities.

I'm all for consistency. The only places where I intentionally refrained
from being consistent was where I felt Ids would get too long or where
there were already ids in place that didn't match my naming scheme.


> It's probably going to be necessary to have follow-on patches,
> because I'm sure there is stuff in the pipeline that adds more
> ID-less tags.  Or do we have a way to create warnings about that?

Agreed. And yes, we do have a limited way to create warnings (that's
part of the other patch).


> I'm unqualified to review CSS stuff, so you'll need to get somebody
> else to review that patch.  But I'd suggest reposting it, else
> the cfbot is going to start whining that the patch-of-record in
> this thread no longer applies.

I will do that. Thanks for your feedback!

Regards,

Brar

  23. Re: doc: add missing "id" attributes to extension packaging page

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-01-10T05:22:53Z

    Brar Piening <brar@gmx.de> writes:
> On 09.01.2023 at 21:18, Tom Lane wrote:
>> * I got rid of a couple of "-et-al" additions, because it did not
>> seem like a good precedent.  That would tempt people to modify
>> existing ID tags when adding variables to an entry, which'd defeat
>> the purpose I think.

> I tried to use it sparsely, mostly where a varlistentry had multiple
> child items and I had arbitrarily pick one of them. It's not important,
> though. I'm curious how you solved this.

I just removed "-et-al", I didn't question your choice of the principal
variable name.  As you say, it didn't seem to matter that much.

			regards, tom lane

  24. Re: doc: add missing "id" attributes to extension packaging page

    Brar Piening <brar@gmx.de> — 2023-01-10T05:28:10Z

    On 09.01.2023 at 23:28, Karl O. Pinc wrote:
> On Mon, 09 Jan 2023 15:18:18 -0500
> Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> It's probably going to be necessary to have follow-on patches,
>> because I'm sure there is stuff in the pipeline that adds more
>> ID-less tags.  Or do we have a way to create warnings about that?
> I am unclear on how to make warnings with xslt.  You can make
> a listing of problems, but who would read it if the build
> completed successfully?  You can make errors and abort.

You can emit warnings to the command line or you can abort with an
error. I've opted for warnings + comments in the output in the styling
patch.

The biggest issue about errors and warnings is the fact that xslt does
not process files in a line-based way which makes it pretty much
impossible to give hints where the problem causing the warning is
located. Since everything is bound together via XML entities, you can't
even tell the source file.

I've worked around this by also emitting an HTML comment to the output
so that I can find a somewhat unique string next to it and the grep the
documentation sources for this string. It's a bit ugly but the best I
could come up with.

I'll repost a rebased version of the styling patch in a minute.

Regards,

Brar

  25. Re: doc: add missing "id" attributes to extension packaging page

    Brar Piening <brar@gmx.de> — 2023-01-10T06:08:08Z

    On 10.01.2023 at 06:28, Brar Piening wrote:
>
> I'll repost a rebased version of the styling patch in a minute.

After checking that there's no need for rebasing I'm reposting the
original patch here, to make cfbot pick it up as the latest one in a
somewhat screwed up thread mixing two patches (sorry for that - won't
happen again).

Althoug the patch is pretty compact you probably need some understanding
of both XSLT and CSS to understand and judge the changes it introduces.

It pretty much does two things:

 1. Make html ids discoverable in the browser by adding a link with a
    hover effect to items sections and varlistentries that have an id.
    Hover one of the psql options and click on the hash mark in [1] to
    see the behavior.
 2. Emit a warning to the command line and a comment to the HTML output
    when the docs build runs into a section without id or a varlistentry
    without id where at least one entry in the varlist already has an id.

The original mail for the patch is at [2], the commitfest entry is at
[3] and the initial discussion leading to this patch starts at [4].

Regards,

Brar

[1] https://pgdocs.piening.info/app-psql.html#APP-PSQL-OPTION-PORT

[2]
https://www.postgresql.org/message-id/d6695820-af71-5e84-58b0-ff9f1c189603%40gmx.de

[3] https://commitfest.postgresql.org/41/4042/

[4]
https://www.postgresql.org/message-id/4364ab38-a475-a1fc-b104-ecd6c72010d0%40enterprisedb.com

  26. Re: doc: add missing "id" attributes to extension packaging page

    Peter Eisentraut <peter.eisentraut@enterprisedb.com> — 2023-01-11T14:22:43Z

    On 09.01.23 21:18, Tom Lane wrote:
> * AFAIK our practice is to use "-" never "_" in XML ID attributes.
> You weren't very consistent about that even within this patch, and
> the overall effect would have been to have no standard about that
> at all, which doesn't seem great.  I changed them all to "-".

In the olden says, "_" was invalid in ID attribute values.  This is no 
longer the case.  But of course it's good to stay consistent with 
existing practice where reasonable.

  27. Re: doc: add missing "id" attributes to extension packaging page

    Karl O. Pinc <kop@karlpinc.com> — 2023-01-16T00:01:50Z

    Hi Brar,

Here's my first review of the make_html_ids_discoverable.patch.


Overall:

To start with, I'd like to say I like the goal and how everything
works when the patch is applied.  I was a bit skeptical, thinking that
the whole thing was going to be distracting when reading the docs or
otherwise odd, but it really works.

The thought comes to mind to take accessibility into account.  I am
unclear on what that would mean in this case.


Regards CSS:

The CSS looks good.  Also, it passes the W3C CSS validator (
https://jigsaw.w3.org/css-validator/) for CSS level 3.


Regards XSLT:

I believe the XSLT needs work.

Without really knowing the style of PG's XSLT, I'm going to suggest
putting the comment
<!-- Make HTML ids discoverable (by hover) by adding links to the ids -->
above the new XSLT.  Precede the comment with 2 blank lines and follow
it by one blank line.  The idea is to make the code you're adding an
identifiable "separate section".

I have a question, it is the docbook html stylesheets for XML that are
being overridden?  It looks like it, and what else would it be, but....
(Sorry, I'm a bit stale on this stuff.)

I believe that overriding the XSLT by copying the original and making
modifications is the "wrong way" (TM).  I think that the right way is
to declare a xsl:default-mode somewhere in the stylesheets.  There
does not seem to be one, so the default mode for PG doc processing
could be something like "postgres-mode".  And I'd expect it to be
declared somewhere right at the top of the xml hierarchy.  (I forget
what that is, probably something like "document" or "book" or
something.)  Then, you'd write your for the <xsl:template
match="varlistentry"> and <xsl:template name="section.heading"> with a
mode of "postgres-mode", and have your templates call the "traditional
default", "modeless", templates.  That way your not copying and
patching upstream code, but are instead, in effect, calling it as a
subroutine.

This should work especially well since, I think, you're just adding
new output to what the upstream templates do.  You're not trying to
insert new output into the middle of the stock output or otherwise
modify the stock output.

You're adding only about 6 lines of XSLT to the upstream templates,
and copying 100+ lines.  There must be a better way.

See: https://www.w3.org/TR/xslt-30/#modes

I've never tried this, although I do recall doing something or another
with modes in the past.  And I've not gone so far as to figure out
(again?) how to call a "modeless template", so you can invoke the
original, upstream, templates.  And setting a default mode seems like
something of a "big hammer", so should probably be checked over by
someone who's more steeped in Postgres docs than myself.  (Like a
committer. :) But I believe it is the way to go.  To make it work
you'll need to figure out the XSLT mode selection process and make
sure that it first selects the "postgres-mode", and then the modeless
templates, and also still works right when a template calls another
and explicitly sets a mode.


Regards visual presentation:

Here's the fun question, what to have "appear" when a section or
varlistentry with an id is hovered over?

I kind of like your choice of the "#" character as the screen element
that becomes visible when you hover over a section or varlistentry
with the mouse, which then shows you the URL of the thing over which
you are hovering.  That's what's in the patch now.  But I wonder why
you chose it.  Is there some sort of standard?  I've seen the anchor
Unicode character before, (⚓) \u2693.  I don't find it particularly
helpful.  The "place of interest" symbol,(⌘) \u2318, might be nice.
There is (◎), \u25ce, the "bullseye" symbol.  There is the link symbol
(🔗), \U0001f517.  Like the anchor, it has generally been confusing
when I come across it.  The link symbol with the "text variant form",
(🔗︎) \U0001f517\ufe0e, looks more link an actual chain and is somewhat
better.  (The opposite of "text variant" is "emoji variant".)  There
is also the paperclip, (📎) \U0001f4ce.  And the paperclip text
variant, (📎︎) \U0001f4ce\ufe0e.

Of all the Unicode choices above, I think I prefer the last.  The text
paperclip.  📎︎

(The hex representations above are Python 3 string literals.  So
print("\U0001f4ce\ufe0e") prints the text paperclip.)

The actual representation of the various Unicode characters is going
to depend on the font.  (Things change for me when looked at in an 
email v.s. in a browser.)

The question I have is should we use "Unicode icons" at all or is
plain old UTF-8 good enough for us (because it was good enough for our
ancestors ;) ?  Of course an image is also possible....  For now I'm
not going to advocate for a change from the "#" already used in the
patch.

Regards,

Karl <kop@karlpinc.com>
Free Software:  "You don't pay back, you pay forward."
                 -- Robert A. Heinlein

  28. Re: doc: add missing "id" attributes to extension packaging page

    Karl O. Pinc <kop@karlpinc.com> — 2023-01-16T00:28:24Z

    On Sun, 15 Jan 2023 18:01:50 -0600
"Karl O. Pinc" <kop@karlpinc.com> wrote:

> Regards XSLT:
> 
> I believe the XSLT needs work.

I also think that the XSLT should error and halt
when there's no id (in the expected places).  
Instead of just giving a warning and keeping going. Otherwise
they'll constantly be ignored warnings and periodically
there will have to be patches to supply missing ids.

To solve the "which id is missing where so I can fix it"
problem, I propose the error text show the chapter title,
all the enclosing sub-section titles, and any previous existing
varlistentry ids occurring before the tag with the
missing attribute.  At least for varlistentry-s.  For
sections you could do chapter and enclosing sub-section
titles and the title of the section with the problem.
That should be enough for an author to find the place
in the source sgml that needs fixing.

Maybe, possibly, you can see how this is done by looking
at whatever XSLT there is that automatically generates
ids for sections without ids, so that the table of contents
have something to link to.  In any case, XSLT is really
good at "looking at" parent/enclosing XML, so producing
a useful error message shouldn't be _that_ hard.  I've
definitely done this sort of thing before so I can tell you
it's readily doable.

Regards,

Karl <kop@karlpinc.com>
Free Software:  "You don't pay back, you pay forward."
                 -- Robert A. Heinlein

  29. Re: doc: add missing "id" attributes to extension packaging page

    Karl O. Pinc <kop@karlpinc.com> — 2023-01-16T17:14:35Z

    On Sun, 15 Jan 2023 18:01:50 -0600
"Karl O. Pinc" <kop@karlpinc.com> wrote:

> Regards XSLT:
> 
> I believe the XSLT needs work.

> I believe that overriding the XSLT by copying the original and making
> modifications is the "wrong way" (TM).  I think that the right way is
> to declare a xsl:default-mode somewhere in the stylesheets.  There
> does not seem to be one, so the default mode for PG doc processing
> could be something like "postgres-mode".  And I'd expect it to be
> declared somewhere right at the top of the xml hierarchy.  (I forget
> what that is, probably something like "document" or "book" or
> something.)  Then, you'd write your for the <xsl:template
> match="varlistentry"> and <xsl:template name="section.heading"> with
> a mode of "postgres-mode", and have your templates call the
> "traditional default", "modeless", templates.  That way your not
> copying and patching upstream code, but are instead, in effect,
> calling it as a subroutine.
> 
> This should work especially well since, I think, you're just adding
> new output to what the upstream templates do.  You're not trying to
> insert new output into the middle of the stock output or otherwise
> modify the stock output.
> 
> You're adding only about 6 lines of XSLT to the upstream templates,
> and copying 100+ lines.  There must be a better way.
> 
> See: https://www.w3.org/TR/xslt-30/#modes
> 
> I've never tried this, although I do recall doing something or another
> with modes in the past.  And I've not gone so far as to figure out
> (again?) how to call a "modeless template", so you can invoke the
> original, upstream, templates.  And setting a default mode seems like
> something of a "big hammer", so should probably be checked over by
> someone who's more steeped in Postgres docs than myself.  (Like a
> committer. :) But I believe it is the way to go.  To make it work
> you'll need to figure out the XSLT mode selection process and make
> sure that it first selects the "postgres-mode", and then the modeless
> templates, and also still works right when a template calls another
> and explicitly sets a mode.

Drat.  I forgot.  We're using xsltproc which is XSLT 1.0.

So this is the relevant spec:

https://www.w3.org/TR/1999/REC-xslt-19991116#modes

In XSLT 1.0 there is no xml:default-mode.  So I _think_ what you do then
is modify the built-in template rules so that the (default) template
(mode='') is invoked when there is no 'postgres-mode' version of the
template, but otherwise the 'postgres-mode' version of the template
is invoked.  Your 'postgres-mode' templates will xsl:call-template
the default template, adding whatever they want to the output produced
by the default template.

See: https://www.w3.org/TR/1999/REC-xslt-19991116#built-in-rule

Regards,

Karl <kop@karlpinc.com>
Free Software:  "You don't pay back, you pay forward."
                 -- Robert A. Heinlein

  30. Re: doc: add missing "id" attributes to extension packaging page

    Karl O. Pinc <kop@karlpinc.com> — 2023-01-17T01:05:50Z

    On Mon, 16 Jan 2023 11:14:35 -0600
"Karl O. Pinc" <kop@karlpinc.com> wrote:

> On Sun, 15 Jan 2023 18:01:50 -0600
> "Karl O. Pinc" <kop@karlpinc.com> wrote:
> 
> > Regards XSLT:
> > 
> > I believe the XSLT needs work.  

> In XSLT 1.0 there is no xml:default-mode.  So I _think_ what you do
> then is modify the built-in template rules so that the (default)
> template (mode='') is invoked when there is no 'postgres-mode'
> version of the template, but otherwise the 'postgres-mode' version of
> the template is invoked.  Your 'postgres-mode' templates will
> xsl:call-template the default template, adding whatever they want to
> the output produced by the default template.

Or maybe the right way is to set a mode at the very top,
the first apply-templates call, and not mess with the
built-in templates at all.  (You'd write your own
"postgres-mode" templates the same way, to "wrap"
and call the default templates.)

Think of the mode as an implicit argument that's preserved and
passed down through each template invocation without having to
be explicitly specified by the calling code.

Regards

Karl <kop@karlpinc.com>
Free Software:  "You don't pay back, you pay forward."
                 -- Robert A. Heinlein

  31. Re: doc: add missing "id" attributes to extension packaging page

    Brar Piening <brar@gmx.de> — 2023-01-17T05:57:23Z

    On 17.01.2023 at 02:05, Karl O. Pinc wrote:
> Or maybe the right way is to set a mode at the very top,
> the first apply-templates call, and not mess with the
> built-in templates at all.  (You'd write your own
> "postgres-mode" templates the same way, to "wrap"
> and call the default templates.)
>
> Think of the mode as an implicit argument that's preserved and
> passed down through each template invocation without having to
> be explicitly specified by the calling code.

I think the document you're missing is [1].

There are multiple ways to customize DocBook XSL output and it sounds
like you want me to write a customization layer which I didn't do
because there is precedent that the typical "way to do it" (TM) in the
PostgreSQL project is [2].

Regards,

Brar

[1] http://www.sagehill.net/docbookxsl/CustomizingPart.html
[2] http://www.sagehill.net/docbookxsl/ReplaceTemplate.html

  32. Re: doc: add missing "id" attributes to extension packaging page

    Karl O. Pinc <kop@karlpinc.com> — 2023-01-17T13:12:42Z

    On Tue, 17 Jan 2023 06:57:23 +0100
Brar Piening <brar@gmx.de> wrote:

> On 17.01.2023 at 02:05, Karl O. Pinc wrote:
> > Or maybe the right way is to set a mode at the very top,
> > the first apply-templates call, and not mess with the
> > built-in templates at all.  (You'd write your own
> > "postgres-mode" templates the same way, to "wrap"
> > and call the default templates.)
> >
> > Think of the mode as an implicit argument that's preserved and
> > passed down through each template invocation without having to
> > be explicitly specified by the calling code.  
> 
> I think the document you're missing is [1].
> 
> There are multiple ways to customize DocBook XSL output and it sounds
> like you want me to write a customization layer which I didn't do
> because there is precedent that the typical "way to do it" (TM) in the
> PostgreSQL project is [2].
> 
> Regards,
> 
> Brar
> 
> [1] http://www.sagehill.net/docbookxsl/CustomizingPart.html
> [2] http://www.sagehill.net/docbookxsl/ReplaceTemplate.html
> 

Sagehill is normally vary good.  But in this case [2] does not
apply.  Or rather it applies but it is overkill because you
do not want to replace what a template is producing.  You
want to add to what a template is producing.  So you want to
wrap the template, with your new code adding output before/
after what the original produces.

[1] does not contain this technique.

If you're not willing to try I am willing to see if I can
produce an example to work from.  My XSLT is starting to
come back.

Regards,

Karl <kop@karlpinc.com>
Free Software:  "You don't pay back, you pay forward."
                 -- Robert A. Heinlein

  33. Re: doc: add missing "id" attributes to extension packaging page

    Brar Piening <brar@gmx.de> — 2023-01-17T18:13:38Z

    On 17.01.2023 at 14:12, Karl O. Pinc wrote:
> If you're not willing to try I am willing to see if I can
> produce an example to work from.  My XSLT is starting to
> come back.

I'm certainly willing to try but I'd appreciate an example in any case.

My XSLT skills are mostly learning by doing combined with trial and error.

Regards,

Brar

  34. Re: doc: add missing "id" attributes to extension packaging page

    Karl O. Pinc <kop@karlpinc.com> — 2023-01-17T22:43:13Z

    On Tue, 17 Jan 2023 19:13:38 +0100
Brar Piening <brar@gmx.de> wrote:

> On 17.01.2023 at 14:12, Karl O. Pinc wrote:
> > If you're not willing to try I am willing to see if I can
> > produce an example to work from.  My XSLT is starting to
> > come back.  
> 
> I'm certainly willing to try but I'd appreciate an example in any
> case.

It's good you asked.  After poking about the XSLT 1.0 spec to refresh
my memory I abandoned the "mode" approach entirely.  The real "right
way" is to use the import mechanism.

I've attached a patch that "wraps" the section.heading template
and adds extra stuff to the HTML output generated by the stock
template.  (example_section_heading_override.patch)

There's a bug.  All that goes into the html is a comment, not
a hoverable link.  But the technique is clear.

On my system (Debian 11, bullseye) I found the URI to import
by looking at:
/usr/share/xml/docbook/stylesheet/docbook-xsl/catalog.xml
(Which is probably the right place to look.)
Ultimately, that file is findable via: /etc/xml/catalog
The "best way" on
Debian is: /usr/share/doc/docbook-xsl/README.gz
In other words, the README that comes with the style sheets.

Supposedly, the href=URLs are really URIs and will be good
no matter what/when.  The XSLT processor should know to look
at the system catalogs and read the imported style sheet
from the local file system.

It might be useful to add --nonet to the xsltproc invocation(s)
in the Makefile(s).  Just in case; to keep from retrieving
stylesheets from the net.  (If the option is not already there.
I didn't look.)

If this is the first time that PG uses the XSLT import mechanism
I imagine that "things could go wrong" depending on what sort
of system is being used to build the docs.  I'm not worried,
but it is something to note for the committer.

> My XSLT skills are mostly learning by doing combined with trial and
> error.

I think of XSLT as a functional programming language.  Recursion is
a big deal, and data directed programming can be a powerful technique
because XSLT is so good with data structures.
(https://mitp-content-server.mit.edu/books/content/sectbyfn/books_pres_0/6515/sicp.zip/full-text/book/book-Z-H-17.html#%_sec_2.4.3)

Regards,

Karl <kop@karlpinc.com>
Free Software:  "You don't pay back, you pay forward."
                 -- Robert A. Heinlein

  35. Re: doc: add missing "id" attributes to extension packaging page

    Brar Piening <brar@gmx.de> — 2023-01-18T05:50:40Z

    On 17.01.2023 at 23:43, Karl O. Pinc wrote:
> It's good you asked.  After poking about the XSLT 1.0 spec to refresh
> my memory I abandoned the "mode" approach entirely.  The real "right
> way" is to use the import mechanism.
>
> I've attached a patch that "wraps" the section.heading template
> and adds extra stuff to the HTML output generated by the stock
> template.  (example_section_heading_override.patch)

Thanks!

I'll give it a proper look this weekend.

Regards,

Brar

  36. Re: doc: add missing "id" attributes to extension packaging page

    Karl O. Pinc <kop@karlpinc.com> — 2023-01-19T04:05:33Z

    On Tue, 17 Jan 2023 16:43:13 -0600
"Karl O. Pinc" <kop@karlpinc.com> wrote:

> It might be useful to add --nonet to the xsltproc invocation(s)
> in the Makefile(s).  Just in case; to keep from retrieving
> stylesheets from the net.  (If the option is not already there.
> I didn't look.)
> 
> If this is the first time that PG uses the XSLT import mechanism
> I imagine that "things could go wrong" depending on what sort
> of system is being used to build the docs.  I'm not worried,
> but it is something to note for the committer.

Looks like doc/src/sgml/stylesheet-fo.xsl already uses
xsl:import, although it is unclear to me whether the import
is applied.

Regards,

Karl <kop@karlpinc.com>
Free Software:  "You don't pay back, you pay forward."
                 -- Robert A. Heinlein

  37. Re: doc: add missing "id" attributes to extension packaging page

    Brar Piening <brar@gmx.de> — 2023-01-26T20:48:54Z

    On 18.01.2023 at 06:50, Brar Piening wrote:

> I'll give it a proper look this weekend.

It turns out I didn't get a round tuit.

... and I'm afraid I probably will not be able to work on this until
mid/end February so we'll have to move this to the next commitfest until
somebody wants to take it over and push it through.

  38. Re: doc: add missing "id" attributes to extension packaging page

    Andres Freund <andres@anarazel.de> — 2023-02-14T20:13:18Z

    Hi,

On 2023-01-26 21:48:54 +0100, Brar Piening wrote:
> On 18.01.2023 at 06:50, Brar Piening wrote:
> 
> > I'll give it a proper look this weekend.
> 
> It turns out I didn't get a round tuit.
> 
> ... and I'm afraid I probably will not be able to work on this until
> mid/end February so we'll have to move this to the next commitfest until
> somebody wants to take it over and push it through.

A small note: As-is this fails on CI, because we don't allow network access
during the docs build anymore (since it always fails these days):

https://cirrus-ci.com/task/5474029402849280?logs=docs_build#L297

[17:02:03.114] time make -s -j${BUILD_JOBS} -C doc
[17:02:04.092] I/O error : Attempt to load network entity http://cdn.docbook.org/release/xsl/current/html/sections.xsl
[17:02:04.092] warning: failed to load external entity "http://cdn.docbook.org/release/xsl/current/html/sections.xsl"
[17:02:04.092] compilation error: file stylesheet-html-common.xsl line 17 element import
[17:02:04.092] xsl:import : unable to load http://cdn.docbook.org/release/xsl/current/html/sections.xsl

I think this is just due to the common URL in docbook packages being
http://docbook.sourceforge.net/release/xsl/current/*
Because of that the docbook catalog matching logic won't work for that file:

E.g. I have the following in /etc/xml/docbook-xsl.xml, on debian unstable:
<delegateURI uriStartString="http://docbook.sourceforge.net/release/xsl/" catalog="file:///usr/share/xml/docbook/stylesheet/docbook-xsl/catalog.xml"/>

As all our other references use the sourceforge address, this should too.

Greetings,

Andres Freund

  39. Re: doc: add missing "id" attributes to extension packaging page

    Karl O. Pinc <kop@karlpinc.com> — 2023-02-15T19:34:37Z

    On Tue, 14 Feb 2023 12:13:18 -0800
Andres Freund <andres@anarazel.de> wrote:

> A small note: As-is this fails on CI, because we don't allow network
> access during the docs build anymore (since it always fails these
> days):
> 
> https://cirrus-ci.com/task/5474029402849280?logs=docs_build#L297
> 
> [17:02:03.114] time make -s -j${BUILD_JOBS} -C doc
> [17:02:04.092] I/O error : Attempt to load network entity
> http://cdn.docbook.org/release/xsl/current/html/sections.xsl
> [17:02:04.092] warning: failed to load external entity
> "http://cdn.docbook.org/release/xsl/current/html/sections.xsl"
> [17:02:04.092] compilation error: file stylesheet-html-common.xsl
> line 17 element import [17:02:04.092] xsl:import : unable to load
> http://cdn.docbook.org/release/xsl/current/html/sections.xsl

This makes me think that it would be useful to add --nonet to the
xsltproc invocations.  That would catch this error before it goes to
CI.

> I think this is just due to the common URL in docbook packages being
> http://docbook.sourceforge.net/release/xsl/current/*
> Because of that the docbook catalog matching logic won't work for
> that file:
> 
> E.g. I have the following in /etc/xml/docbook-xsl.xml, on debian
> unstable: <delegateURI
> uriStartString="http://docbook.sourceforge.net/release/xsl/"
> catalog="file:///usr/share/xml/docbook/stylesheet/docbook-xsl/catalog.xml"/>
> 
> As all our other references use the sourceforge address, this should
> too.

Agreed.

I'm also noticing that the existing xsl:import-s all import entire
docbook stylesheets.  It does not hurt to do this; the output is
unaffected, although I can't say what it means for build performance.
It does keep it simple.  Only one import is needed no matter which
templates we use the import mechanism to extend.  And by importing
"everything" there's no concern about any (unlikely) changes to
the the "internals" of the catalog.

Should we import only what we need or all of docbook?  I don't know.

Regards,

Karl <kop@karlpinc.com>
Free Software:  "You don't pay back, you pay forward."
                 -- Robert A. Heinlein

  40. Re: doc: add missing "id" attributes to extension packaging page

    Andres Freund <andres@anarazel.de> — 2023-02-15T20:49:47Z

    Hi,

On 2023-02-15 13:34:37 -0600, Karl O. Pinc wrote:
> This makes me think that it would be useful to add --nonet to the
> xsltproc invocations.  That would catch this error before it goes to
> CI.

We are doing that now :)

commit 969509c3f2e3b4c32dcf264f9d642b5ef01319f3
Author: Tom Lane <tgl@sss.pgh.pa.us>
Date:   2023-02-08 17:15:23 -0500
 
    Stop recommending auto-download of DTD files, and indeed disable it.



> I'm also noticing that the existing xsl:import-s all import entire
> docbook stylesheets.  It does not hurt to do this; the output is
> unaffected, although I can't say what it means for build performance.
> It does keep it simple.  Only one import is needed no matter which
> templates we use the import mechanism to extend.  And by importing
> "everything" there's no concern about any (unlikely) changes to
> the the "internals" of the catalog.
> 
> Should we import only what we need or all of docbook?  I don't know.

It couldn't hurt to check if performance improves when you avoid doing so. I
suspect it won't make much of a difference, because the time is actually spent
evaluating xslt rather than parsing it.

Greetings,

Andres Freund

  41. Re: doc: add missing "id" attributes to extension packaging page

    Gregory Stark (as CFM) <stark.cfm@gmail.com> — 2023-03-20T18:47:24Z

    On Thu, 26 Jan 2023 at 15:55, Brar Piening <brar@gmx.de> wrote:
>
> On 18.01.2023 at 06:50, Brar Piening wrote:
>
> > I'll give it a proper look this weekend.
>
> It turns out I didn't get a round tuit.
>
> ... and I'm afraid I probably will not be able to work on this until
> mid/end February so we'll have to move this to the next commitfest until
> somebody wants to take it over and push it through.


Looks like a lot of good work was happening on this patch right up
until mid-February. Is there a lot of work left? Do you think you'll
have a chance to wrap it up this commitfest for this release?


-- 
Gregory Stark
As Commitfest Manager

  42. Re: doc: add missing "id" attributes to extension packaging page

    Brar Piening <brar@gmx.de> — 2023-03-21T13:57:28Z

    On 20.03.2023 at 19:47, Gregory Stark (as CFM) wrote:
> Looks like a lot of good work was happening on this patch right up
> until mid-February. Is there a lot of work left? Do you think you'll
> have a chance to wrap it up this commitfest for this release?

Thanks for the ping.

I had another look this morning and I think I can probably finish this
by the end of the week.

  43. Re: doc: add missing "id" attributes to extension packaging page

    Brar Piening <brar@gmx.de> — 2023-03-21T22:16:25Z

    On 17.01.2023 at 23:43, Karl O. Pinc wrote:
> It's good you asked.  After poking about the XSLT 1.0 spec to refresh
> my memory I abandoned the "mode" approach entirely.  The real "right
> way" is to use the import mechanism.

It actually is not.

After quite some time trying to figure out why things don't work as
intended, I ended up reading the XSLT 1.0 spec.

As the name already suggests, <xsl:apply-imports> is closely related to
<xsl:apply-templates> with the difference that it *applies* a *template
rule* from an imported style sheet instead of applying a template rule
from the current style sheet
(https://www.w3.org/TR/1999/REC-xslt-19991116#apply-imports). What it
does not do is *calling* a *named template*
(https://www.w3.org/TR/1999/REC-xslt-19991116#named-templates).

What this basically means is that in XSLT 1.0 you can use
<xsl:apply-imports> to override template rules (<xsl:template
match="this-pattern-inside-match-makes-it-a-template-rule">) but you
cannot use it to override named templates (<xsl:template
name="this-id-inside-name-makes-it-a-named-template">). If you want to
override named templates you basically have to duplicate and change them.

While there are mechanisms to call overriden named templates in XSLT 3,
this is out of scope here, since we're bound to XSLT 1.0

As a consequence, there was little I could change in the initial patch
to avoid the code duplication and all attempts to do so, ultimately led
to even longer and more complex code without really reducing the amount
of duplication.

The <xsl:apply-imports> approach actually does work in the varlistentry
case, although this doesn't really change a lot regarding the length of
the patch (it's a bit nicer though since in this case it really avoids
duplication). I've also taken the advice to terminate the build and
print the xpath if a required id is missing.

The attached patch is my best-effort approach to implement discoverable
links.

Best regards,

Brar

  44. Re: doc: add missing "id" attributes to extension packaging page

    Karl O. Pinc <kop@karlpinc.com> — 2023-03-23T03:09:19Z

    On Tue, 21 Mar 2023 23:16:25 +0100
Brar Piening <brar@gmx.de> wrote:

> On 17.01.2023 at 23:43, Karl O. Pinc wrote:
> > It's good you asked.  After poking about the XSLT 1.0 spec to
> > refresh my memory I abandoned the "mode" approach entirely.  The
> > real "right way" is to use the import mechanism.  

> After quite some time trying to figure out why things don't work as
> intended, I ended up reading the XSLT 1.0 spec.
> 
> As the name already suggests, <xsl:apply-imports> is closely related
> to <xsl:apply-templates> with the difference that it *applies* a
> *template rule* from an imported style sheet instead of applying a
> template rule from the current style sheet
> (https://www.w3.org/TR/1999/REC-xslt-19991116#apply-imports). What it
> does not do is *calling* a *named template*
> (https://www.w3.org/TR/1999/REC-xslt-19991116#named-templates).
> 
> What this basically means is that in XSLT 1.0 you can use
> <xsl:apply-imports> to override template rules (<xsl:template
> match="this-pattern-inside-match-makes-it-a-template-rule">) but you
> cannot use it to override named templates (<xsl:template
> name="this-id-inside-name-makes-it-a-named-template">). If you want to
> override named templates you basically have to duplicate and change
> them.
> 
> While there are mechanisms to call overriden named templates in XSLT
> 3, this is out of scope here, since we're bound to XSLT 1.0

(It was reassuring to see you take the steps above; I once did exactly
the same with and had the same excitements and disappointments.  I
feel validated.  ;-)

(One of my disappointments is that xsltproc supports only XSLT 1.0,
and nothing later.  IIRC, apparently one big reason is not the amount
work needed to develop the program but the work required to develop a
test suite to validate conformance.)

> As a consequence, there was little I could change in the initial patch
> to avoid the code duplication and all attempts to do so, ultimately
> led to even longer and more complex code without really reducing the
> amount of duplication.

You're quite right.  I clearly didn't have my XSLT turned on.  Importing
only works when templates are matched, not called by name.

Sorry for the extra work I've put you through.

> The <xsl:apply-imports> approach actually does work in the
> varlistentry case, although this doesn't really change a lot
> regarding the length of the patch (it's a bit nicer though since in
> this case it really avoids duplication).


You've put in a lot of good work.  I'm attaching 2 patches
with only minor changes.


001-add-needed-ids_v1.patch

This separates out the addition of ids from the XSLT changes, just
to keep things tidy.  Content is from your patch.


002-make_html_ids_discoverable_v4.patch

I changed the linked text, the #, so that the leading space
is not linked.  This is arguable, as the extra space makes
it easier to put the mouse on the region.  But it seems
tidy.

I've tided up so the lines are no longer than 80 chars.

> I've also taken the advice
> to terminate the build and print the xpath if a required id is
> missing.

This looks awesome.  I love the xpath!  I've changed the format of the
error message.  What do you think?  (Try it out by _not_ applying
001-add-needed-ids_v1.patch.)

Also, the error message now has leading and trailing newlines to make
it stand out.  I'm normally against this sort of thing but thought I'd
add it anyway for others to review.

I'm ready to send these on to a committer but if you don't
like what I did please send more patches for me to review.


Outstanding questions (for committer?):

The 002-make_html_ids_discoverable_v4.patch generates xhtml <h1>,
<h2>, etc. attributes using a XSLT <element> element with a
"namespace" attribute.  I'm unclear on the relationship PG has with
xhtml and namespaces.  Looks right to me, since the generated html has
the same namespace name appearing in the xmlns attribute of the html
tag, but somebody who knows more than me might want to review this.

Using the namespace attribute does not seem to have affected the
generated html, as far as my random sampling of output can tell.


What character should be used to represent a link anchor?

I've left your choice of "#" in the patch.

If we go to unicode, My preference is the text paperclip  📎︎

Here's a table of all the choices I came up with, there may be others
that are suitable.  (The hex representations are Python 3 string
literals.  So print("\U0001f4ce\ufe0e") prints the text paperclip.)

Hash mark                  #   (ASCII, used in the patch, \u0023)
Anchor                     ⚓  \u2693
Place of interest          ⌘   \u2318
Bullseye                   ◎   \u25ce
Link (emoji variant)       🔗  \U0001f517
Link (text variant)        🔗︎   \U0001f517\ufe0e
Paperclip (emoji variant)  📎  \U0001f4ce
Paperclip (text variant)   📎︎  \U0001f4ce\ufe0e


Regards,

Karl <kop@karlpinc.com>
Free Software:  "You don't pay back, you pay forward."
                 -- Robert A. Heinlein

  45. Re: doc: add missing "id" attributes to extension packaging page

    Brar Piening <brar@gmx.de> — 2023-03-23T07:24:48Z

    On 23.03.2023 at 04:09, Karl O. Pinc wrote:
> You're quite right.  I clearly didn't have my XSLT turned on.  Importing
> only works when templates are matched, not called by name.
>
> Sorry for the extra work I've put you through.

No problem. As always I've learnt something which may help me in the future.


> You've put in a lot of good work.  I'm attaching 2 patches
> with only minor changes.

Thanks. When comparing things I also realized that I had accidentally
created a reversed patch. Thanks for fixing this.


> 001-add-needed-ids_v1.patch
>
> This separates out the addition of ids from the XSLT changes, just
> to keep things tidy.  Content is from your patch.

+1


> 002-make_html_ids_discoverable_v4.patch
>
> I changed the linked text, the #, so that the leading space
> is not linked.  This is arguable, as the extra space makes
> it easier to put the mouse on the region.  But it seems
> tidy.

I tend to prefer a slightly bigger mouseover-region but I don't really mind.


> I've tided up so the lines are no longer than 80 chars.

+1


> This looks awesome.  I love the xpath!  I've changed the format of the
> error message.  What do you think?  (Try it out by _not_ applying
> 001-add-needed-ids_v1.patch.)
>
> Also, the error message now has leading and trailing newlines to make
> it stand out.  I'm normally against this sort of thing but thought I'd
> add it anyway for others to review.

+1


> I'm ready to send these on to a committer but if you don't
> like what I did please send more patches for me to review.

I like it and think it's ready for commiter.


> Outstanding questions (for committer?):
>
> The 002-make_html_ids_discoverable_v4.patch generates xhtml <h1>,
> <h2>, etc. attributes using a XSLT <element> element with a
> "namespace" attribute.

I'm not sure I follow. I cannot see any namespacing weirdness in my output.

Are you using the v1.79.2 styleshhets?


> What character should be used to represent a link anchor?

It's not the first time this is coming up. See my response in the old
thread:
https://www.postgresql.org/message-id/e50193ea-ca5c-e178-026a-f3fd8942252d%40gmx.de

Personally I'd advise to stick with ASCII for now.

In any case changing the symbol at some point would be a very minor
effort if we deem it necessary.

Maybe this could be part of some general overhaul of the visual
apperance and website styling by a person with more talent for this than
I have.

Regards,

Brar

  46. Re: doc: add missing "id" attributes to extension packaging page

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2023-03-23T09:35:55Z

    Thanks, Brar and Karl, I hope we can get this done soon.

As with the <simplelist> patch, we'll need to patch the CSS used in the
website for the docs too, as that's the most important place where docs
are visited.  See this commit for an example:
https://git.postgresql.org/gitweb/?p=pgweb.git;a=commitdiff;h=0b89ea0fff28d29ed177c82a274144453e3c7f82

In order to test locally that your patched stylesheet works correctly,
you'd have to compile the docs with "make html STYLE=website" in the doc
subdir, and tweak one of the CSS files there (I think it's
docs-complete.css) so that it references your local copy instead of
fetching it from the website.

> diff --git a/doc/src/sgml/stylesheet.css b/doc/src/sgml/stylesheet.css
> index cc14efa1ca..15bcc95d41 100644
> --- a/doc/src/sgml/stylesheet.css
> +++ b/doc/src/sgml/stylesheet.css
> @@ -169,3 +169,13 @@ acronym		{ font-style: inherit; }
>      width: 75%;
>    }
>  }
> +
> +/* Links to ids of headers and definition terms */
> +a.id_link {
> +        color: inherit;
> +        visibility: hidden;
> +}
> +
> +*:hover > a.id_link {
> +        visibility: visible;
> +}

I'm not clear on what exactly becomes visible when one hovers over what.
Can you please share a screenshot?

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/

  47. Re: doc: add missing "id" attributes to extension packaging page

    Brar Piening <brar@gmx.de> — 2023-03-23T10:13:49Z

    On 23.03.2023 at 10:35, Alvaro Herrera wrote:
> As with the <simplelist> patch, we'll need to patch the CSS used in the
> website for the docs too, as that's the most important place where docs
> are visited.  See this commit for an example:
> https://git.postgresql.org/gitweb/?p=pgweb.git;a=commitdiff;h=0b89ea0fff28d29ed177c82a274144453e3c7f82
>
> In order to test locally that your patched stylesheet works correctly,
> you'd have to compile the docs with "make html STYLE=website" in the doc
> subdir, and tweak one of the CSS files there (I think it's
> docs-complete.css) so that it references your local copy instead of
> fetching it from the website.

Thanks I'll take care of this tonight.


> I'm not clear on what exactly becomes visible when one hovers over what.
> Can you please share a screenshot?

I could, but since hover effects don't really come across in screenshots
I've posted a build of the docs including the patch to
https://pgdocs.piening.info

See https://pgdocs.piening.info/app-psql.html as an example.

  48. Re: doc: add missing "id" attributes to extension packaging page

    Karl O. Pinc <kop@karlpinc.com> — 2023-03-23T12:51:44Z

    On Thu, 23 Mar 2023 08:24:48 +0100
Brar Piening <brar@gmx.de> wrote:

> On 23.03.2023 at 04:09, Karl O. Pinc wrote:

> > Sorry for the extra work I've put you through.  
> 
> No problem. As always I've learnt something which may help me in the
> future.

I don't know about you, but sadly, my brain eventually leaks. ;-)

> > I'm attaching 2 patches
> > with only minor changes.  

> > 001-add-needed-ids_v1.patch
> >
> > This separates out the addition of ids from the XSLT changes, just
> > to keep things tidy. 

> > 002-make_html_ids_discoverable_v4.patch
> >
> > I changed the linked text, the #, so that the leading space
> > is not linked.  This is arguable, as the extra space makes
> > it easier to put the mouse on the region.

> I tend to prefer a slightly bigger mouseover-region but I don't
> really mind.

I'm leaving it for the committer to review.

> I've changed the format of
> > the error message.  What do you think?  (Try it out by _not_
> > applying 001-add-needed-ids_v1.patch.)
> >
> > Also, the error message now has leading and trailing newlines to
> > make it stand out.

Including the error message/make output here, so everyone can see
easily.

--------------<snip>------------
/usr/bin/xsltproc --nonet --path . --stringparam pg.version '16devel'  stylesheet.xsl postgres-full.xml

Ids are required in order to provide the public HTML documentation with stable URLs for <varlistentry> element content; id missing at: /book[@id = 'postgres']/part[@id = 'appendixes']/appendix[@id = 'contrib']/sect1[@id = 'pgwalinspect']/sect2[@id = 'pgwalinspect-funcs']/variablelist
 
no result for postgres-full.xml
make: *** [Makefile:146: html-stamp] Error 10
--------------<snip>------------

> I like it and think it's ready for commiter.

I've marked it ready for the committer in the commitfest.

> > Outstanding questions (for committer?):
> >
> > The 002-make_html_ids_discoverable_v4.patch generates xhtml <h1>,
> > <h2>, etc. attributes using a XSLT <element> element with a
> > "namespace" attribute.  
> 
> I'm not sure I follow. I cannot see any namespacing weirdness in my
> output.

There's nothing weird in the output, it's all about how
you're generating it in the xslt with

 <xsl:element name="h$level" namespace="...

Output looks right to me.

> Are you using the v1.79.2 styleshhets?

Yes.  But I've got both the ones with namespaces and without
installed.

I've just never had to look at what PG is doing with namespaces
before.  What you've done looks right to me, but I'm pretty
clueless so somebody else should double check.]

> > What character should be used to represent a link anchor?  

> Personally I'd advise to stick with ASCII for now.

+1

Regards,

Karl <kop@karlpinc.com>
Free Software:  "You don't pay back, you pay forward."
                 -- Robert A. Heinlein

  49. Re: doc: add missing "id" attributes to extension packaging page

    Karl O. Pinc <kop@karlpinc.com> — 2023-03-23T13:02:07Z

    On Thu, 23 Mar 2023 10:35:55 +0100
Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:

> As with the <simplelist> patch, we'll need to patch the CSS used in
> the website for the docs too, as that's the most important place
> where docs are visited.  See this commit for an example:
> https://git.postgresql.org/gitweb/?p=pgweb.git;a=commitdiff;h=0b89ea0fff28d29ed177c82a274144453e3c7f82
> 
> In order to test locally that your patched stylesheet works correctly,
> you'd have to compile the docs with "make html STYLE=website" in the
> doc subdir, and tweak one of the CSS files there (I think it's
> docs-complete.css) so that it references your local copy instead of
> fetching it from the website.

I should have known to put the css in a separate patch.

> I'm not clear on what exactly becomes visible when one hovers over
> what. Can you please share a screenshot?

Attached are 2 screenshots.  I don't know why, but for
some reason you can't see the mouse pointer.

"#' shows up when the mouse is anywhere over the html heading element.

Regards,

Karl <kop@karlpinc.com>
Free Software:  "You don't pay back, you pay forward."
                 -- Robert A. Heinlein

  50. Re: doc: add missing "id" attributes to extension packaging page

    Brar Piening <brar@gmx.de> — 2023-03-23T19:08:52Z

    On 23.03.2023 at 10:35, Alvaro Herrera wrote:
> As with the <simplelist> patch, we'll need to patch the CSS used in the
> website for the docs too, as that's the most important place where docs
> are visited.

Ok, I've created and tested a patch for this too.

Since the need for ids is starting to grow again (ecb696527c added an id
to a varlistentry in doc/src/sgml/ref/create_subscription.sgml) I've
also amended the add-needed-ids patch once again so that the build does
not fail after applying the make_html_ids_discoverable patch.

I've also attached the (unchanged)  make_html_ids_discoverable patch for
convenience so this email now contains two patches for postgresql
(ending with .postgresql.patch) and one patch for pgweb (ending with
.pgweb.patch).

TBH I'm a bit afraid that people will immediately start complaining
about the failing docs builds after this got applied since it forces
them to add ids to all varlistenries in a variablelist if they add one,
which can be perceived as quite a burden (also committers and reviewers
will have to get used to always watch out for failing docs builds
because of this).

Since breaking the build on missing ids was an intentional decision we
can theoretically soften this by only issuing a warning or removing the
check for missing id's altogether but this would probably defeat the
purpose since it would lead to an increasing number of entries that lack
an id after a while.

Regards,

Brar

  51. Re: doc: add missing "id" attributes to extension packaging page

    Corey Huinker <corey.huinker@gmail.com> — 2023-03-23T19:40:45Z

    >
> TBH I'm a bit afraid that people will immediately start complaining
> about the failing docs builds after this got applied since it forces
> them to add ids to all varlistenries in a variablelist if they add one,
> which can be perceived as quite a burden (also committers and reviewers
> will have to get used to always watch out for failing docs builds
> because of this).
>

As a person who had to re-rebase a patch because I discovered that id tags
had been added to one of the files I was working on, I can confidently say
"don't worry". It wasn't that big of a deal, I wasn't even following this
thread at the time and I immediately figured out what had happened and what
was expected of me. So it isn't that much of an inconvenience. If there is
a negative consequence to this change, it would be that it might
incentivize patch writers to omit documentation completely at the early
stages. But I don't think that's a problem because people generally see a
lack of documentation as a clue that maybe the patch isn't ready to be
reviewed, and this change would only reinforce that litmus test.

I had suggested we do something like this a few years back (the ids, that
is. the idea that we could check for compliance was beyond my imagination
at the time), and I'm glad to see both finally happening.

While I can foresee people overlooking the docs build, such oversights
won't go long before being caught, and the fix is simple.  Now if we can
just get a threaded version of xlstproc to make the builds faster...

p.s. I'm "Team Paperclip" when it comes to the link hint, but let's get the
feature in first and worry about the right character later.

  52. Re: doc: add missing "id" attributes to extension packaging page

    Karl O. Pinc <kop@karlpinc.com> — 2023-03-23T22:31:23Z

    Hi Brar,

It occurs to me that I had not actually tested the
way the anchor is put only after the last term in a
varlistentry.  (The code looked obviously right
and should work if any varlistentry terms have anchors,
which they do, but....)

Have you tested this?  If not, catalog.sgml, the
DEPENDENCY_PARTITION_SEC term is a 2nd term and usable
for at test case.  But, at the moment there are no ids
for any of these varlistentries so you'd have to hack
them in.

Apologies for missing this.

Regards,

Karl <kop@karlpinc.com>
Free Software:  "You don't pay back, you pay forward."
                 -- Robert A. Heinlein

  53. Re: doc: add missing "id" attributes to extension packaging page

    Karl O. Pinc <kop@karlpinc.com> — 2023-03-23T22:55:30Z

    This is for the committer, as an FYI.

I cut out the <xsl:template name="section.heading"> portion
of the docbook XSLT and diffed it with the code for the
same template in the patch.  The diff looks like:

-- /tmp/sections.xsl	2023-03-22 13:00:33.432968357 -0500
+++ /tmp/make_html_ids_discoverable_v3.patch	2023-03-22 13:03:39.776930603 -0500
@@ -52,5 +52,8 @@
       </xsl:call-template>
     </xsl:if>
     <xsl:copy-of select="$title"/>
+    <xsl:call-template name="pg.id.link">
+      <xsl:with-param name="object" select="$section"/>
+    </xsl:call-template>
   </xsl:element>
 </xsl:template>

(So, this output would start with line 52 of the template,
not from the top of the stock sections.xsl file.)

However, I am not really familiar with exactly what flavor
of docbook, version, namespace-d or not, etc., that PG
uses.  So I could be diffing with the wrong thing.

Hope this helps and is not just noise.

Regards,

Karl <kop@karlpinc.com>
Free Software:  "You don't pay back, you pay forward."
                 -- Robert A. Heinlein

  54. Re: doc: add missing "id" attributes to extension packaging page

    Karl O. Pinc <kop@karlpinc.com> — 2023-03-24T04:09:25Z

    Hi Brar,

An observation:  The # that shows up when hovering
over section-level headings is styled as the
section-level heading is.  But the # that shows
up when hovering over varlistentrys has the default
text style.

This works for me.  It's nice to have the "section #"s
look like the section heading.  But the varlistentry's
terms are smaller than the normal font, and their
line width is less heavy than normal.  I'm not really
invested one way or the other, but I find it kind of
nice that the varlistentry's #s are easier to click
on and more noticable because they're slightly larger
than might be expected.

Regards,

Karl <kop@karlpinc.com>
Free Software:  "You don't pay back, you pay forward."
                 -- Robert A. Heinlein

  55. Re: doc: add missing "id" attributes to extension packaging page

    Brar Piening <brar@gmx.de> — 2023-03-24T05:05:06Z

    On 23.03.2023 at 23:31, Karl O. Pinc wrote:
> Hi Brar,
>
> It occurs to me that I had not actually tested the
> way the anchor is put only after the last term in a
> varlistentry.  (The code looked obviously right
> and should work if any varlistentry terms have anchors,
> which they do, but....)
>
> Have you tested this?

Yes, I have. See
https://pgdocs.piening.info/app-psql.html#APP-PSQL-META-COMMAND-DE for
an extreme case.

  56. Re: doc: add missing "id" attributes to extension packaging page

    Brar Piening <brar@gmx.de> — 2023-03-24T05:48:17Z

    On 24.03.2023 at 05:09, Karl O. Pinc wrote:
> Hi Brar,
>
> An observation:  The # that shows up when hovering
> over section-level headings is styled as the
> section-level heading is.  But the # that shows
> up when hovering over varlistentrys has the default
> text style.
>
> This works for me.  It's nice to have the "section #"s
> look like the section heading.  But the varlistentry's
> terms are smaller than the normal font, and their
> line width is less heavy than normal.  I'm not really
> invested one way or the other, but I find it kind of
> nice that the varlistentry's #s are easier to click
> on and more noticable because they're slightly larger
> than might be expected.

TBH I didn't bother a lot with this.

Most of the time it's actually not the font size but rather the
font-family which gets inherited from the parent element if you don't
set it explicitly.

The link just inherits everithing (including the color, which I have set
to inherit explicitly since links don't inherit the parent's color by
default) from it's parent, which is the HTML <dt> element (ultimately
the inheritance probably goes up to the <body> element style in pretty
much all cases).

In some instances the input <term> element contains elements that are
styled differently in the output (e.g.: <literal> which translates to
HTML <code> which has "font-family: monospace;") which makes the # from
the link appear differently than the visible element it appears after.

Since (after tweaking the color) the general visual appearence looked ok
to me, I didn't bother with this any further.

Regards,

Brar

  57. Re: doc: add missing "id" attributes to extension packaging page

    Brar Piening <brar@gmx.de> — 2023-03-24T05:59:33Z

    On 24.03.2023 at 06:48, Brar Piening wrote:
>
> Since (after tweaking the color) the general visual appearence looked
> ok to me, I didn't bother with this any further.

Also, if we go on with this we'll probably end up in an almost
prototypical bikeshedding scenario where PostgreSQL itself is the
nuclear power plant and the visual appearence of the hover links on the
documentation website is the color of the bikeshed.

;-)

  58. Re: doc: add missing "id" attributes to extension packaging page

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2023-03-24T09:45:03Z

    On 2023-Mar-24, Brar Piening wrote:

> On 23.03.2023 at 23:31, Karl O. Pinc wrote:
> > Hi Brar,
> > 
> > It occurs to me that I had not actually tested the
> > way the anchor is put only after the last term in a
> > varlistentry.  (The code looked obviously right
> > and should work if any varlistentry terms have anchors,
> > which they do, but....)
> > 
> > Have you tested this?
> 
> Yes, I have. See
> https://pgdocs.piening.info/app-psql.html#APP-PSQL-META-COMMAND-DE for
> an extreme case.

But why are there no anchors next to <h3> items on that page?  For
example, how do I get the link for the "Meta Commands" subsection?

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
<inflex> really, I see PHP as like a strange amalgamation of C, Perl, Shell
<crab> inflex: you know that "amalgam" means "mixture with mercury",
       more or less, right?
<crab> i.e., "deadly poison"

  59. Re: doc: add missing "id" attributes to extension packaging page

    Brar Piening <brar@gmx.de> — 2023-03-26T07:57:17Z

    On 24.03.2023 at 10:45, Alvaro Herrera wrote:
> But why are there no anchors next to <h3> items on that page?  For
> example, how do I get the link for the "Meta Commands" subsection?

I somehow knew that responding from a crappy mobile phone e-mail client
will mess up the format and the thread...

For those trying to follow the thread in the archives: my response (it's
probably a refsect which isn't supported yet) ended up here:
https://www.postgresql.org/message-id/1N1fn0-1qd4xd1MyG-011ype%40mail.gmx.net

Regards,

Brar

  60. Re: doc: add missing "id" attributes to extension packaging page

    Brar Piening <brar@gmx.de> — 2023-03-27T17:05:52Z

    On 23.03.2023 at 20:08, Brar Piening wrote:
> Since the need for ids is starting to grow again (ecb696527c added an
> id to a varlistentry in doc/src/sgml/ref/create_subscription.sgml)
> I've also amended the add-needed-ids patch once again so that the
> build does not fail after applying the make_html_ids_discoverable patch.

New add-needed-ids patch since it was outdated again.

Regards,

Brar

  61. Re: doc: add missing "id" attributes to extension packaging page

    Peter Smith <smithpb2250@gmail.com> — 2023-03-27T22:11:57Z

    On Tue, Mar 28, 2023 at 4:06 AM Brar Piening <brar@gmx.de> wrote:
>
> On 23.03.2023 at 20:08, Brar Piening wrote:
> > Since the need for ids is starting to grow again (ecb696527c added an
> > id to a varlistentry in doc/src/sgml/ref/create_subscription.sgml)
> > I've also amended the add-needed-ids patch once again so that the
> > build does not fail after applying the make_html_ids_discoverable patch.
>
> New add-needed-ids patch since it was outdated again.
>

FYI, there is a lot of overlap between this last attachment and the
patches of Kuroda-san's current thread here [1] which is also adding
ids to create_subscription.sgml.

(Anyway, I guess you might already be aware of that other thread
because your new ids look like they have the same names as those
chosen by Kuroda-san)

------
[1] https://www.postgresql.org/message-id/flat/CAHut%2BPvzo6%3DKKLqMR6-mAQdM%2Bj_dse0eUreGmrFouL7gbLbv2w%40mail.gmail.com#7da8d0e3b73096375847c16c856b4aed

Kind Regards,
Peter Smith.
Fujitsu Australia

  62. Re: doc: add missing "id" attributes to extension packaging page

    Brar Piening <brar@gmx.de> — 2023-03-28T10:17:28Z

    On 28.03.2023 at 00:11, Peter Smith wrote:
> FYI, there is a lot of overlap between this last attachment and the
> patches of Kuroda-san's current thread here [1] which is also adding
> ids to create_subscription.sgml.
>
> (Anyway, I guess you might already be aware of that other thread
> because your new ids look like they have the same names as those
> chosen by Kuroda-san)
>
> ------
> [1] https://www.postgresql.org/message-id/flat/CAHut%2BPvzo6%3DKKLqMR6-mAQdM%2Bj_dse0eUreGmrFouL7gbLbv2w%40mail.gmail.com#7da8d0e3b73096375847c16c856b4aed

Thanks, I actually was not aware of this.

Also, kudos for capturing the missing id and advocating for consistency
regarding ids even before this is actively enforced. This nurtures my
optimism that consistency might actually be achieveable without
everybody getting angry at me because my patch will enforce it.

Regarding the overlap, I currently try to make it as easy as possible
for a potential committer and I'm happy to rebase my patch upon request
or if Kuroda-san's patch gets applied first.

Regards,

Brar

  63. RE: doc: add missing "id" attributes to extension packaging page

    Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> — 2023-03-29T04:52:56Z

    Dear my comrade Brar,

> Thanks, I actually was not aware of this.
> 
> Also, kudos for capturing the missing id and advocating for consistency
> regarding ids even before this is actively enforced. This nurtures my
> optimism that consistency might actually be achieveable without
> everybody getting angry at me because my patch will enforce it.
> 
> Regarding the overlap, I currently try to make it as easy as possible
> for a potential committer and I'm happy to rebase my patch upon request
> or if Kuroda-san's patch gets applied first.

FYI - my patch is pushed [1]. Could you please rebase your patch?
I think it's ok to just remove changes from logical-replication.sgml, ref/alter_subscription.sgml,
and ref/create_subscription.sgml.

[1]: https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=de5a47af2d8003dee123815bb7e58913be9a03f3

Best Regards,
Hayato Kuroda
FUJITSU LIMITED

  64. Re: doc: add missing "id" attributes to extension packaging page

    Brar Piening <brar@gmx.de> — 2023-03-29T16:03:48Z

    On 29.03.2023 at 06:52, Hayato Kuroda (Fujitsu) wrote:
> FYI - my patch is pushed

Thanks!


> Could you please rebase your patch?

Done and tested. Patch is attached.

Regards,

Brar

  65. RE: doc: add missing "id" attributes to extension packaging page

    Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> — 2023-03-30T08:11:13Z

    Dear Brar,

Thank you for updating the patch. The patch looks good to me.

Best Regards,
Hayato Kuroda
FUJITSU LIMITED

  66. Re: doc: add missing "id" attributes to extension packaging page

    Peter Eisentraut <peter.eisentraut@enterprisedb.com> — 2023-04-04T14:22:14Z

    On 29.03.23 18:03, Brar Piening wrote:
> On 29.03.2023 at 06:52, Hayato Kuroda (Fujitsu) wrote:
>> FYI - my patch is pushed
> 
> Thanks!
> 
> 
>> Could you please rebase your patch?
> 
> Done and tested. Patch is attached.

I have committed the most recent patch that added some missing IDs.  (I 
also added a missing xreflabel in passing.)

I'll look at the XSLT patch next.

  67. Re: doc: add missing "id" attributes to extension packaging page

    Peter Eisentraut <peter.eisentraut@enterprisedb.com> — 2023-04-04T14:54:23Z

    On 23.03.23 20:08, Brar Piening wrote:
> I've also attached the (unchanged)  make_html_ids_discoverable patch for
> convenience so this email now contains two patches for postgresql
> (ending with .postgresql.patch) and one patch for pgweb (ending with
> .pgweb.patch).

Here is my view on this:

First of all, it works very nicely and is very useful.  Very welcome.

The XSLT implementation looks sound to me.  It would be a touch better 
if it had some comments about which parts of the templates were copied 
from upstream stylesheets and which were changed.  There are examples of 
such commenting in the existing customization layer.  Also, avoid 
introducing whitespace differences during said copying.

However, I wonder if this is the right way to approach this.  I don't 
think we should put these link markers directly into the HTML.  It feels 
like this is the wrong layer.  For example, if you have CSS turned off, 
then all these # marks show up by default.

It seems to me that the correct way to do this is to hook in some 
JavaScript that does this transformation directly on the DOM.  Then we 
don't need to carry this presentation detail in the HTML.  Moreover, it 
would avoid tight coupling between the website and the documentation 
sources.  You can produce the exact same DOM, that part seems okay, just 
do it elsewhere.  Was this approach considered?  I didn't see it in the 
thread.

  68. Re: doc: add missing "id" attributes to extension packaging page

    Brar Piening <brar@gmx.de> — 2023-04-04T19:52:31Z

    On 04.04.2023 at 16:54, Peter Eisentraut wrote:
>
> First of all, it works very nicely and is very useful.  Very welcome.

Thank you!


> The XSLT implementation looks sound to me.  It would be a touch better
> if it had some comments about which parts of the templates were copied
> from upstream stylesheets and which were changed.  There are examples
> of such commenting in the existing customization layer.  Also, avoid
> introducing whitespace differences during said copying.

I will amend the patch if we agree that this is the way forward.


> However, I wonder if this is the right way to approach this.  I don't
> think we should put these link markers directly into the HTML.  It
> feels like this is the wrong layer.  For example, if you have CSS
> turned off, then all these # marks show up by default.

I'd consider this a feature rather than a problem but this is certainly
debatable. I cannot reliably predict what expectations a user who is
browsing the docs with CSS disabled might have. The opposite is true
too. If we'd move the id links feature to javascript, a user who has
javascript disabled will not see them. Is this what they'd want? I don't
know.

Also, while about 1-2% of users have Javascript disabled, I haven't
heard of disabling CSS except for debugging purposes.

In general I'd consider the fact that CSS or Javascript might be
disabled a niche problem that isn't really worth much debating but there
is definitely something to consider regarding people using screen
readers who might suffer from one or the other behavior and I'd
definitely be interested what behavior these users would expect. Would
they want to use the id link feature or would the links rather disrupt
their reading experience - I have no idea TBH and I hate speculating
about other people's preferences.


> It seems to me that the correct way to do this is to hook in some
> JavaScript that does this transformation directly on the DOM. Then we
> don't need to carry this presentation detail in the HTML. Moreover, it
> would avoid tight coupling between the website and the documentation
> sources.  You can produce the exact same DOM, that part seems okay,
> just do it elsewhere.  Was this approach considered?  I didn't see it
> in the thread.

I briefly touched the topic in [1] and [2] but we didin't really follow
up on the best approach.


Regards,

Brar


[1]
https://www.postgresql.org/message-id/68b9c435-d017-93cc-775a-c604db9ec683%40gmx.de

[2]
https://www.postgresql.org/message-id/a75b6d7c-3fa4-d6a8-cf23-6b5180237392%40gmx.de

  69. Re: doc: add missing "id" attributes to extension packaging page

    Karl O. Pinc <kop@karlpinc.com> — 2023-04-04T21:21:42Z

    On Tue, 4 Apr 2023 21:52:31 +0200
Brar Piening <brar@gmx.de> wrote:

> On 04.04.2023 at 16:54, Peter Eisentraut wrote:

> > The XSLT implementation looks sound to me.  It would be a touch
> > better if it had some comments about which parts of the templates
> > were copied from upstream stylesheets and which were changed.

I like this idea.  A lot. (Including which stylesheets were copied
from.)

> > However, I wonder if this is the right way to approach this.  I
> > don't think we should put these link markers directly into the
> > HTML.  It feels like this is the wrong layer.  For example, if you
> > have CSS turned off, then all these # marks show up by default.  
> 
> I'd consider this a feature rather than a problem but this is
> certainly debatable.

I too would consider this a feature.  If you don't style your
html presentation, you see everything.  The "#" links to content
are, well, content.

> > It seems to me that the correct way to do this is to hook in some
> > JavaScript that does this transformation directly on the DOM. Then
> > we don't need to carry this presentation detail in the HTML.

I would argue the opposite.  The HTML/CSS is delivered to the browser
which is then free to present the content to the user in the
form preferred by the user.  This puts control of presentation
in the hands of the end-user, where, IMO, it belongs.

Using JavaScript to manipulate the DOM is all well and good when
using AJAX to interact with the server to produce dynamic content.
But otherwise manipulating the DOM with JavaScript seems overly
heavy-handed, even though popular.  It seems like JavaScript is
used more because CSS is difficult and an "extra technology" when
instead JavaScript can "do everything".  So CSS is put aside.

I may be biased, not being a JavaScript fan.  (I tend to be one
of those cranky individuals who keep JavaScript turned off.)
I'd rather not have code executing when such overhead/complication
can be avoided.  (Insert here exciting argument about "what is code
and what is data".)

Glancing at the documentation source, I don't see JavaScript used
at all.  Introducing it would be adding something else to the mix.
Not that this would be bad if it provides value.

In the end, I don't _really_ care.  And don't do web development
all day either so my fundamentals could be just wrong.  But
this is my take.

> > Moreover, it would avoid tight coupling between the website and the
> > documentation sources. 

I don't really understand this statement.  Are you saying that
the documentation's source CSS needn't/shouldn't be the CSS used
on the website?  That seems counter-intuitive.  But then I don't
understand why the default CSS used when developing the documentation
is not the CSS used on the website.

I can imagine administrative arguments around server maintenance for
wanting to keep the website decoupled from the PG source code.
(I think.)  But I can't speak to any of that.

Regards,

Karl <kop@karlpinc.com>
Free Software:  "You don't pay back, you pay forward."
                 -- Robert A. Heinlein

  70. Re: doc: add missing "id" attributes to extension packaging page

    Maciek Sakrejda <m.sakrejda@gmail.com> — 2023-04-05T22:00:45Z

    For what it's worth, I think having the anchors be always-visible when
CSS disabled is a feature. The content is still perfectly readable,
and the core feature from this patch is available. Introducing
JavaScript to lose that functionality seems like a step backwards.

By the way, the latest patch attachment was not the full patch series,
which I think confused cfbot: [1]  (unless I'm misunderstanding the
state of the patch series).

And thanks for working on this. I've hunted in the page source for ids
to link to a number of times. I look forward to not doing that
anymore.

Thanks,
Maciek

[1]: https://commitfest.postgresql.org/42/4042/

  71. Re: doc: add missing "id" attributes to extension packaging page

    Peter Eisentraut <peter.eisentraut@enterprisedb.com> — 2023-04-06T09:06:18Z

    On 04.04.23 21:52, Brar Piening wrote:
>> The XSLT implementation looks sound to me.  It would be a touch better
>> if it had some comments about which parts of the templates were copied
>> from upstream stylesheets and which were changed.  There are examples
>> of such commenting in the existing customization layer.  Also, avoid
>> introducing whitespace differences during said copying.
> 
> I will amend the patch if we agree that this is the way forward.

Ok, it appears everyone agrees that this is the correct approach. 
Please post an updated patch.  There have been so many partial patches 
posted recently, I'm not sure which one is the most current one and who 
is really the author.

  72. Re: doc: add missing "id" attributes to extension packaging page

    Brar Piening <brar@gmx.de> — 2023-04-06T14:19:30Z

    On 06.04.2023 at 11:06, Peter Eisentraut wrote:
> On 04.04.23 21:52, Brar Piening wrote:
>>> The XSLT implementation looks sound to me.  It would be a touch better
>>> if it had some comments about which parts of the templates were copied
>>> from upstream stylesheets and which were changed.  There are examples
>>> of such commenting in the existing customization layer.  Also, avoid
>>> introducing whitespace differences during said copying.
>>
>> I will amend the patch if we agree that this is the way forward.
>
> Ok, it appears everyone agrees that this is the correct approach.
> Please post an updated patch.  There have been so many partial patches
> posted recently, I'm not sure which one is the most current one and
> who is really the author.
>
Attached are two patches:

001-make_html_ids_discoverable_v5.postgresql.patch which needs to be
applied to the postgresql repository. It adds the XSLT to generate the
id links and the CSS to hide/display them. I've added comments as
suggested above.

002-add-discoverable-id-style_v1.pgweb.patch which needs to be applied
to the pgweb repository. It adds the CSS to the offical documentation site.

At the moment (commit 983ec23007) there are no missing ids, so the build
should just work after applying the patch but, as we already know, this
may change with every commit that gets added.

Reviewer is Karl O. Pink

Author is Brar Piening (with some additions from Karl O. Pink)

Best regards,

Brar

  73. Re: doc: add missing "id" attributes to extension packaging page

    Karl O. Pinc <kop@karlpinc.com> — 2023-04-06T15:05:52Z

    On Thu, 6 Apr 2023 16:19:30 +0200
Brar Piening <brar@gmx.de> wrote:

> Reviewer is Karl O. Pink

"Karl O. Pinc" actually, with a "c".

Regards,

Karl <kop@karlpinc.com>
Free Software:  "You don't pay back, you pay forward."
                 -- Robert A. Heinlein

  74. Re: doc: add missing "id" attributes to extension packaging page

    Peter Eisentraut <peter.eisentraut@enterprisedb.com> — 2023-04-13T08:31:43Z

    On 06.04.23 16:19, Brar Piening wrote:
> 001-make_html_ids_discoverable_v5.postgresql.patch which needs to be
> applied to the postgresql repository. It adds the XSLT to generate the
> id links and the CSS to hide/display them. I've added comments as
> suggested above.
> 
> 002-add-discoverable-id-style_v1.pgweb.patch which needs to be applied
> to the pgweb repository. It adds the CSS to the offical documentation site.

The first patch has been committed.

The second patch should be sent to pgsql-www for integrating into the 
web site.

Side project: I noticed that these new hover links don't appear in the 
single-page HTML output (make postgres.html), even though the generated 
HTML source code looks correct.  Maybe someone has an idea there.

  75. Re: doc: add missing "id" attributes to extension packaging page

    Brar Piening <brar@gmx.de> — 2023-04-13T14:01:35Z

    On 13.04.2023 at 10:31, Peter Eisentraut wrote:
> The first patch has been committed.

Yay - thank you!

> The second patch should be sent to pgsql-www for integrating into the
> web site.
Done via [1]. Thanks for the hint.

> Side project: I noticed that these new hover links don't appear in the
> single-page HTML output (make postgres.html), even though the
> generated HTML source code looks correct.  Maybe someone has an idea
> there.
I feel responsible for the feature to work for all use cases where it
makes sense. I'll investigate this and post back.

Regards,
Brar

[1]
https://www.postgresql.org/message-id/d987a4a7-62c3-7e0c-860f-1c96fc2117d9%40gmx.de

  76. Re: doc: add missing "id" attributes to extension packaging page

    Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> — 2023-04-13T14:58:03Z

    Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:

> On 06.04.23 16:19, Brar Piening wrote:
>> 001-make_html_ids_discoverable_v5.postgresql.patch which needs to be
>> applied to the postgresql repository. It adds the XSLT to generate the
>> id links and the CSS to hide/display them. I've added comments as
>> suggested above.
>> 002-add-discoverable-id-style_v1.pgweb.patch which needs to be applied
>> to the pgweb repository. It adds the CSS to the offical documentation site.
>
> The first patch has been committed.
>
> The second patch should be sent to pgsql-www for integrating into the
> web site.
>
> Side project: I noticed that these new hover links don't appear in the
> single-page HTML output (make postgres.html), even though the generated 
> HTML source code looks correct.  Maybe someone has an idea there.

Another side note: I notice the links don't appear on <refsectN> elements
(e.g. https://www.postgresql.org/docs/devel/sql-select.html#SQL-WITH),
only <sectN>.

- ilmari

  77. Re: doc: add missing "id" attributes to extension packaging page

    Karl O. Pinc <kop@karlpinc.com> — 2023-04-13T15:40:09Z

    On Thu, 13 Apr 2023 15:58:03 +0100
Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> wrote:

> Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:

> > The first patch has been committed.

> Another side note: I notice the links don't appear on <refsectN>
> elements (e.g.
> https://www.postgresql.org/docs/devel/sql-select.html#SQL-WITH), only
> <sectN>.

This we know.  Working with <refsectN> elements is a different
dive into the XSLT which was deliberately put off for future
work.

Regards,

Karl <kop@karlpinc.com>
Free Software:  "You don't pay back, you pay forward."
                 -- Robert A. Heinlein

  78. Re: doc: add missing "id" attributes to extension packaging page

    Karl O. Pinc <kop@karlpinc.com> — 2023-04-13T15:53:31Z

    On Thu, 13 Apr 2023 16:01:35 +0200
Brar Piening <brar@gmx.de> wrote:

> On 13.04.2023 at 10:31, Peter Eisentraut wrote:
> > The first patch has been committed.  
> 
> Yay - thank you!
> 
> > The second patch should be sent to pgsql-www for integrating into
> > the web site.  
> Done via [1]. Thanks for the hint.
> 
> > Side project: I noticed that these new hover links don't appear in
> > the single-page HTML output (make postgres.html), even though the
> > generated HTML source code looks correct.  Maybe someone has an idea
> > there.  
> I feel responsible for the feature to work for all use cases where it
> makes sense. I'll investigate this and post back.

Looks to me like the ">" in the CSS was transformed into the &gt;
HTML entity when the stylesheet was included into the single-file
HTML.

Regards,

Karl <kop@karlpinc.com>
Free Software:  "You don't pay back, you pay forward."
                 -- Robert A. Heinlein

  79. Re: doc: add missing "id" attributes to extension packaging page

    Karl O. Pinc <kop@karlpinc.com> — 2023-04-14T17:41:23Z

    On Thu, 13 Apr 2023 10:53:31 -0500
"Karl O. Pinc" <kop@karlpinc.com> wrote:

> On Thu, 13 Apr 2023 16:01:35 +0200
> Brar Piening <brar@gmx.de> wrote:
> 
> > On 13.04.2023 at 10:31, Peter Eisentraut wrote:  

> > > Side project: I noticed that these new hover links don't appear in
> > > the single-page HTML output (make postgres.html), even though the
> > > generated HTML source code looks correct.  Maybe someone has an
> > > idea there.    
> > I feel responsible for the feature to work for all use cases where
> > it makes sense. I'll investigate this and post back.  
> 
> Looks to me like the ">" in the CSS was transformed into the &gt;
> HTML entity when the stylesheet was included into the single-file
> HTML.

The XSLT 1.0 spec says that characters in <style> elements should
not be escaped when outputting HTML.  [4]  But (I think) the
generate.css.header parameter method [1][2] of 
inserting the CSS into the HTML expands the CSS content
in an XML context, not a HTML context.  

I've played around with it, going so far as to make stylesheet.css
look like:

--------------<snip>--------
<!DOCTYPE  xsl:stylesheet [
<!ENTITY css SYSTEM "stylesheet.css">
]>
<xsl:stylesheet
  xmlns:xsl="http://www.w3.org/1999/XSL/Transform">
  &css;
</xsl:stylesheet>
--------------<snip>--------

and even substituted the actual text of stylesheet.css in
place of the &css; entity reference.  In these cases the
"<" character is still entity expanded, resulting in broken CSS.

My conclusion is that this method is broken.

(The other possibility, I suppose, is that xsltproc is broken.)

I think the thing to try is the sagehill.net approach [4].
This overrides the user.head.content template.  My hope is
that because the &css; entity is seen in a <style> element
in the template, that xsltproc then recognizances style element
content in a HTML output context.

(I don't know how xsltproc is supposed to know that it is
in a HTML output context.  I suppose exploring this would
be another avenue should the above fail.)


1
https://docbook.sourceforge.net/release/xsl/current/doc/html/custom.css.source.html

2
https://docbook.sourceforge.net/release/xsl/current/doc/html/generate.css.header.html

3 http://sagehill.net/docbookxsl/HtmlHead.html#EmbedCSS

4 https://www.w3.org/TR/xslt-10/#section-HTML-Output-Method

Regards,

Karl <kop@karlpinc.com>
Free Software:  "You don't pay back, you pay forward."
                 -- Robert A. Heinlein

  80. Re: doc: add missing "id" attributes to extension packaging page

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2025-11-14T10:59:51Z

    On 2023-Apr-13, Karl O. Pinc wrote:

> Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> wrote:

> > Another side note: I notice the links don't appear on <refsectN>
> > elements (e.g.
> > https://www.postgresql.org/docs/devel/sql-select.html#SQL-WITH),
> > only <sectN>.
> 
> This we know.  Working with <refsectN> elements is a different
> dive into the XSLT which was deliberately put off for future
> work.

Hi, has anybody been able to spend some time on the <refsectN> aspect of
these link things?

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"No hay ausente sin culpa ni presente sin disculpa" (Prov. francés)

  81. Re: doc: add missing "id" attributes to extension packaging page

    Brar Piening <brar@gmx.de> — 2025-11-15T10:53:34Z

    On 2025-11-14 at 11:59 Alvaro Herrera wrote:
> On 2023-Apr-13, Karl O. Pinc wrote:
>
>> Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> wrote:
>>> Another side note: I notice the links don't appear on <refsectN>
>>> elements (e.g.
>>> https://www.postgresql.org/docs/devel/sql-select.html#SQL-WITH),
>>> only <sectN>.
>> This we know.  Working with <refsectN> elements is a different
>> dive into the XSLT which was deliberately put off for future
>> work.
> Hi, has anybody been able to spend some time on the <refsectN> aspect of
> these link things?

Hi,

tbh, this has been in my backlog since I did the initial thing.

I haven't prioritized it since it is really a completely different area 
XSLT-wise that cannot build on the changes I have made and I wanted to 
see how well my initial work will prove its value and be accepted by the 
maintainers and the community before starting to work on this and 
introducing even more changes in this kind of ugly XSLT area that nobody 
really likes to touch.

I think, now that some  time has passed, everybody working on patches in 
the docs area should have some opinion on how annoying it is to be 
forced to provide ids for various elements and if the advantage of being 
able to refer to specific items in the HTML docs is really worth it.

Also, as we've learnt the last time, a patch that enforces the presence 
of ids in refsections would need to be applied in a quick and 
cooperative way once we agree on the general structure, since it needs 
to be rebased and enhanced whenever a new refsection without ids (or 
other element where it enforces ids) is added by some patch and will 
break all subsequent patches that try to add refsections without ids 
(forcing their developers/reviewers to rebase and add ids accordingly).

If we can build some consensus that we want this for the refsections as 
well, I'm willing to put some time aside and work on this (probably at 
some point next year since I'm currently drowning in work).

Regards,

Barr