RE: [PGdocs] fix description for handling pf non-ASCII characters

From: "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>
To: "'Karl O(dot) Pinc'" <kop(at)karlpinc(dot)com>
Cc: 'jian he' <jian(dot)universality(at)gmail(dot)com>, Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: RE: [PGdocs] fix description for handling pf non-ASCII characters
Date: 2023-09-27 12:58:54
Message-ID: TYAPR01MB58663EB061888B2715A39217F5C2A@TYAPR01MB5866.jpnprd01.prod.outlook.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Dear Karl,

Thank you for reviewing!

> A related thing that's nice to have is to limit the line
> length of the documentation source to 80 characters or less.
> 79 is probably best. Since the source text around your patch
> conforms to this convention you should also.

IIUC it is not hard limit, but I followed this.

> Should the committer be interested, your patch applies cleanly
> and the docs build as expected.

Yeah, but cfbot accepted previous version. Did you have anything in your mind?

> Also, based on the comments in the
> patch which changed the system's behavior, I believe that
> your patch updates all the relevant places in the documentation.

Thanks. Actually, I think it should be backpatched to PG16 because the commit was
done last year. I will make the patch for it after deciding the explanation.

>
> I now think that you should consider another change to your wording.
> Instead of starting with "Characters that are not printable ASCII ..."
> consider writing "The bytes of the string which are not printable ASCII
> ...". Notice above that characters (e.g. あ) generate output for
> each non-ASCII byte (e.g. \xe3\x81\x82). So my thought is that
> the docs should be talking about bytes.
>
> For the last hunk you'd change around "anything". Write:
> "... it will be truncated to less than NAMEDATALEN characters and
> the bytes of the string which are not printable ASCII characters ...".
>

Hmm, what you said looked right. But as Peter pointed out [1], the fix seems too
much. So I attached three version of patches. How do you think?
For me, type C is best.

A. A patch which completely follows your comments. The name is "v3-0001-...patch".
Cfbot tests it.
B. A patch which completely follows Peter's comments [1]. The name is "Peter_v3-....txt".
C. A patch which follows both comments. Based on b, but some comments
(Don't use the future tense, "Other characters"->"The bytes of other characters"...)
were picked. The name is "Both_v3-....txt".

[1]: https://www.postgresql.org/message-id/CAHut%2BPvEbKC8ABA_daX-XPNOTFzuAmHGhjPj%3DtPZYQskRHECOg%40mail.gmail.com

Best Regards,
Hayato Kuroda
FUJITSU LIMITED

Attachment Content-Type Size
Both_v3-0001-Fix-description-for-handling-of-non-printable-AS.txt text/plain 3.9 KB
v3-0001-Fix-description-for-handling-of-non-printable-AS.patch application/octet-stream 4.5 KB
Peter_v3-0001-Fix-description-for-handling-of-non-printable-AS.txt text/plain 3.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Hayato Kuroda (Fujitsu) 2023-09-27 12:59:47 RE: [PGdocs] fix description for handling pf non-ASCII characters
Previous Message Peter Eisentraut 2023-09-27 12:56:08 Re: should frontend tools use syncfs() ?