On Thu, 15 May 2025 17:27:42 GMT, David Beaumont <d...@openjdk.org> wrote:
>> src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java line 167:
>>
>>> 165: return null;
>>> 166: }
>>> 167: case String label when READ_WRITE.label.equals(label)
>>> -> {
>>
>> I haven't yet fully caught up on the newer features of switch/case. Does
>> this have any advantage as compared to the simpler:
>>
>>
>> case "readWrite" -> {
>> return READ_WRITE;
>> }
>> case "readOnly" -> {
>> return READ_ONLY;
>> }
>
> Or just an if :)
>
> The reason for the new style was the "need" for the "String label when"
> qualifier, which I don't actually need because String.equals(xx) copes with
> being given non-strings fine. You can't use the syntax you suggested in the
> new switch since "value" isn't a String.
Thank you for updating this to a traditional and trivial `if/else`.
>> src/jdk.zipfs/share/classes/module-info.java line 277:
>>
>>> 275: * <td>
>>> 276: * A value defining the desired read/write access mode of the
>>> file system
>>> 277: * (either <em>read-write</em> or <em>read-only</em>).
>>
>> This line is slightly confusing. Initially I thought `read-write` and
>> `read-only` are the actual values that this environment property will
>> accept. But further review suggests that the actual literals supported are
>> `readOnly` and `readWrite` and this line here I think is trying to explain
>> the supported semantics of the file system.
>>
>> Perhaps we could reword this to something like:
>>
>>>
>>> A value defining the desired access mode of the file system. A ZIP file
>>> system can be created to allow for read-write access or read-only access.
>
> Yeah, I tried to distinguish the accessMode flags (for which there are 3
> states, ro, rw, n/a) and the final state of the file system from
> fs.isReadOnly(). Hence using `<em>...</em>` for whenever the actually
> resulting state is mentioned. I guess it wasn't clear enough.
Thank you for updating this part, this is much more clearer now.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25178#discussion_r2095625151
PR Review Comment: https://git.openjdk.org/jdk/pull/25178#discussion_r2095622485
