<Swing Dev> Review Request for 8039966: fix doclint block tag issues in swing filechooser and colorchooser classes
Alexander Scherbatiy
alexandr.scherbatiy at oracle.com
Thu May 15 16:02:16 UTC 2014
Hi Steve,
src/share/classes/javax/swing/filechooser/FileView.java
-------------------------
/**
* A human readable description of the file. For example,
* a file named <i>jag.jpg</i> might have a description that read:
* "A JPEG image file of James Gosling's face".
+ * @return a {@code String} containing a description of the file or
+ * {@code null} if it is not available.
+ *
*/
public String getDescription(File f) {
return null;
}
-------------------------
The current javadoc for the getDescription(File f) method does not
specify the returned value if the description is not available.
It means that there can be applications that extend FileView class and
returns an empty string instead of null in this case.
If you have the reason that the return value must be null if the
description is not available
you need to create the CCC request for the API change.
Make also sure that your changes does not break the backward compatibility.
Thanks,
Alexandr.
On 4/30/2014 1:09 PM, sergey malenkov wrote:
> Now it looks OK for CCC request:
> http://ccc.us.oracle.com/docs/process
>
> Thanks,
> SAM
>
> On 30.04.2014 2:56, Steve Sides wrote:
>> Review Request for 8039966: fix doclint block tag issues in swing
>> filechooser and colorchooser classes
>>
>> A few more changes, mostly just as noted.
>> http://cr.openjdk.java.net/~ssides/8039966/8039966.5
>>
>> -steve
>>
>> On 4/25/2014 8:34 AM, sergey malenkov wrote:
>>> Hi Steve,
>>>
>>> FileSystemView:
>>>
>>> /**
>>> * Returns a File object constructed from the given path string.
>>> + *
>>> + * @param path {@code String} representation of path
>>> + * @return a {@code File} object created from the given String
>>> */
>>> public File createFileObject(String path) {
>>>
>>>
>>> I think, that "from the given {@code path}" is better than "from the
>>> given String".
>>>
>>> /**
>>> * Gets the list of shown (i.e. not hidden) files.
>>> + *
>>> + * @param dir the root directory of files to be returned
>>> + * @param useFileHiding determine if hidden files are returned
>>> + * @return an array of {@code File} objects. Include hidden
>>> files if
>>> + * {@code useFileHiding} is false.
>>> */
>>>
>>> I suggest to rewrite last sentence:
>>> @ return an array of {@code File} objects representing files and
>>> directories in the given {@code dir}.
>>> It includes hidden files if {@code useFileHiding} is {@code false}.
>>>
>>> FileView:
>>>
>>> Note, that all @return tags differ from each other.
>>> + * @return a String containing a description of the file
>>> + * @return a {@code String} containing a description of the
>>> type of the file
>>> + * @return an {@code Icon} which represents the specified
>>> {@code File}
>>> I recommend to use something like this:
>>> @return a {@code String} containing a description of the given file,
>>> or {@code null} if it is not available.
>>>
>>> /**
>>> * Whether the directory is traversable or not. This might be
>>> * useful, for example, if you want a directory to represent
>>> * a compound document and don't want the user to descend into it.
>>> + *
>>> + * @param f a {@code File} object representing a directory
>>> + * @return true if the directory is traversable
>>> */
>>> public Boolean isTraversable(File f) {
>>>
>>> Note that the method may return null. We should describe it somehow.
>>> For example,
>>> @return true if the directory is traversable,
>>> false if it is not traversable,
>>> null if the file system should be asked.
>>> @see FileSystemView#isTraversable
>>>
>>> Thanks,
>>> SAM
>>>
>>>
>>>
>>> On 24.04.2014 22:00, Steve Sides wrote:
>>>> Hi Sergey, et. all,
>>>> changes as per comments:
>>>> http://sqeweb.us.oracle.com/coretools/ssides/webrevs/8039966/8039966.5/
>>>>
>>>>
>>>> Some comments in line...
>>>>
>>>> On 4/24/2014 5:16 AM, sergey malenkov wrote:
>>>>> Hello,
>>>>>
>>>>> /**
>>>>> * The icon that represents this file in the
>>>>> <code>JFileChooser</code>.
>>>>> + *
>>>>> + * @param f a {@code File} object
>>>>> + * @return an {@code Icon} to be displayed by the file chooser
>>>>> */
>>>>> public Icon getIcon(File f) {
>>>>>
>>>>> What if the FileView class is used from another component, not the
>>>>> file chooser.
>>>>> I prefer something like this: an {@code Icon} that represent the
>>>>> specified {@code File}.
>>>> I see your point, although, this is in the 'filechooser' package
>>>> and is documented as
>>>> "an abstract class that can be implemented to provide the
>>>> filechooser with UI information for a |File|."
>>>> However, I used a return comment more akin to what you suggested.
>>>>
>>>>>
>>>>>
>>>>> /**
>>>>> * Returns all root partitions on this system. For example, on
>>>>> * Windows, this would be the "Desktop" folder, while on DOS
>>>>> this
>>>>> * would be the A: through Z: drives.
>>>>> + *
>>>>> + * @return an array of {@code File} objects
>>>>> */
>>>>> public File[] getRoots() {
>>>>>
>>>>> It is obvious that this method returns an array of File objects.
>>>>> It is better to specify these objects. For example:
>>>>>
>>>>> + * @return an array of File objects which represent root
>>>>> partitions of this file system.
>>>> got it.
>>>>>
>>>>> /**
>>>>> * Returns a File object constructed in dir from the given
>>>>> filename.
>>>>> + * If {@code dir} is {@code null}, then the new {@code File}
>>>>> instance is
>>>>> + * created as if by invoking the single-argument {@code File}
>>>>> constructor
>>>>> + * on the given {@code filename} string.
>>>>> + *
>>>>> + * @param dir an abstract pathname denoting a directory
>>>>> + * @param filename a {@code String} representation of a pathname
>>>>> + * @return a {@code File} object created from {@code dir} and
>>>>> {@code filename}
>>>>> */
>>>>> public File createFileObject(File dir, String filename) {
>>>>>
>>>>> Seems this javadoc describes an implementation details.
>>>>> We should specify here that the created File object represents a
>>>>> file with specified filename, that is located in specified folder.
>>>> I was looking into what this does and meandered on to the File doc
>>>> and kind of borrowed the javadoc idea from there, since this
>>>> does the same. I put it back to the original.
>>>>
>>>> -steve
>>>>
>>>>>
>>>>> etc...
>>>>>
>>>>>
>>>>> Note that all such changes in the API require an approval from CCC.
>>>>> This is not a small typo. You really change the *public* API!
>>>>>
>>>>> Thanks
>>>>> SAM
>>>>>
>>>>> On 18.04.2014 2:57, Steve Sides wrote:
>>>>>> Hello,
>>>>>>
>>>>>> Could you please review the fix for the following bug:
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8039966
>>>>>>
>>>>>> Webrev corresponding:
>>>>>> http://cr.openjdk.java.net/~ssides/8039966/8039966.4/
>>>>>>
>>>>>> This addresses missing @parm and @return block tags in javadoc
>>>>>> for javax/swing/colorchooser and filechooser classes as noted by
>>>>>> doclint.
>>>>>>
>>>>>> It does not address methods which are missing javadoc comments
>>>>>> completely.
>>>>>>
>>>>>> You may want to check the wording of @param for
>>>>>> (un)installChooserPanel(). The original seemed a little incorrect
>>>>>> to me.
>>>>>>
>>>>>> thanks,
>>>>>>
>>>>>> -steve
>>>>>>
>>>>>
>>>>
>>>
>>
>
More information about the swing-dev
mailing list