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

[Semyon Sadetsky]

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