On Mon, 20 Apr 2026 16:34:31 GMT, Naoto Sato <[email protected]> wrote:
>> make/jdk/src/classes/build/tools/cldrconverter/CLDRConverter.java line 1096:
>>
>>> 1094: formatter.format("\\u%04x", (int)aChar);
>>> 1095: } else {
>>> 1096: if (aChar == 0x0022) {
>>
>> What about using the corresponding char literal (`'"'`) to make it easier to
>> understand what this is escaping?
>>
>> Suggestion:
>>
>> if (aChar == '"') {
>>
>>
>> (This is only a suggestion; I am not an OpenJDK member, feel free to ignore
>> this comment.)
>
> Thanks. I added a comment explaining it is for ASCII quotation marks
Thanks, I think it would have been better to replace the `0x0022` with `'"'`
(which would be functionality identical), but I assume you have your reasons
why you did not want to do that.
As side note: I just noticed that another possibility would be to merge this
`"` handling with the other escaping logic in the `case` branches above,
specifically `case '\'`, e.g.:
...
case '\', '"':
// Escape the char
outBuffer.append('\');
outBuffer.append(aChar);
break;
...
Though this was just a suggestion anyway, and this PR is already approved, so
feel free to ignore.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/30797#discussion_r3116084226
