Summary: | possibly incorrect JDT POST_CHANGE event fired when a file is replaced | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | [Eclipse Project] JDT | Reporter: | Karen Butzke <karenfbutzke> | ||||||
Component: | Core | Assignee: | Jay Arthanareeswaran <jarthana> | ||||||
Status: | VERIFIED FIXED | QA Contact: | |||||||
Severity: | normal | ||||||||
Priority: | P3 | CC: | amj87.iitr, markus.kell.r, satyam.kandula | ||||||
Version: | 3.7.1 | ||||||||
Target Milestone: | 3.8 M5 | ||||||||
Hardware: | PC | ||||||||
OS: | Windows XP | ||||||||
Whiteboard: | |||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 312664 | ||||||||
Attachments: |
|
Description
Karen Butzke
2011-11-02 14:33:15 EDT
Hi Karen, can you please mention the jdt/core plugin version or the build id you're using? Thanks! I'm using 3.7.1 The problem here is the CopyResourceElementsOperation, regardless of existing resources, adds the delta (Foo.java[+]). And later on the DeltaProcessor realizes that the resource Foo has changed. But at the same time the DeltaProcessor has no idea that the Foo was not really ADDED but just CHANGED. I guess we have to add the right delta during the copy operation or somehow let the DeltaProcessor know that the ADDED delta was indeed an 'overwrite'. Looks like there are quite a few existing tests that are expecting the ADDED delta for the overwrite scenario. Don't know if the tests can be altered as they appear to be deliberate. Will investigate other options that will allow us to keep the ADDED delta along with either REMOVED or CHANGED. There are about 6 failing tests in CopyMoveResourcesTests and 2 in RenameTests, all failing more or less for the same reason - they are expecting that a new element was added. The problem is, even when we record a REMOVED delta followed by a ADDED, the resultant delta would be CHANGED, which makes sense. The existing tests don't make sense to me either. But it probably means some existing clients are depending on this behavior, albeit buggy. Perhaps JDT/UI can tell us what we want to do here. Created attachment 206448 [details]
Proposed patch
Patch + regression test.
Should we ever decide that the existing tests need to be fixed, we can use this patch. Of course, it doesn't yet contain fixes for those failing tests.
This first works great for my use case, thanks! The request and the analysis sound reasonable to me. I didn't look at the failing core tests, but the patch makes sense and I didn't get any failures in the UI tests with it. Good to go from my side. Created attachment 207292 [details]
Patch with updated tests
This patch is different from the previous: Instead of registering a remove and add delta, I identify the overwrite scenario and only register a content change delta. This addresses reported bug.
Also I modified a bunch of existing test case I mentioned earlier.
Since Jay had to go on unavoidable unplanned time off for several days, this is not ready: retargetting to 3.8 M5. Released the fix and tests: http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=2e99c72cc03690dcb30e61cdafeb829a4f1ada2f Verified for 3.8M5 using build I20120122-2000 |