Re: First SVG graphic

On 07.02.19 12:06, Peter Eisentraut wrote:

> First, let's fix some of these whitespace errors:
>
> firstSvg_2.patch:677: trailing whitespace.
> <rect x="1" y="1" width="99.4%" height="99.4%" rx="1%"
> firstSvg_2.patch:752: trailing whitespace.
> <rect x="1" y="1" width="99.4%" height="99.4%" rx="1%"
> firstSvg_2.patch:705: new blank line at EOF.
> +
> firstSvg_2.patch:936: new blank line at EOF.
> +
> warning: 4 lines add whitespace errors.

Done.

> Let's not use mixed-case file names:
>
> Inkscape/
> PageLayout.svg
> gin.svg
> pgDump.svg
Done.
>> @@ -152,15 +156,15 @@ postgres.txt: postgres.html
>> postgres.pdf:
>> $(error Invalid target; use postgres-A4.pdf or postgres-US.pdf as targets)
>>
>> -%-A4.fo: stylesheet-fo.xsl %.sgml $(ALLSGML)
>> +%-A4.fo: stylesheet-fo.xsl %.sgml
>> $(XMLLINT) $(XMLINCLUDE) --noout --valid $(word 2,$^)
>> $(XSLTPROC) $(XMLINCLUDE) $(XSLTPROCFLAGS) --stringparam paper.type A4 -o $@ $(wordlist 1,2,$^)
>>
>> -%-US.fo: stylesheet-fo.xsl %.sgml $(ALLSGML)
>> +%-US.fo: stylesheet-fo.xsl %.sgml
>> $(XMLLINT) $(XMLINCLUDE) --noout --valid $(word 2,$^)
>> $(XSLTPROC) $(XMLINCLUDE) $(XSLTPROCFLAGS) --stringparam paper.type USletter -o $@ $(wordlist 1,2,$^)
>>
>> -%.pdf: %.fo
>> +%.pdf: %.fo $(ALLSGML) $(SVGSRC)
>> $(FOP) -fo $< -pdf $@
>>
> This seems a bit wrong. The .fo target does depend on ALLSGML. The
> .pdf target does not, but it presumably does depend on SVGSRC.
It's a transitive dependency: the pdf target is triggered after changes
in svg (or sgml), this triggers the fo targets. Therefore it's not
necessary to have svg (or sgml) dependencies at the fo level.
> The variable name SVGSRC is a bit confusing. What is it the source of?
The variable SVGSRC points to all svg-files, similar to ALLSGML which
points to the sgml files. Whenever any of them changes, certain targets
will fire.
>> @@ -209,7 +213,7 @@ check: postgres.sgml $(ALLSGML) check-tabs
>> install: install-html install-man
>>
>> installdirs:
>> - $(MKDIR_P) '$(DESTDIR)$(htmldir)'/html $(addprefix '$(DESTDIR)$(mandir)'/man, 1 3 $(sqlmansectnum))
>> + $(MKDIR_P) '$(DESTDIR)$(htmldir)'/html/svg html/svg $(addprefix '$(DESTDIR)$(mandir)'/man, 1 3 $(sqlmansectnum))
>>
>> # If the install used a man directory shared with other applications, this will remove all files.
>> uninstall:
> html/svg is not an installation directory. You need to create it
> somewhere else.
Please help. I haven't understood the distinction between installation
directory and DESTDIR. On the other hand, in the Makefile there is a -
redundant - command within the html-stamp target: $(MKDIR_P) html/svg.
But this will run frequently, which is not necessary.
>> + <part>
>> + <title>Lists of Figures, Tables and Examples</title>
>> + <appendix id="list-of-figures">
>> + <title>List of Figures</title>
>> + <para />
>> + </appendix>
>> + <appendix id="list-of-tables">
>> + <title>List of Tables</title>
>> + <para />
>> + </appendix>
>> + <appendix id="list-of-examples">
>> + <title>List of Examples</title>
>> + <para />
>> + </appendix>
>> + </part>
> These ought to be created by the stylesheet. We have probably turned
> them off somewhere, so you should see where you can turn them on.

There is a simple mechanism to create those list of figures: change line
55 of stylesheet-html-common.xsl to "book toc,title,*figure*". But the
result is ugly - see attached screenshot. The list is out-of-line.
Additionally, in the future we will have many figures (and examples and
tables). This will lead to similar problems we actually faced with the
release notes. The proposed solution moves this inflation of lists to
deeper levels of the TOC. We can and have defined theirs layout within
stylesheet-common.xsl.

>> diff --git a/doc/src/sgml/ref/pg_dump.sgml b/doc/src/sgml/ref/pg_dump.sgml
>> index 9e0bb93f08..d31ee2d210 100644
>> --- a/doc/src/sgml/ref/pg_dump.sgml
>> +++ b/doc/src/sgml/ref/pg_dump.sgml
>> @@ -73,6 +73,21 @@
>> architectures.
>> </para>
>>
>> + <figure id="pg_dump_svg">
>> + <title><command>pg_dump</command>: Formats and Restore Proceedings</title>
> This doesn't work for man page output.
>
> I think we should avoid putting images into reference pages. This one
> could perhaps go into the Backup chapter.

What is the problem here? Actually I don't have enough time to evaluate
it in deep. If it is an urgent problem (I have seen that the
commitfest-entry is tagged as "release 12") we shall shift the pg_dump
figure to a later release.

I think that the Backup chapter isn't a good place for this because it
does not explain the complete interaction between pg_dump, psql and restore.

> Also, it should be linked to from somewhere. An image that's just
> floating around and not referred to in the text seems odd.
>
> Also we tend to use hyphens instead of underscores for IDs. (At some
> point, underscores where not allowed. I'm surprised that that's no
> longer the case.)
Done. (But I hate the use of the minus-signs within any identifier of
any language. For me it's an operator.)
> I also wouldn't put "_svg" into the ID. The format
> is irrelevant to the ID.
>
I changed _svg to _figure. I agree that the format is irrelevant. But it
may be of interest, whether it is an id to a text or a figure.

Kind regards, Jürgen