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

[Alexander Scherbatiy]

It is better to have concrete methods like Image 
getFileChooserIcon(String iconName) rather to have one unified method 
for all cases.

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