Community
Participate
Working Groups
The APIs and extension point added in the bug 183164 need to be integrated into Eclipse help. Also we should consider adding an overview article (into Platform plug-in developer guide -> Programmer's guide ?) describing new Bidi functionality.
I added links for the items generated from Javadoc and schema: http://git.eclipse.org/c/platform/eclipse.platform.common.git/commit/?id=cdabafc570b9e2d7ce4e092864f484f1a4156049
The changes are in the recent builds and looks good for the first cut. Mati, now that we have a way to see them in their proper format, could you go other them to make sure they are as expected? The easiest way to see pages of interest: open Eclipse help in this week's buils (Nov 21 or later) and search for "equinox bidi". I did a very quick look, not really going into contents: => org.eclipse.equnix.bidi package file: "A number of standard type handlers are supplied in the associated package org.eclipse.equinox.bidi.internal.consumable." and "There are two other packages associated with the current one: •org.eclipse.equinox.bidi.internal •org.eclipse.equinox.bidi.internal.consumable" We must not refer to internal class in the API docs. Also, link's formatting seems to be off. => "This package provides classes for processing structured text." The "structured text" might need an explanation - or may be avoid the term altogether at this level? => Package files for org.eclipse.equinox.bidi.custom, org.eclipse.equinox.bidi.advanced - the package files - at the top: "See Description" appears at the top -> that is not needed => Package file org.eclipse.equinox.bidi.custom "This package provides classes for developing structured text type handlers." This does not seem like it provides much information. How about something like: "APIs in this package allow plug-in developers contribute handlers for spefic types of bi-directional expressions not supported in the standard distribution". => "It is to be used together with package org.eclipse.equinox.bidi." This does not seem to be necessary ===== STextProcessor docs - needs quick explanation of the "structured text", "lean", and "full" text". Better yet to avoid introducing those terms at all at this level ==== ISTextExpert "Advanced API for processing structured text. For a general introduction to structured text, see the org.eclipse.equinox.bidi package documentation." That does not convey the purpose of this method. Think in terms of answering a question: "Why would I need to know about it?" "In such cases, the state must be managed by a ISTextExpert instance obtained with the getStateful method." Link to "getStateful" does not seem to be correct Same for "These values can only be used in setState calls" ==== STextEnvironment For some reason has Javadoc for hashCode() and equals(). Usually description from java.lang.Object suffices; the class Javadoc can mention some specifics on when objects are equal if that is important to the user (which does not seem to be in this case?). ==== STextExpertFactory Those links do not seem to be properly formatted: "•stateful, obtained by calling getStatefulExpert." "•not stateful, obtained by calling getExpert." "In other words, the methods getState, setState and clearState of ISTextExpert" "in which case the DEFAULT environment " ===== STextOffsets Does not seems to have Javadoc for constructor. (Just placing "Constructor" or "Default constructor" in the Javadoc will be fine if there is nothing interesting to describe there.) ===== STextTypeHandler Links not formatted as expected. ======================================================================= Again, I am pleasantly surprised at how well this first cut of formatted Javadoc looks like. It usually takes a number of iterations to get it to this stage. Thanks Mati!
I will reply to Comment #2 separately for each file mentioned within it. After completing all the answers, I will submit a patch with all the fixes.
(In reply to comment #2) > > => org.eclipse.equnix.bidi package file: > > "A number of standard type handlers are supplied in the associated package > org.eclipse.equinox.bidi.internal.consumable." Will be replaced by: <new text> A number of standard type handlers are supplied with this package, <end of new text> > > and > > "There are two other packages associated with the current one: > •org.eclipse.equinox.bidi.internal > •org.eclipse.equinox.bidi.internal.consumable" These lines and the following paragraphs until the end of the section will be replaced by: <new text> The source code of packages org.eclipse.equinox.bidi.* can serve as example of how to develop processors for currently unsupported types of structured text. However, users wishing to process the currently supported types of structured text typically don't need to deal with the org.eclipse.equinox.bidi.custom package. <end of new text> > > We must not refer to internal class in the API docs. Also, link's formatting > seems to be off. > I am not sure what you is meant by "link's formatting seems to be off". Please clarify.
(In reply to comment #2) > > => org.eclipse.equnix.bidi package file: > > => "This package provides classes for processing structured text." > The "structured text" might need an explanation - or may be avoid the term > altogether at this level? > Will be replaced by: <new text> This package provides classes for processing bidirectional text with a special structure to ensure its proper presentation. <end of new text>
(In reply to comment #2) > > => Package files for org.eclipse.equinox.bidi.custom, > org.eclipse.equinox.bidi.advanced - the package files - at the top: > > "See Description" appears at the top -> that is not needed > I agree, but I don't know what I can do about it. This text is not part of the package HTML file, it is added automatically by the javadoc generator. All other packages in Eclipse seem to have the same "See Description" lines.
(In reply to comment #2) > > => Package file org.eclipse.equinox.bidi.custom > > "This package provides classes for developing structured text type handlers." > > This does not seem like it provides much information. How about something like: > "APIs in this package allow plug-in developers contribute handlers for spefic > types of bi-directional expressions not supported in the standard > distribution". The current text will be replaced by: <new text> This package provides APIs for plug-in developers to contribute handlers for specific types of bidirectional expressions not supported in the standard distribution. <end of new text> > > => "It is to be used together with package org.eclipse.equinox.bidi." > > This does not seem to be necessary > Will be removed.
(In reply to comment #2) > > STextProcessor docs > - needs quick explanation of the "structured text", "lean", and "full" text". > Better yet to avoid introducing those terms at all at this level > The class description will be replaced as follows: <new text> Provides methods to process bidirectional text with a specific structure. The methods in this class are the most straightforward way to add directional formatting characters to the source text to ensure correct presentation, or to remove those characters to restore the original text (for more explanations, please see the package documentation). <end of new text>
(In reply to comment #2) > > ISTextExpert > "Advanced API for processing structured text. For a general introduction to > structured text, see the org.eclipse.equinox.bidi package documentation." > That does not convey the purpose of this method. Think in terms of answering a > question: "Why would I need to know about it?" This text will be replaced by the following: <new text> Provides advanced methods for processing bidirectional text with a specific structure to ensure proper presentation. For a general introduction to structured text, see the org.eclipse.equinox.bidi package documentation. For details when the advanced methods are needed, see this package documentation. <end of new text> > > "In such cases, the state must be managed by a ISTextExpert instance obtained > with the getStateful method." > > Link to "getStateful" does not seem to be correct > Good catch: the text should say "getStatefulExpert method". > Same for "These values can only be used in setState calls" > I don't see the problem here. Can you explain why you think it is wrong?
(In reply to comment #2) > > STextEnvironment > > For some reason has Javadoc for hashCode() and equals(). Usually description > from java.lang.Object suffices; the class Javadoc can mention some specifics on > when objects are equal if that is important to the user (which does not seem to > be in this case?). > Since these are public methods, there will be some javadoc text for them anyway. I think it is better to have text specific to this implementation rather than the standard text from the Object class, which really will not teach anything we don't know.
(In reply to comment #2) > > STextExpertFactory > > Those links do not seem to be properly formatted: > "•stateful, obtained by calling getStatefulExpert." > "•not stateful, obtained by calling getExpert." > "In other words, the methods getState, setState and clearState of ISTextExpert" > "in which case the DEFAULT environment " > What do you mean "Those links do not seem to be properly formatted"? Is it that expect to see the list of arguments together with the method names? IMHO, this makes the text less readable without any practical benefit. If you insist, I will let the argument list be displayed, but I would rather not.
(In reply to comment #2) > > STextOffsets > > Does not seems to have Javadoc for constructor. (Just placing "Constructor" or > "Default constructor" in the Javadoc will be fine if there is nothing > interesting to describe there.) > I will add "Default constructor".
(In reply to comment #2) > > STextTypeHandler > > Links not formatted as expected. > What do you mean "Links not formatted as expected"? Is it that expect to see the list of arguments together with the method names? IMHO, this makes the text less readable without any practical benefit, especially in this class where some methods have quite long argument lists. If you insist, I will let the argument list be displayed, but I would rather not.
(In reply to comment #3) > I will reply to Comment #2 separately for each file mentioned within it. After > completing all the answers, I will submit a patch with all the fixes. Thank you! (In reply to comment #4) > I am not sure what you is meant by "link's formatting seems to be off". Please > clarify. See how generated help picks up links to class members and methods and formats them? Some of the links are formatted differently, probably because Javadoc attempted to provide some formatting for them? If this is not clear, don't mind those, I can fix this later. (In reply to comment #5) That sounds good. (In reply to comment #6) > I agree, but I don't know what I can do about it. This text is not part of the > package HTML file, it is added automatically by the javadoc generator. All > other packages in Eclipse seem to have the same "See Description" lines. I see. I'll try to figure this out later, just skip it for now. (In reply to comment #7) Sounds good. (In reply to comment #8) Sounds good. (In reply to comment #9) > I don't see the problem here. Can you explain why you think it is wrong? This is similar to the link formatting I mentioned in the comment 4. If there is uncertainty about it, let's skip it, this can be adjusted later. (In reply to comment #10) > Since these are public methods, there will be some javadoc text for them > anyway. I think it is better to have text specific to this implementation > rather than the standard text from the Object class, which really will not > teach anything we don't know. Methods like this are usually gets "non-Javadoc" comments. It is probably too much information to describe what they do internally. I'd just use the standard header of: /* * (non-Javadoc) * * @see org.eclipse.... */ (In reply to comment #11) > What do you mean "Those links do not seem to be properly formatted"? This is similar to the comment 4, the generated help pages format "implied" links in certain way which seems to be different from the "explicit" links in Javadoc. Again, if this is unclear, let's skip it; this is a mechanical change that can be done later. (In reply to comment #12) Sounds good. (In reply to comment #13) > What do you mean "Links not formatted as expected"? Again, this is the same as the comment 4 and let's skip it if it is unclear. Hope this helps. Thank you and looking forward for the patch!
Created attachment 208393 [details] Javadoc updates, no changes in executing code This patch addresses all javadoc comments in comment #2 up to comment #14.
(In reply to comment #15) > Created attachment 208393 [details] > Javadoc updates, no changes in executing code > > This patch addresses all javadoc comments in comment #2 up to comment #14. Thanks Mati, patch applied to Git master. http://git.eclipse.org/c/equinox/rt.equinox.bundles.git/commit/?id=4698e4c5d201111fbdcfe95a5cc211bdcb002806
