doc: add missing "id" attributes to extension packaging page
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
Attachments:
v1-0001-doc-add-id-attributes-to-extension-documentation.patchtext/x-patch; charset=US-ASCII; name=v1-0001-doc-add-id-attributes-to-extension-documentation.patchDownload+3-4
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
/messages/by-id/3bac458c-b121-1b20-8dea-0665986faa40@gmx.de
and this other one
/messages/by-id/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/
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
/messages/by-id/3bac458c-b121-1b20-8dea-0665986faa40@gmx.de
and this other one
/messages/by-id/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
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
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/
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
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:
/messages/by-id/f900c5e1-a18a-84cc-6536-e85ec655c7d7@gmx.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
Attachments:
add_html_ids.patchtext/plain; charset=UTF-8; name=add_html_ids.patchDownload+1359-1359
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
Attachments:
make_html_ids_discoverable.patchtext/plain; charset=UTF-8; name=make_html_ids_discoverable.patchDownload+121-0
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
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
Attachments:
add_html_ids_v2.patchtext/x-patchDownload+1357-1357
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
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
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]http://cfbot.cputube.org/patch_41_4041.log, 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
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
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
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
Attachments:
add_html_ids_v3.patchtext/plain; charset=UTF-8; name=add_html_ids_v3.patchDownload+1372-1372
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]/messages/by-id/20221206083809.3kaygnh2xswoxslj@alvherre.pgsql
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]https://pgdocs.piening.info/app-psql.html#APP-PSQL-OPTION-PORT to see the behavior. This
one still needs reviewing and there is no discussion around it yet.
Regards,
Brar
[1]: /messages/by-id/20221206083809.3kaygnh2xswoxslj@alvherre.pgsql
/messages/by-id/20221206083809.3kaygnh2xswoxslj@alvherre.pgsql
[2]: https://pgdocs.piening.info/app-psql.html#APP-PSQL-OPTION-PORT
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
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
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
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
