<Swing Dev> RFR: 8182043: Access to Windows Large Icons [v13]

Phil Race prr at openjdk.java.net
Tue May 25 23:20:28 UTC 2021


On Tue, 25 May 2021 22:01:36 GMT, Alexander Zuev <kizune at openjdk.org> wrote:

>> Fix updated after first round of review.
>
> Alexander Zuev has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Changed API to support non-square icons and updated documentation
>   accordingly
>   Fixed test to support updated API
>   Added negative test cases

src/java.desktop/share/classes/javax/swing/filechooser/FileSystemView.java line 282:

> 280:     * @return an icon as it would be displayed by a native file chooser
> 281:     * or null if invalid parameters are passed such as reference to a
> 282:     * non-existing file.

non-existent not non-existing, but I think null deserves an IAE - as I had written a couple of days ago.

I see you dropped the words about null if the size is too large.
Should I interpret that as meaning one of the things I suggested - that this is just another case of a closest match ?

src/java.desktop/share/classes/javax/swing/filechooser/FileSystemView.java line 286:

> 284:     * @see AbstractMultiResolutionImage
> 285:     * @see FileSystemView#getSystemIcon(File)
> 286:     * @since 17

You need to add the IAE to the javadoc.

src/java.desktop/share/classes/javax/swing/filechooser/FileSystemView.java line 290:

> 288:     public Icon getSystemIcon(File f, int width, int height) {
> 289:         if (height <1 || width < 1) {
> 290:             throw new IllegalArgumentException("Icon size can not be below 1");

(minor) spacing. <1

src/java.desktop/share/classes/javax/swing/filechooser/FileSystemView.java line 294:

> 292: 
> 293:         if (f == null || !f.exists()) {
> 294:             return null;

I suggested the a null File ought to be IAE too - this is showing up in the conversation thread but not here.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2875


More information about the swing-dev mailing list