On 12/3/2015 2:02 PM, Alexander Scherbatiy wrote:

It is better to have concrete methods like Image getFileChooserIcon(String iconName) rather to have one unified method for all cases.
Please see the updated webrev: http://cr.openjdk.java.net/~ssadetsky/8081722/webrev.03/ get(String) is replaced with getChooserComboBoxFiles(). Other get() keys are not in use by NetBeans.

Thanks,
Alexandr.

On 24/11/15 14:01, Semyon Sadetsky wrote:


On 11/2/2015 11:09 PM, Sergey Bylokhov wrote:
On 29.10.15 21:30, Phil Race wrote:
We should specify what happens if you pass in to get(String)
a) null
b) an unrecognised name.

Would it make sense to define string constants on FileSystemView
as otherwise people have to spell these exactly right without compiler
help ?

Also I expect (hope!) that we do not assert any privileges here so there
will be a SecurityException if the caller does not have the necessary
permissions.
That should be documented.

I find it odd that isLink(File) catches FNFE and just returns "false"
like this
was a normal file. Why is that ? In fact I find it particularly odd since
getLinkLocation() *does* throw FNFE if it does not exist.

I guess the new code should follow the rules which are used by other methods in the same class, most of them catch FNFE and return some default value. Also most of them returns default value if the passed File is null. Anyway I think that NPE is better than IAE. At least we should discuss this.
Could you explain why NPE is better? I supposed in case of an incorrect method usage IAE is more precise than generic NPE. The FNFE is used in some methods of the class. I think that is justified if the linked file is not found. In other cases it is caught.

I am not sure why this methods are static unlike old/others methods().
I agree, we should be following the class "style", so I have removed static modifiers. The updated webrev: http://cr.openjdk.java.net/~ssadetsky/8081722/webrev.02/

public static Object get(String key) {}
Probably this method too flexible? What kind of use case for this method inside the users application? How the user will know which parameters to use in a cross-platform manner?

For example "fileChooserDefaultFolder" property already covered by FileSystemView.getDefaultDirectory(), the "roots" is covered by getRoots().
I don't think that we should elaborate a new Shell API in this fix, because we initially aimed to make the minimal change we can to support NetBeans ShellFolder extension. We did a poll on the public alias which showed nobody else need this API to be opened.



Also the docs casually say
"a shell interpreted link" and "platform shell" without explaining
what they mean by a shell. Should this term be used at all or
should the docs describe this in some other words ?

getLinkLocation says

"Returns a file linked to the specified file if the specified file"

What that reads like to me is that you get back a link if
you pass in a regular file, whereas (I believe) you mean
the opposite.

I think you mean :
"Returns the regular file referenced by the specified link file"

I also note that one of the uses we located was of
ShellFolder.getShellFolder(File)
That internal API returns a ShellFolder but it we expose that it would
have to
be the java.io.File superclass I expect.

-phil.









-phil.


On 10/26/2015 06:51 AM, Semyon Sadetsky wrote:
Hello,

Please review fix for JDK9:

bug: https://bugs.openjdk.java.net/browse/JDK-8081722
webrev: http://cr.openjdk.java.net/~ssadetsky/8081722/webrev.00

As the solution it is suggested to have 3 new static methods in the
javax.swing.filechooser.FileSystemView class. Those methods proxy
sun.awt.shell.ShellFolder calls and it is enough to cover NetBeans
request. getShellFolder() method is not added because it returns the
ShellFolder instance which is not for public use under the proposed
approach.
The CCC request will be rework when the fix is approved.

--Semyon






Reply via email to