Dear Peter, Thank you for reviewing!
> > > > > 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". > > > > > > > > I also like C. Fewer words is better. So long > > > > as nothing is left unsaid fewer words make for clarity. > > > > > > > > However, in the last hunk, "of other than" does not read well. > > > > Instead of writing > > > > "and the bytes of other than printable ASCII characters" > > > > you want "and the bytes that are not printable ASCII characters". > > > > That would be my suggestion. > > > > > > > > > > I also prefer Option C, but... Okay, C was chosen. > > > > > > ~~~ > > > > > > + <varname>application_name</varname> value. > > > + The bytes of other characters are replaced with > > > + <link linkend="sql-syntax-strings-escape">C-style escaped > > > hexadecimal > > > + byte values</link>. > > > > > > V > > > > > > + <varname>cluster_name</varname> value. > > > + The bytes of other characters are replaced with > > > + <link linkend="sql-syntax-strings-escape">C-style escaped > > > hexadecimal > > > + byte values</link>. > > > > > > V > > > > > > + <symbol>NAMEDATALEN</symbol> characters and the bytes of > other > > > than > > > + printable ASCII characters are replaced with <link > > > + linkend="sql-syntax-strings-escape">C-style escaped > > > hexadecimal byte > > > + values</link>. > > > > > > > > > IIUC all of these 3 places can have exactly the same wording change > > > (e.g. like Karl's last suggestion [1]). > > > > > > SUGGESTION > > > Any bytes that are not printable ASCII characters are replaced with > > > <link linkend="sql-syntax-strings-escape">C-style escaped hexadecimal > > > byte values</link>. Hmm, I felt that using exactly the same wording seemed strange here, so similar words were used. Also, based on the comment [1], "byte" was removed. > > I had in mind something like a SHIFT-JIS encoding where a single > "character" may include some trail bytes that happen to be in the > ASCII printable range. AFAIK because the new logic is processing > bytes, not characters, I thought the end result could be a mix of > escaped and unescaped bytes for the single SJIS character. In that > context, I felt "The bytes of other characters" was not quite > accurate. > > But now looking at PostgreSQL-supported character sets [1] I saw SJIS > is not supported anyhow. Unfortunately, I am not familiar enough with > other encodings to know if there is still a chance of similar > printable ASCII trail bytes so I am fine with whatever wording is > chosen. Based on the discussion [1], I did not handle the part. > > > But because I like short sentences I now think that it's a good > > idea to break the long sentence of the last hunk into two. > > Add a period and use the Peter's SUGGESTION above as the > > text for the second sentence. > > > > Is this desireable? > > > > +1. OK, divided. New patch is available in [2]. [1]: https://www.postgresql.org/message-id/803569.1695863971%40sss.pgh.pa.us [2]: https://www.postgresql.org/message-id/TYAPR01MB5866DD962CA4FC03E338C6BBF5C1A%40TYAPR01MB5866.jpnprd01.prod.outlook.com Best Regards, Hayato Kuroda FUJITSU LIMITED
