Community
Participate
Working Groups
Created attachment 99431 [details] Simple CU with big Javadoc Build ID: I20080502-0100 Steps To Reproduce: 1. Open some compilation unit, with many JavaDoc comments, formatted in 3.4M6; 2. Format full compilation unit; 3. Notice that look of JavaDoc comments is now very different. More information: I see following problems: 1. for @param two tabs are used, but location of first "tab" is so that next tab position is just 1 character, so first "tab" looks as just space. 2. Same problem for wrapping @return - single "tab", that looks as space. 3. {@link ITypeBinding} can now be wrapped in the middle, so "{@link" stays on one line, and "ITypeBinding}" - on next line. This saves some space, but hurts readability. Now questions for these problems: 1. May be add one more "space" after "* ", so first tab will be rendered as 4 spaces and provide enough indent? 2. Same for @return - one more "space" after "* ". 3. I'm not sure, may be space is more important, but may be make wrapping behavior configurable? 4. In general, I find all these changes very annoying, because I have big codebase, formatted with old style, and use formatting as "Clean Up" step, so now I see massive changes in CVS in each compilation unit that I touch. :-(
Created attachment 99433 [details] Current look of JavaDoc
Created attachment 99434 [details] Space added for first @param and wrapped @return
I'll investigate
(In reply to comment #0) > > More information: > I see following problems: > > 1. for @param two tabs are used, but location of first "tab" is so that next > tab position is just 1 character, so first "tab" looks as just space. > The output is like that because your in your preferences you use TABS only and tab length is 4. So, I would say that it works as specified in your preferences. If you do not like this output, play with your preferences to change it: you can either use spaces instead of tabs or increase the tabs length... > 2. Same problem for wrapping @return - single "tab", that looks as space. > So, same answer. > 3. {@link ITypeBinding} can now be wrapped in the middle, so "{@link" stays on > one line, and "ITypeBinding}" - on next line. This saves some space, but hurts > readability. > There's no specification whether inline tags could be wrapped or not while formatting a javadoc comment. Note that this depends on the max length you set in your preferences for the comment line. Using the Eclipse build-int default, none of the inline tags are split in two lines in your example. Note also that old comment formatter also split inline tags; that means we didn't change the formatter behavior on this point... However, I agree this could be configurable as it effectively makes the comment a little bit less readable as the end of the inline tag is not painted when split on 2 lines. So, do you agree to keep this bug open as a requirement to make wrapping inside inline tags configurable?
(In reply to comment #4) > (In reply to comment #0) > > > > More information: > > I see following problems: > > > > 1. for @param two tabs are used, but location of first "tab" is so that next > > tab position is just 1 character, so first "tab" looks as just space. > > > The output is like that because your in your preferences you use TABS only and > tab length is 4. So, I would say that it works as specified in your > preferences. If you do not like this output, play with your preferences to > change it: you can either use spaces instead of tabs or increase the tabs > length... Yes, yes, I know that this is how tabs are rendered. What was reason to change JavaDoc formatting/look? Is there any bug report that I can read to see motivation? Even if I use spaces, visual indentation is still almost same - less than it was in 3.4M6. And how can I work now, when I have big code base with old style, and now JDT re-formats my code in absolutely different way, without any chance to configure for using old style? Such changes shake my faith in using JDT for long running projects. :-( > > 3. {@link ITypeBinding} can now be wrapped in the middle, so "{@link" stays on > > one line, and "ITypeBinding}" - on next line. This saves some space, but hurts > > readability. > > > There's no specification whether inline tags could be wrapped or not while > formatting a javadoc comment. Note that this depends on the max length you set > in your preferences for the comment line. Using the Eclipse build-int default, > none of the inline tags are split in two lines in your example. Note also that Yes, because I've specially added "zzzzz yy" to shift @link into position when it will be splitted. I don't speak about exact text, but about situation in general. > old comment formatter also split inline tags; that means we didn't change the > formatter behavior on this point... However, I agree this could be configurable > as it effectively makes the comment a little bit less readable as the end of > the inline tag is not painted when split on 2 lines. > > So, do you agree to keep this bug open as a requirement to make wrapping inside > inline tags configurable? > I would prefer to make JavaDoc configurable enough to allow me use any style, including old one. New style is worse than old one in any case, so should be fixed.
(In reply to comment #5) > Yes, yes, I know that this is how tabs are rendered. > What was reason to change JavaDoc formatting/look? Is there any bug report > that I can read to see motivation? > Even if I use spaces, visual indentation is still almost same - less than it > was in 3.4M6. > > And how can I work now, when I have big code base with old style, and now JDT > re-formats my code in absolutely different way, without any chance to configure > for using old style? Such changes shake my faith in using JDT for long running > projects. :-( > The origin of this change was bug 102780. We need to rewrite entirely the comments formatter to make it in only one pass instead of two in 3.3 version. Doing this we get a gain of approximatively 30% on formatting compilation units which seems to be a reasonable effect for such a change and perhaps will bring you some faith in JDT long running projects. We did lot of tests for this new formatter (we currently formatting around 40,0000 units) and all known clients seemed to be happy with this new version but, of course, we guess that it may still have some troubles in it. In your case, it seems that the change causing your grief is the usage of tabulations to indent tags description. My formatter understanding was that all indentation should be done using the TAB character specified in the preferences. Discovering that the old formatter didn't respect this, I thought it was an issue in the old formatter and so, fixed it while rewritting it. However, if you definitely think that this new behavior is a bug or that user should be able to retrieve this old behavior, I'm okay to discuss about that and try to find a solution which will make everybody happy. CC'ed Olivier and Dani to have their feeling about this and perhaps some feedback on the old formatter behavior? > > Yes, because I've specially added "zzzzz yy" to shift @link into position > when it will be splitted. I don't speak about exact text, but about situation > in general. > > > > old comment formatter also split inline tags; that means we didn't change the > > formatter behavior on this point... However, I agree this could be configurable > > as it effectively makes the comment a little bit less readable as the end of > > the inline tag is not painted when split on 2 lines. > > > > So, do you agree to keep this bug open as a requirement to make wrapping inside > > inline tags configurable? > > > > I would prefer to make JavaDoc configurable enough to allow me use any style, > including old one. New style is worse than old one in any case, so should be > fixed. > I understand, but as I said in comment 4, the old formatter already did the same, so I think your grief is only on the first point (tabulations inside tags description instead spaces as the old formatter did)
(In reply to comment #6) > > And how can I work now, when I have big code base with old style, and now JDT > > re-formats my code in absolutely different way, without any chance to configure > > for using old style? Such changes shake my faith in using JDT for long running > > projects. :-( > > > The origin of this change was bug 102780. We need to rewrite entirely the > comments formatter to make it in only one pass instead of two in 3.3 version. > Doing this we get a gain of approximatively 30% on formatting compilation units > which seems to be a reasonable effect for such a change and perhaps will bring > you some faith in JDT long running projects. > > We did lot of tests for this new formatter (we currently formatting around > 40,0000 units) and all known clients seemed to be happy with this new version > but, of course, we guess that it may still have some troubles in it. Of course I sure that process of developing JDT is very good, uses tests, and JavaDoc formatting should be integrated into general formatting. This is all good. > > In your case, it seems that the change causing your grief is the usage of > tabulations to indent tags description. My formatter understanding was that all > indentation should be done using the TAB character specified in the > preferences. Discovering that the old formatter didn't respect this, I thought > it was an issue in the old formatter and so, fixed it while rewritting it. > > However, if you definitely think that this new behavior is a bug or that user > should be able to retrieve this old behavior, I'm okay to discuss about that > and try to find a solution which will make everybody happy. > > CC'ed Olivier and Dani to have their feeling about this and perhaps some > feedback on the old formatter behavior? My grief has two points: 1. changes in formatting during life of project. This makes harder compare then compilation unit with its history, because real changes will be saturated by changes caused by formatting. May be this is not so important for most people, and I'm too sensitive for this... 2. in 4 spaces per tab mode current formatting looks less readable. From some point of view, there are no problem here: you use two tabs for @param and one tab for wrapping @return. But only if you look on source as on sequence of characters. Once you look on it it editor, you will see that first of these tabs is not used. Well, just unfortunate effect of using 4 spaces for tab, in 8 spaces per tab mode things look not so bad. But I think that many people use 4 spaces. I'm not sure what to do this. Tabs are good, no problem. But what is more important IMHO is that indentation should be reasonable, not 1 visual space, as it is now for @return. But using always more tabs is not good idea, because this will hurt people with more spaces per tab... Hm... may be option to use spaces for indentation? I mean only text for @param, wrapped @return and may be other tags. JavaDoc itself and Java source still should be formatted using tabs. This will solve most of (1) and is also reasonable solution for (2). > > > > > Yes, because I've specially added "zzzzz yy" to shift @link into position > > when it will be splitted. I don't speak about exact text, but about situation > > in general. > > > > > > > old comment formatter also split inline tags; that means we didn't change the > > > formatter behavior on this point... However, I agree this could be configurable > > > as it effectively makes the comment a little bit less readable as the end of > > > the inline tag is not painted when split on 2 lines. > > > > > > So, do you agree to keep this bug open as a requirement to make wrapping inside > > > inline tags configurable? > > > > > > > I would prefer to make JavaDoc configurable enough to allow me use any style, > > including old one. New style is worse than old one in any case, so should be > > fixed. > > > I understand, but as I said in comment 4, the old formatter already did the > same, so I think your grief is only on the first point (tabulations inside tags > description instead spaces as the old formatter did) > Do you mean wrapping/splitting @link? I just checked with 3.3.2, @link always wrapped as single word, without splitting on two lines.
I'd would vote to preserve space use for formatting comments in 3.4 (opening a feature request for allowing use of tabs in comments - which is always arguable on right hand side of a '*'). On inline tags splitting, maybe it should not split in between @link and the actual reference. It could split beyond that point, as I understand there could be arbitrary text after that point. Still unclear to me: what if the reference in inline tag is super long ? Can you still split the reference itself ?
Created attachment 99444 [details] Old formatter output It seems I was wrong saying that the old formatter split the inline tags. So, I will revert these 2 behavior to the old ones: 1) do not use tabs for description indentation 2) do not split inline tags between tag and reference I will also study more carefully whether the old formatter split inline tags after the reference or not (e.g. in the inline tag text) and also if it allows to split long qualified references or keep them in a single line and so print them over the comment line max length.
>And how can I work now, when I have big code base with old style, and now JDT >re-formats my code in absolutely different way, without any chance to configure >for using old style? Such changes shake my faith in using JDT for long running >projects. :-( If good argued diffs remain once this bug is finished, then you still have the possibility to reformat everything and check it in again in on pass.
It looks like I was completely wrong with description indentation for javadoc tags. I have to apologize about this and I finally agree to have seriously broken the comment formatter in this area. Here are the rules for these indentation: 1) the indentations are always done with spaces 2) the first indentation must put the description line on the same position than the beginning of the tag reference or first line description if the tag has no reference. 3) The second indentation for param tags must be calculated from the previous indentation position as: previous indentation + tab size As a consequence, the following simple example: public class X { /** * @param str the param description on 2 lines. * * @return the return description which is also on 2 lines */ public void foo() { } } must be formatted as follow using Eclipse built-in + max length = 40: public class X231263b { /** * @param str *............the param description *............on 2 lines. * * @return the return description *.........which is also on 2 lines */ public void foo() { } } (I used points for spaces to make it clearer)
As there are two separated issues, I prefer split this bug in 2. This will make fixes and reviews easier to do. I keep this one for the indentation issue, hence the summary change, and I will open a new open for inline tag unexpected splitting...
Created attachment 99640 [details] Proposed patch Finally it was not possible to make two separated patches, so this patch also include the fix for bug 231297. The patch is quite big because all existing tests of FormatterCommentTests were impacted by this change. I change the output of the formatter for all the test suite tests and took by default the old formatter output. Each test which output was modified from the old formatter has a comment explaining the reason(s) of the change (usually an obvious incorrect behavior of the old formatter). I also added some TODOs to decide whether the new behavior is justified or not. I'll bundle these questions and send a mail on jdt-core-dev mailing list and newsgroup to warn the community of the changes and get an agreement on the new ones.
Jerome, can you please review? Sorry, I know this it is again a really big patch but definitely safe as it does not introduce any new failure on Eclipse 3.0 full source workspace and Ganymede M5 one (respectively 0 and 17 failures).
Created attachment 99907 [details] New proposed patch This new patch fixes a potential issue with length of immutable tags.
org.eclipse.jdt.internal.formatter.Scribe.printJavadocBlock(FormatJavadocBlock): - should set the 'commentIndentation' field to null if commentIndentationLevel==0 - the edit to insert comment indentation could be extracted from printJavadocText() org.eclipse.jdt.internal.formatter.Scribe.printJavadocHtmlTag(FormatJavadocText, FormatJavadocBlock, boolean) - shouldn't the 'Number of empty lines to preserve' be applied inside a <pre> tag (instead of applying the 'Remove blank lines' option) ? org.eclipse.jdt.internal.formatter.Scribe.printJavadocTextLine(StringBuffer, int, int, FormatJavadocBlock, boolean, boolean, boolean) - condition for "token != TerminalTokens.TokenNameAT" is always true since the token is a white space
Created attachment 99962 [details] Last patch including 2 of the remarks of comment 16
I should have said that no regression was introduced by the last changes, I still get the same numbers of failures (0/17)...
(In reply to comment #17) > Created an attachment (id=99962) [details] > Last patch including all the remarks of comment 16 > Sorry, except: - the edit to insert comment indentation could be extracted from printJavadocText()
Comment on attachment 99962 [details] Last patch including 2 of the remarks of comment 16 I also didn't change code for the remark: "- shouldn't the 'Number of empty lines to preserve' be applied inside a <pre> tag (instead of applying the 'Remove blank lines' option) ?" => Obviously change the patch description...
I'll investigate the 2 remaining remarks post 3.4.0 as they are definitely not critical.
Reviewed patch with Frederic: +1
Released for 3.4RC1 in HEAD stream.
Created attachment 99991 [details] JDT/UI patch As some changes have been reverted, CleanUpStressTests need to be updated as well...
Committed JDT UI part. for I20080513-2000.
Verified for 3.4RC1 using build I20080513-2000
