Community
Participate
Working Groups
M20070905-1045. The Javadoc for the JavaCore options is a generic blurb: /** * Possible configurable option ID. * @see #getDefaultOptions() * @since 3.1 */ This renders Javadoc view and hovers useless. Clients have to open JavaCore, then navigate to getDefaultOptions() and then search in the 900! lines of comment for their option. Please move (or copy) the Javadoc to their corresponding element so that clients can use the Javadoc view and the Javadoc hover to get information about JavaCore option constants.
See also bug 143427.
Exactly: the doc is already there and in good shape. The only thing which is needed is to move or copy it to the corresponding constant(s).
see bug 124093...
This is really a major pain for every user of this API. Clients should always reference the string constants and when they do this, they should also see their meaning without having to do another lookup in the supersized Javadoc of getDefaultOptions().
This bug also renders the Javadoc and the content assist help useless as it says for all JDT Core constants: Possible configurable option ID. @see #getDefaultOptions()
I agree, this is a pain. I probably initiated this a long time ago, but now there are just too many entries. It is probably time to split the doc across all the options.
(In reply to comment #6) Yipee! To avoid problems with typos as in bug 100278 comment 16, I'd suggest to - use the {@value} Javadoc tag - initialize the constants with CompilerOptions.OPTION_* The ideal solution would be to define the constants in JavaCore and let CompilerOptions refer to the API, but I guess that would be an internal layer break for the standalone compiler. Still, having manifest strings only at one spot and use field accesses everywhere else is the best way to ensure correct mappings and to ease navigation for readers of the code (using reference search or call hierarchy).
Thinking to what we could do to further improve the situation, I believe we could afford to rename the internal constants after the API ones. Or at least, to give it a try. Doing so would reduce confusion, and mitigate the inconvenience of our being compelled to have the API layer reference the internal constants. Would anyone strongly object to this? I also initially thought I could add a test that would check getDefaultOptions' doc against the constants'. But thinking more about it, the constants are grouped enough to make a linear reading of their specification easy enough. Do we want/need to have @see entries in getDefaultOptions' javadoc?
>Do we want/need to have @see entries in getDefaultOptions' javadoc? In addition to each entry: option id: "org.eclipse.jdt.core.classpath.exclusionPatterns" You could add: constant: {@link #CORE_ENABLE_CLASSPATH_EXCLUSION_PATTERNS}
I was on a more radical thought path: move options doc to constants, and decide between an almost empty javadoc for getDefaultOptions, and the same plus @see tags. If I get you well, you would prefer a more verbose getDefaultOptions javadoc (something like the one we have, plus links). My bet is that, once each constant is properly described, there is no need for too verbose a getDefaultOptions javadoc any more. So amongst the three following options, my preference would go to 1 or 2: 1 - simplistic getDefaultOptions javadoc; 2 - 1 plus links to constants; 3 - current getDefaultOptions javadoc, augmented with links to the relevant constants. What about trying 2 and seeing how it fares?
>If I get you well, you would prefer a more verbose getDefaultOptions >javadoc (something like the one we have, plus links). Nope, I also prefer the radical solution but was not aware you're going that far ;-)
(In reply to comment #10) I'd vote for > 1 - simplistic getDefaultOptions javadoc; , but > 2 - 1 plus links to constants; would also be fine with me. > 3 - current getDefaultOptions javadoc, augmented with links to the relevant > constants. That would be hard to manage and keep in sync, and I don't see a real use case for this. You could also add an @category tag to each COMPILER_ constant and refer to that tag in the Javadoc of getDefaultOptions(). This would implicitly define all options that are returned in the Hashtable and avoid any duplicated definitions.
Issues/notes: - now see that @throws, @deprecated, @Deprecated (others?) get interpreted by JDT UI when between <pre> and </pre> (but are not by javadoc itself); need to check if a bug is open for this; don't quite see any workaround for now (except dropping the @ sign); at least one entry made the choice to reference @throws using "A 'throws' tag"; may want to follow that pattern consistently; - @category is not recognized by javadoc (running with default config); a side effect is that refering to the categories in getDefaultOptions doc introduces kind of an unreferenced link; must improve that (keeping categories if possible - they are kind of neat in Eclipse); - BTW, masking the uncategorized members on Linux with I20071218-0800 simply drops the class from the Outline view; need to check on latest; - I do not think that @value would be enough for IDs; we want the literal value to be readable in the code and in the javadoc; may use it for option values, since those get their literals defined in place; for IDs, I'll craft a white box test to check that values match the docs; - the use of CompilerOptions values for initialization is the subject of bug 216277, I won't address this here; - found a constant that had to be deprecated, see bug 216571; - some constants documentation may need to be re-rewritten to become more meaningful; this goes far beyond this bug though, and I won't undertake a formal review for it; - fixed at least one unbalanced pair of {} in possible values specs; - since we use <pre> tags, we should keep the number of columns within reasonable limits; not addressed here; - quite a few @since tags are missing; not addressed here; probably needs that a bug be opened; - will probably change cross-options reference from the use of literals to the use of {@link}; this made little sense within the getDefaultOptions doc bag, but would be the best solution in the reorganized doc; - the hosting of builder options by the java core options set must pertain to history and might need a second look; not addressed here; - may want to craft a test that checks that the documented default value matches the real one; - use of <br> vs <p>; - use of <br> or <p> within <pre>; - since the first sentence of the javadoc is presented in a table at the top of the class documentation, may want to collapse the first lines for each constant into something meaningful.
Created attachment 87857 [details] Fix - wip: moved the docs
(In reply to comment #13) > Issues/notes: ... > - BTW, masking the uncategorized members on Linux with I20071218-0800 simply > drops the class from the Outline view; need to check on latest; According to bug 153406, this is an expected behavior and the cure would be to add the categories we want to the class itself. Will make a try and see how I like it.
(In reply to comment #13) > - now see that @throws, @deprecated, @Deprecated (others?) get interpreted by > JDT UI when between <pre> and </pre> (but are not by javadoc itself); need to > check if a bug is open for this; don't quite see any workaround for now Limitation of the syntax coloring for Javadoc tags (JDT/Text). AST looks fine. A workaround could be to replace '@' with '@', but that's ugly in source. > - @category is not recognized by javadoc (running with default config); a side > effect is that refering to the categories in getDefaultOptions doc introduces > kind of an unreferenced link; must improve that (keeping categories if possible > - they are kind of neat in Eclipse); @category is not (yet?) an official tag: http://java.sun.com/j2se/javadoc/proposed-tags.html I don't think a link in getDefaultOptions() would make much sense (would have to link to all occurrences of the respective @category...). Referring to them in plain text is a good solution. > - I do not think that @value would be enough for IDs; we want the literal value > to be readable in the code and in the javadoc; Constant value is available in Javadoc hover in 3.4 (at the end of the bold header). I'm working on support for automatically replacing {@value} inside the Javadoc with the value of the referenced constant. +1 for the rest of your comments. I would replace the <pre> with normal text and HTML definition lists.
(In reply to comment #16) > (In reply to comment #13) > > > - now see that @throws, @deprecated, @Deprecated (others?) get interpreted by > > JDT UI when between <pre> and </pre> (but are not by javadoc itself); need to > > check if a bug is open for this; don't quite see any workaround for now > Limitation of the syntax coloring for Javadoc tags (JDT/Text). AST looks fine. > A workaround could be to replace '@' with '@', but that's ugly in source. A search on '<pre> hover' only returned bug 40596 which I believe is unrelated. Any bug open for that? If non, I'll open one and will anchor a fup bug for us on it, and I'll move our current @xxx tags to "tag 'xxx'", unless the definition list trick helps in some way. > > - @category is not recognized by javadoc (running with default config); a side > > effect is that refering to the categories in getDefaultOptions doc introduces > > kind of an unreferenced link; must improve that (keeping categories if possible > > - they are kind of neat in Eclipse); > @category is not (yet?) an official tag: > http://java.sun.com/j2se/javadoc/proposed-tags.html Knew it. My intent was to collect advantages and drawbacks of using the @category pattern, and the fact that the generated javadoc does not leverage it as of today is a limitation, since we have to find another way to tell the reader where to look for relevant constants, and that very limitation becomes a drawback if you consider that whatever means we use to improve the javadoc as needed, there will be duplication between the @category values and the other classification device. The question left is: does the @category annotation bring enough value in the context of the IDE to outbalance the information duplication issue? > I don't think a link in getDefaultOptions() would make much sense (would have > to link to all occurrences of the respective @category...). Referring to them > in plain text is a good solution. ack > > - I do not think that @value would be enough for IDs; we want the literal value > > to be readable in the code and in the javadoc; > Constant value is available in Javadoc hover in 3.4 (at the end of the bold > header). I'm working on support for automatically replacing {@value} inside the > Javadoc with the value of the referenced constant. Still. For constants which literal value is available in the code, @value will make perfect sense once its support is complete. This would be the case for option values. For constants which initialization points to another constant, which will be the case for option IDs if we solve bug 216277 by leveraging the language and using CompilerOptions constants, the fact that the javadoc displays the value is invaluable. > > +1 for the rest of your comments. ack > I would replace the <pre> with normal text > and HTML definition lists. Will try and consider the results.
(In reply to comment #15) > (In reply to comment #13) > > Issues/notes: > ... > > - BTW, masking the uncategorized members on Linux with I20071218-0800 simply > > drops the class from the Outline view; need to check on latest; > According to bug 153406, this is an expected behavior and the cure would be to > add the categories we want to the class itself. Will make a try and see how I > like it. Thinking more about it, I came to consider that this was not the way to go. @category is supposed to help classify *members* of a class, which makes using it on a top-level class irrelevant. Markus reopened bug 153406 before I attempted to. I'll proceed on this bug as if bug 153406 was to be fixed.
> I'll proceed on this bug as if bug 153406 was to be fixed. Sorry but we won't work on that bug.
(In reply to comment #19) > > I'll proceed on this bug as if bug 153406 was to be fixed. > Sorry but we won't work on that bug. My impression from the reading of bug 153406 and related ones was that you considered making the use of categories in the outline view smoother in the case where the top-level class would not bear any @category tag itself, somewhat getting closer to the semantics of the @category tag. Seems I got it wrong. Do you confirm that, when only category 'visible' is checked in its filter, the outline view will show f for listing 1 but not listing 2, and that you plan to change nothing to this? // listing 1 /** @category visible */ public class X { /** @category visible */ public int f; } // listing 2 public class X { /** @category visible */ public int f; } If so, is there any existing or planned combination of options that would make the use of listing 2 desirable in any ways? I definitely do not want to go for listing 1.
>the outline view will show f for listing 1 but not listing 2, and that >you plan to change nothing to this? Right - at least not for 3.4. Of course if you go into the type (Go Into Top Level Type) then f will be shown for both listings.
(In reply to comment #17) > (In reply to comment #16) > > (In reply to comment #13) > > > > > - now see that @throws, @deprecated, @Deprecated (others?) get interpreted by > > > JDT UI when between <pre> and </pre> (but are not by javadoc itself); need to > > > check if a bug is open for this; don't quite see any workaround for now > > Limitation of the syntax coloring for Javadoc tags (JDT/Text). AST looks fine. > > A workaround could be to replace '@' with '@', but that's ugly in source. > A search on '<pre> hover' only returned bug 40596 which I believe is unrelated. > Any bug open for that? If non, I'll open one and will anchor a fup bug for us > on it, and I'll move our current @xxx tags to "tag 'xxx'", unless the > definition list trick helps in some way. Using <code> seems to work in all situations, aka <code>@deprecated</code>.
(In reply to comment #21) > >the outline view will show f for listing 1 but not listing 2, and that > >you plan to change nothing to this? > Right - at least not for 3.4. Of course if you go into the type (Go Into Top > Level Type) then f will be shown for both listings. Tested that and looks neat. Suspect some users will be able to get the right options set in place, hence I'll keep categories.
Created attachment 88007 [details] Fix wip This new proposal does the following: - used definition lists in place of pre-formatted comments enclosed within <pre> tags; - used <code> tags to neutralized unwanted side effects on javadoc tags used in descriptions (for example 'when encountering a <code>@param</code>', which wants to discuss about the said tag, not to insert such a tag); - used @link when referring a constant from another constant; - used @value tags in option values descriptions; - replaced the use of <br> by the use of <p>; - (loosely) referenced the categories from options getters and setters; since @category is not supported yet, I preferred to use a loose link than getting the reader of the javadoc confused; users of the IDE should be able to mentally close the gap between the categories as documented in the text (aka 'Compiler option ID') and in category tags (aka 'CompilerOptionID'); note that using white spaces in @category specifies as many categories as words, which is not what we want, whereas the use of the CompilerOptionID format is the text would look odd; - improved the rendering into the javadoc headers by collapsing the first lines of description (the summary table now has entries like ' CODEASSIST_ARGUMENT_SUFFIXES Code assist option ID: Define the Suffixes for Argument Name.'. Still have to check again the results and to craft a couple of white-box tests that would cross check the docs and the real values.
(In reply to comment #24) Nice. I would also use inline @links for possible values, e.g.: * <dt>Possible values:</dt><dd>{{@link #GENERATE}, {@link #DO_NOT_GENERATE}}</dd> * <dt>Default:</dt><dd>{@link #GENERATE}</dd> Clients don't really care about the concrete value and they always refer to the OptionValue constants if they are sane. In #getDefaultOptions(), I would just use the category identifiers: * (categorized in CodeAssistOptionID, CompilerOptionID and CoreOptionID)
(In reply to comment #25) > (In reply to comment #24) > > Nice. thx > I would also use inline @links for possible values, e.g.: > * <dt>Possible values:</dt><dd>{{@link #GENERATE}, {@link > #DO_NOT_GENERATE}}</dd> > * <dt>Default:</dt><dd>{@link #GENERATE}</dd> > > Clients don't really care about the concrete value and they always refer to the > OptionValue constants if they are sane. I won't do that in 3.4 M5 in any case (we're supposed to freeze soon). You may want to open a fup bug (not committing to change anything though ;-)). > In #getDefaultOptions(), I would just use the category identifiers: > * (categorized in CodeAssistOptionID, CompilerOptionID and CoreOptionID) Since @category is not leveraged by javadoc, and I need a way to talk about these categories even in the context of the generated javadoc, I preferred to 'refer loosely' to categories (see discussion in comment #24).
Created attachment 88294 [details] Fix + test case This proposal adds a white-box test that checks ids and default values correlation between the javadoc and the real code. It also aligns the javadoc, that suffered a bit during the previous steps. Kent, would you please let me know what you think (M5 is coming soon and I'd like to release at least a first set of improvements)? Note that I have not run the tests other than the APIDocumentationTests yet. My changes should not affect the code, hence it should be OK. Launching them straight away.
Looks good. Some of the tabs seem to be off in the patch, but assuming that doesn't affect the UI presentation then it shouldn't matter.
Thx. I'll release this today then, invalidating the test though (I do not know of our having similar tests yet, and I believe that getting it to run significantly or to get skipped gracefully on releng while running in local will need some tinkering - and may break a nightly build or two. I'll do that in early M6).
Created attachment 88382 [details] Fix + test case Same with: - HEAD catchup; - tabs instead of leading white spaces in docs; - invalidated API doc test (need to refine this after M5, opened fup bug 217223 to this effect;
Released for 3.4M5.
Verified in the code for 3.4M5 using build I20080204-0010
