Re: Patch to document base64 encoding

On Wed, 6 Mar 2019 19:30:16 +0100 (CET)
Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> wrote:

> "... section 6.8" -> "... Section 6.8" (capital S).

Fixed.

> "The string and the binary encode and decode functions..." sentence
> looks strange to me, especially with the English article that I do
> not really master, so maybe it is ok. I'd have written something more
> straightforward, eg: "Functions encode and decode support the
> following encodings:",

It is an atypical construction because I want to draw attention that
this is documentation not only for the encode() and decode() in
section 9.4. String Functions and Operators but also for
the encode() and decode in section 9.5. Binary String Functions
and Operators. Although I can't think of a better approach
it makes me uncomfortable that documentation written in
one section applies equally to functions in a different section.

Do you think it would be useful to hyperlink the word "binary"
to section 9.5?

The idiomatic phrasing would be "Both the string and the binary
encode and decode functions..." but the word "both" adds
no information. Shorter is better.

> and also I'd use a direct "Function
> <...>decode</...> ..." rather than "The <function>decode</function>
> function ..." (twice).

The straightforward English would be "Decode accepts...". The problem
is that this begins the sentence with the name of a function.
This does not work very well when the function name is all lower case,
and can have other problems where clarity is lost depending
on documentation output formatting.

I don't see a better approach.

> Maybe I'd use the exact same grammatical structure for all 3 cases,
> starting with "The <>whatever</> encoding converts bla bla bla"
> instead of varying the sentences.

Agreed. Good idea. The first paragraph of each term has to
do with encoding and the second with decoding.
Uniformity in starting the second paragraphs helps make
this clear, even though the first paragraphs are not uniform.
With this I am not concerned that the first paragraphs
do not have a common phrasing that's very explicit about
being about encoding.

Adjusted.

> Otherwise, all explanations look both precise and useful to me.

When writing I was slightly concerned about being overly precise;
permanently committing to behavior that might (possibly) be an artifact
of implementation. E.g., that hex decoding accepts both
upper and lower case A-F characters, what input is ignored
and what raises an error, etc. But it seems best
to document existing behavior, all of which has existed so long
anyway that changing it would be disruptive. If anybody cares
they can object.

I wrote the docs by reading the code and did only a little
actual testing to be sure that what I wrote is correct.
I also did not check for regression tests which confirm
the behavior I'm documenting. (It wouldn't hurt to have
such regression tests, if they don't already exist.
But writing regression tests is more than I want to take on
with this patch. Feel free to come up with tests. :-)

I'm confident that the behavior I documented is how PG behaves
but you should know what I did in case you want further
validation.

Attached: doc_base64_v8.patch

Regards,

Karl <kop(at)meme(dot)com>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein