Re: doc: add missing "id" attributes to extension packaging page - Mailing list pgsql-hackers

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