Bugzilla – Full Text Bug Listing
| Summary: | [formatter] Count comment width from starting position | ||
|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | Jim <jimkoke2> |
| Component: | Core | Assignee: | Mateusz Matela <mateusz.matela> |
| Status: | VERIFIED FIXED | QA Contact: | Ayushman Jain <amj87.iitr> |
| Severity: | enhancement | ||
| Priority: | P3 | CC: | amj87.iitr, daniel_megert, gautier.desaintmartinlacaze, jarthana, jimkoke2, manoj.palat, mateusz.matela |
| Version: | 3.8 | ||
| Target Milestone: | 4.7 M6 | ||
| Hardware: | All | ||
| OS: | All | ||
| See Also: |
https://bugs.eclipse.org/bugs/show_bug.cgi?id=472688 https://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=1dcd1ffd4858a91f82384dc586064d6b7f6fd698 https://bugs.eclipse.org/bugs/show_bug.cgi?id=512335 https://bugs.eclipse.org/bugs/show_bug.cgi?id=514017 https://bugs.eclipse.org/bugs/show_bug.cgi?id=514019 |
||
| Whiteboard: | |||
Will consider, time permitting. I've got the same idea independently :) I'm willing to work on this, if you consider it worth adding to Eclipse - Jay, Dani, what do you think? I think the "Maximum line width for comments" setting should be changed to "Maximum width for comments", with a new checkbox underneath: "Count comment width from the beginning of the line". If checked, the formatter will behave in the old way. Maybe we could even uncheck it by default? It will make more sense if we keep comment width at 80, see bug 472688. I guess the general line width setting will take precedence over comment width, so that if line width is 120, comment width is 80 and a comment starts at column 50, its maximum width will be 70. (In reply to Mateusz Matela from comment #2) > I've got the same idea independently :) > I'm willing to work on this, if you consider it worth adding to Eclipse - > Jay, Dani, what do you think? I like it. > I think the "Maximum line width for comments" setting should be changed to > "Maximum width for comments", with a new checkbox underneath: "Count comment > width from the beginning of the line". If checked, the formatter will behave > in the old way. +1, but I'd probably add an unchecked option: [ ] Ignore leading whitespace > Maybe we could even uncheck it by default? It will make more sense if we > keep comment width at 80, see bug 472688. I'd like to avoid that. People using format on save hate if their code gets unexpected formatter changes. > I guess the general line width setting will take precedence over comment > width, so that if line width is 120, comment width is 80 and a comment > starts at column 50, its maximum width will be 70. I'm fine either way, but if leading whitespace isn't ignored (current state), the comment value must not be clipped (current state). +1 and I also agree with the points Dani mentioned. (In reply to Dani Megert from comment #3) > +1, but I'd probably add an unchecked option: [ ] Ignore leading whitespace This is not precise, because leading whitespace is not the only thing that can affect a comment's starting position - there can also be some code in the same line. How about: [ ] Count width from comment's starting position >> Maybe we could even uncheck it by default? It will make more sense if we >> keep comment width at 80, see bug 472688. >I'd like to avoid that. People using format on save hate if their code gets >unexpected formatter changes. This kind of discussion pops up in a few bugs already, I'd like to raise a point I haven't seen mentioned before. Your argument is valid in the short term, but if the new setting is really useful and most people would like it, then in the long run the number of people who "suffer" because they don't know about it or have to spend extra time looking for it will eventually greatly exceed the number of people "spared" at the beginning. So the earlier we change the setting the better in the long run :) And for the people you mentioned we can add a mechanism of copying the old default to a separate profile during workspace migration, right? Or just make a section at the beginning of "new and noteworthy" that warns about changed defaults. (In reply to Mateusz Matela from comment #5) > (In reply to Dani Megert from comment #3) > > +1, but I'd probably add an unchecked option: [ ] Ignore leading whitespace > > This is not precise, because leading whitespace is not the only thing that > can affect a comment's starting position - there can also be some code in > the same line. How about: > [ ] Count width from comment's starting position +1. > >> Maybe we could even uncheck it by default? It will make more sense if we > >> keep comment width at 80, see bug 472688. > >I'd like to avoid that. People using format on save hate if their code gets >unexpected formatter changes. > > This kind of discussion pops up in a few bugs already, I'd like to raise a > point I haven't seen mentioned before. Your argument is valid in the short > term, but if the new setting is really useful and most people would like it, > then in the long run the number of people who "suffer" because they don't > know about it or have to spend extra time looking for it will eventually > greatly exceed the number of people "spared" at the beginning. So the > earlier we change the setting the better in the long run :) I'd be OK to do this if we can keep the old behavior for those projects that do have formatter preferences stored in the project. > And for the people you mentioned we can add a mechanism of copying the old > default to a separate profile during workspace migration, right? Or just > make a section at the beginning of "new and noteworthy" that warns about > changed defaults. We should try to avoid such migration code. Fixed with http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=1dcd1ffd4858a91f82384dc586064d6b7f6fd698 (In reply to Dani Megert from comment #6) > > I'd be OK to do this if we can keep the old behavior for those projects that > do have formatter preferences stored in the project. > Has something like that been done before? ProfileVersioner doesn't seem to support it. The default is false for now to keep the old vehavior. (In reply to Mateusz Matela from comment #7) > Fixed with > http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=1dcd1ffd4858a91f82384dc586064d6b7f6fd698 > > > (In reply to Dani Megert from comment #6) > > > > I'd be OK to do this if we can keep the old behavior for those projects that > > do have formatter preferences stored in the project. > > > > Has something like that been done before? No. > ProfileVersioner doesn't seem to > support it. I think the only thing you'd have to check in the formatter code is whether the project has project specific formatter settings (not the one we add) and in that case set use 'false'. (In reply to Dani Megert from comment #8) > I think the only thing you'd have to check in the formatter code is whether > the project has project specific formatter settings (not the one we add) and > in that case set use 'false'. I think we could be even more conservative and consistent by changing the behavior only in the built-in profiles. This way people who didn't bother to create their own profile (or create it in the future by copying a default) will have the new feature enabled while everyone else will not see unexpected changes. If I understand this correctly, we can achieve this by changing the default value to true but setting it to false in ProfileVersioner. What do you think? This would mean a new commit to both jdt-core and jdt-ui. Shall I submit new bug reports for this or just use the existing ones? Verified for Eclipse Oxygen 4.7 M6 Build id: I20170305-2000 (In reply to Mateusz Matela from comment #9) > (In reply to Dani Megert from comment #8) > > I think the only thing you'd have to check in the formatter code is whether > > the project has project specific formatter settings (not the one we add) and > > in that case set use 'false'. > > I think we could be even more conservative and consistent by changing the > behavior only in the built-in profiles. This way people who didn't bother to > create their own profile (or create it in the future by copying a default) > will have the new feature enabled while everyone else will not see > unexpected changes. > If I understand this correctly, we can achieve this by changing the default > value to true but setting it to false in ProfileVersioner. What do you think? > > This would mean a new commit to both jdt-core and jdt-ui. Shall I submit new > bug reports for this or just use the existing ones? Sorry, that comment got lost in my haystack. I like the idea. Please file two new bugs and tag them for M7. |
Build Identifier: Build id: 20110916-0149 The formatter for comments (Java editor) provides for a line width to be specified - comments are wrapped at this column, regardless of the starting column. If this value is "80" and the comment begins at column 60 (indented code), then the comment block is 20 columns wide and many many lines tall. I suggest that the line width be augmented with a "comment width" value that specifies the width of a comment block - a value of "80" will format all comments to 80 columns, even if started in column 60. The exiting line width can be preserved for existing functionality and/or for hard limits such as anyone who prints source code. If you make the comment line width very wide, then the comments starting on the left will be very wide and difficult to read. A fixed width comment, about the width of the display, would remedy this issue and make long lines palatable. Personally, I set the line width to 150 and let the screen scroll. I either have to put up with really wide comments or squashed comments. Reproducible: Always Steps to Reproduce: 1. Edit the Java editor formatting options, Comments tab, set the line width to 40 (easier to see the issue). 2. On the Line Wrapping tab, set the max line width to 80 3. Enter multiple nested blocks (if (true){}) to indent the code to column 80 4. On a line that starts in column 30, start a /* */ comment above it and enter a lot of text. 5. Run the Formatter - the comment will be wrapped into a mess, ending at column 40.