<Swing Dev> Review Request for 8039966: fix doclint block tag issues in swing filechooser and colorchooser classes

[Steve Sides]

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