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