Community
Participate
Working Groups
Follow-up to bug 308000 We have a compatibility problem with 3.6 clients (e.g. our old UI). A 3.6 client expects IJavaProject#getOption(..) and #getOptions(..) to return values for the deprecated formatter options, but these are removed now.
Should be fixed for 3.7M2.
Created attachment 178758 [details] Proposed patch This patch addresses the options compatibility issue in a global manner. First it keeps deprecated options in the map. Second, it tries to migrate them automatically when storing the values into the preferences. That has the advantage of avoiding to keep old entries in the preferences but ensure that old API clients will not lose their options values when calling the JDT/Core *options* methods... I think I have added all possible project and workspaces configurations variations with a deprecated option to verify that we won't have any longer options backward compatibility issues.
Olivier, please review, thanks
Patch looks good. I'll post a patch that adds two new tests and that adjusts the copyrights.
Created attachment 178760 [details] Proposed fix + regression tests Add two new tests + change one copyright. Everything else is the same.
(In reply to comment #5) > Created an attachment (id=178760) [details] > Proposed fix + regression tests > > Add two new tests + change one copyright. > Everything else is the same. Thanks Olivier :-) Released for 3.7M2 in HEAD stream.
Sorry, I just tried this patch, but now nothing works for me any more. When I revert bug 318010, then I'd expect that the old UI still works, but it doesn't. And with everything from HEAD, I can change the new options, but when I then try to format a CU, it only considers the deprecated options but not the new ones. I think the safest option for M2 is to revert this patch.
Reverted the fix. Frederic, please investigate using the latest patch. Thanks. Moving to M3 for safety.
It would also help if the formatter constants said more clearly what happens when both old and new (or only old) options are set. A further complication is that options are inherited from the instance scope.
Markus, I'm still missing clear scenarios highlighting the problems you found to make us reverting the patch. From my point of view, the classic scenarios were: 1) an client using options API methods with deprecated options 2) read deprecated preferences from an old project/workspaces As I said those two scenarios were covered by the proposed patch. So could you be more precise on the following points: 1) what does exactly mean: "reverting 318010"? Does it match a scenario that a common user may encounter? If so, could you describe it? 2) how both old and new preferences could be set at the same time? As the proposed patch does not persist the deprecated preferences, I was not able to find a use case where that can happen... I agree that new and deprecated *options* can exist simultaneously in the map but they are supposed to have the same value. Again, if you have a precise scenario showing that the options can be desynchronized and screwed up the formatter, then please describe it to help me to reproduce and fix it. TIA
I think this bug should be fixed for 3.7... As I'm a little bit less busy with Jazz those days, would it be possible to restart work on it? TIA
Sorry, we're in the week before M4 now, and I won't have time to get this done for M4. Scheduling for early M5. The main 3 scenarios we should support are these: a) pure 3.7 b) Eclipse 3.7, but project with 3.6 settings c) Eclipse 3.6, but project with 3.7 settings (will not understand the new options, but should use reasonable values for the deprecated options) (c) requires that we still store the old options in 3.7. Parts of the problem could also be in the current JDT/UI code. We don't use the setOption* methods but set the preferences directly in the store.
(In reply to comment #12) > Sorry, we're in the week before M4 now, and I won't have time to get this done > for M4. Scheduling for early M5. > > The main 3 scenarios we should support are these: > a) pure 3.7 > b) Eclipse 3.7, but project with 3.6 settings > c) Eclipse 3.6, but project with 3.7 settings (will not understand the new > options, but should use reasonable values for the deprecated options) > > (c) requires that we still store the old options in 3.7. > > Parts of the problem could also be in the current JDT/UI code. We don't use the > setOption* methods but set the preferences directly in the store. Unless I miss something, a) and b) are already covered by the proposed patch. And, unfortunately, I'm not sure to understand the real use case of c). Maybe this is related to comment 7 when you tried to "revert 318010"... So, as I was asking in comment 10, Markus, could you be a little bit more precise on this scenario and give me clear steps to help me to understand it and reproduce it easily? As I'm now 100% on Jazz, I won't have too much time to work on this problem (only extra hours to be honest...;-)). Hence any hints which could help me to save time to figure out what is the exact compatibility problem here will be more than welcome :-) TIA
Removing target milestone.
Ping... Markus, I guess this needs to be fixed before M6. So, could you please answer my comment 10 and comment 13? As I said in comment 13, I won't have too much time to spend on this regarding my Jazz work overload, hence the sooner I have more information about the real issue for this bug, the better it is for me to plan to work on a possible fix... TIA
Too late for 3.7, but I think we should target 3.8. Olivier, what do you think?
I would close as WONTFIX since once 3.7 is done, there is no need to work on this anymore.
I found a compatibility problem when moving a project (org.eclipse.jdt.junit) to 3.7. Looks like we currently keep both the old and the new options in the .prefs file, and the old options wins. After disabling all options in the UI, I still get newlines after annotations. I'm investigating.
Created attachment 195065 [details] Fix 2 There was a bug in the UI (bug 345003), which prevented profiles from being written into the .prefs file right after changing the profile version. I think I got tricked by that problem when I reopened this bug. But after that bug is fixed and a formatter profile is successfully changed, we still have a compatibility issue in HEAD: When a project contains a deprecated formatter.insert_new_line_after_annotation_on_* option and one of the replacement options, then the deprecated option wins. This is bad if we want to keep at least some compatibility with old Eclipse versions in the project options (think of a shared project that is used with 3.6 and 3.7). The 3.7 UI just leaves the old options in the file and adds the new options. In this situation, the new option should win and the old option should only be used as a fallback. This is broken both in HEAD and with Frederic's fix. I think Frederic's fix makes sense and is useful for clients that use the jdt.core APIs to read/write preferences (including the code formatter itself). The code formatter UI currently reads/writes the preferences directly via the Eclipse preference store, so we are not directly affected by the original patch. This patch (Fix 2) is Frederic's patch with the following changes: - DefaultCodeFormatterOptions lets the new options win over the deprecated ones - JavaModelManager and JavaProject's getOption methods use String compatibleOption = compatibleOptions[co]; if (!options.containsKey(compatibleOption)) options.put(compatibleOption, propertyValue); ... instead of ... options.put(compatibleOptions[co], propertyValue); - doesn't include the tests
Olivier (or Frederic, if you're still in the loop), could you please review Fix 2? I would release this for RC1.
The patch looks good. However this is still an issue for users that want to share the same project between 3.6 and 3.7. Also the tests need to be updated as they are using mixed options (old and new) and expect the old one to win.
Created attachment 195105 [details] Updated regression tests
Created attachment 195218 [details] Fix 4 Final patch against HEAD including all tests.
Fixed in HEAD and BETA_JAVA7.
Verified for 3.7RC1 with build I20110511-0800.
