Bugzilla – Full Text Bug Listing
| Summary: | [formatter] successive line comments are being indented an extra level | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [WebTools] JSDT | Reporter: | Ian Tewksbury <itewksbu> | ||||||||||
| Component: | General | Assignee: | Ian Tewksbury <itewksbu> | ||||||||||
| Status: | RESOLVED FIXED | QA Contact: | Nitin Dahyabhai <thatnitind> | ||||||||||
| Severity: | normal | ||||||||||||
| Priority: | P3 | CC: | cmjaun, david_williams, eclipse, fcsonline, jaap, neil.hauge, starwhisperer | ||||||||||
| Version: | 3.2 | Flags: | david_williams:
pmc_approved+
thatnitind: pmc_approved? (raghunathan.srinivasan) thatnitind: pmc_approved? (naci.dai) deboer: pmc_approved+ neil.hauge: pmc_approved+ thatnitind: pmc_approved? (kaloyan) cmjaun: review+ thatnitind: review+ |
||||||||||
| Target Milestone: | 3.2 RC4 | ||||||||||||
| Hardware: | All | ||||||||||||
| OS: | All | ||||||||||||
| Whiteboard: | PMC_approved | ||||||||||||
| Attachments: |
|
||||||||||||
Created attachment 170697 [details]
Fix Patch
This is my proposed fix. It fixes the issue and I do not see any regressions or other side effects. It passes all of the existing formatting tests but the existing formatting tests do not seem to go through this code path. I have been trying to make JUnits that due go through the code path in question but its truing out to be trickier then I hoped it would be. Looks like the tests will have to actually create an viewer and run the formatting through the viewer due to the multiple formatters that get run through that code path.
Created attachment 170792 [details]
Fix Patch - Update 1
Same fix now with JUnits to test for regression.
* Explain why you believe this is a stop-ship defect. Or, if it is a "hotbug" (requested by an adopter) please document it as such. This alters the source in a manner unexpected, and hard to detect, by the user. It also affects AST rewrite, and thus any sources constructed purely through API calls. * Is there a work-around? If so, why do you believe the work-around is insufficient? None. * How has the fix been tested? Is there a test case attached to the bugzilla record? Has a JUnit Test been added? Manual testing plus JUnit. * Give a brief technical overview. Who has reviewed this fix? Changes the formatter so that if two successive line comments are both over the maximum allowed length, breaking the first into two lines does not further indent the following lines. Chris and I have reviewed the changes. * What is the risk associated with this fix? Low as it is limited to formatting of line comments; block comments and other statements are unaffected. I wouldn't say "major" but source formatting is near and dear :) But, I'm not sure I understand the patch. On the surface it appears it just junits? If both, can you provide separate patches for junit vs. function? thanks, Created attachment 170895 [details]
functionality patch
Created attachment 170896 [details]
tests patch
(In reply to comment #4) > I wouldn't say "major" but source formatting is near and dear :) > > But, I'm not sure I understand the patch. On the surface it appears it just > junits? If both, can you provide separate patches for junit vs. function? > > thanks, Sure. k, thanks. Looks simple enough the payoff is worth the low risk. I know in some cases mis-formaatting source is seen by users as "damaging their assests" so I think a good idea to fix now. Approved based on isolated, low risk fix. Released v201006030742. *** Bug 312494 has been marked as a duplicate of this bug. *** *** Bug 286654 has been marked as a duplicate of this bug. *** *** Bug 249225 has been marked as a duplicate of this bug. *** Could you test this code? In my Eclipse fails.
function func1 () {
if (true) {
return true;
}
}
function func2() {
}
But, with the semicolons at the end of functions, works for me:
function func1 () {
if (true) {
return true;
}
};
function func2() {
};
|
In the JS editor successive line comments are being indented an extra level when using the formatter EXAMPLE: dojo.declare("myDojo.Test", [], { // this is a really long comment that will have to be wrapped into multiple lines because it is so very very long // this is a really long comment that will have to be wrapped into multiple lines because it is so very very long // Constructor function. Called when instance of this class is created constructor : function() { } }); RESULT: dojo.declare("myDojo.Test", [], { // this is a really long comment that will have to be wrapped into multiple // lines because it is so very very long // this is a really long comment that will have to be wrapped into // multiple lines because it is so very very long // Constructor function. Called when instance of this class is created constructor : function() { } }); EXPECTED: dojo.declare("myDojo.Test", [], { // this is a really long comment that will have to be wrapped into multiple // lines because it is so very very long // this is a really long comment that will have to be wrapped into multiple // lines because it is so very very long // Constructor function. Called when instance of this class is created constructor : function() { } });