<Swing Dev> [9] Review Request for 8081722: Provide public API for file hierarchy provided by sun.awt.shell.ShellFolder

[Semyon Sadetsky]

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
>>
>
>