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

[sergey malenkov]

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