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".
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 |
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() ? |