Bugzilla – Full Text Bug Listing
| Summary: | [quickfixes] Fixing all 'remove unused NLS' breaks file | ||
|---|---|---|---|
| Product: | [ECD] Orion | Reporter: | Michael Rennie <Michael_Rennie> |
| Component: | JS Tools | Assignee: | Curtis Windatt <curtis.windatt.public> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | major | ||
| Priority: | P3 | CC: | curtis.windatt.public |
| Version: | 11.0 | ||
| Target Milestone: | 11.0 | ||
| Hardware: | All | ||
| OS: | All | ||
| Whiteboard: | |||
Looks like the logic in there to remove whitespace is a bit to powerful :) consider the simple snippet: var v = 10; //$NON-NLS-1$ //$NON-NLS-2$ run the 'fix all' on it and it eats the ';'. The problem seems to be the space between the NON-NLS entries. if you change: //$NON-NLS-1$ //$NON-NLS-2$ to: //$NON-NLS-1$ //$NON-NLS-2$ it works fine. http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=06df5f4e95cdb203558d6537b543fe8f20d8b02b Fixed in master with tests In some of the logic I was removing leading whitespace, while in other logic I removed trailing whitespace. In this case, the space between the two tags was being removed twice. There is one test that had this overlapping effect already, but the tests check the contents of the changes, not the result of the whole file afterward. Similarly, in my test runs on the workspace I didn't run into it because often we put two spaces before the first NON-NLS tag, whereas in the failing case there was only one space. |
Steps: 1. open occurrences.js in the JS bundle 2. find an unused NLS error 3. use the fix and 'remove all' 4. notice the if statement on line 618 has been broken (the opening '{' has been removed during the fix)