Summary: | [model] Let working copy owner control the problem requestor used | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | [Eclipse Project] JDT | Reporter: | Frederic Fusier <frederic_fusier> | ||||||||||
Component: | Core | Assignee: | Frederic Fusier <frederic_fusier> | ||||||||||
Status: | VERIFIED FIXED | QA Contact: | |||||||||||
Severity: | enhancement | ||||||||||||
Priority: | P3 | CC: | jerome_lanneluc, martinae, philippe_mulet | ||||||||||
Version: | 3.3 | Keywords: | api | ||||||||||
Target Milestone: | 3.3 M6 | ||||||||||||
Hardware: | PC | ||||||||||||
OS: | Windows XP | ||||||||||||
Whiteboard: | |||||||||||||
Attachments: |
|
Description
Frederic Fusier
2007-02-23 05:21:34 EST
I would like to see the patch, assuming the old method would get deprecated. +1 for the API Created attachment 59793 [details]
Proposed patch
Comment on attachment 59793 [details]
Proposed patch
Obsolete because I forgot to deprecate becomeWorkingCopy(IProblemRequestor, IProgressMonitor) method.
While adding the deprecation on becomeWorkingCopy(IProblemRequestor, IProgressMonitor) makes me feel that ICompilationUnit.getWorkingCopy(WorkingCopyOwner, IProblemRequestor, IProgressMonitor) should also be deprecated... Jerome, Martin, what's your mind about this? I agree. There's also a 3rd case in IClassFile. Agree to deprecate ICompilationUnit.getWorkingCopy(WorkingCopyOwner, IProblemRequestor, IProgressMonitor). For the third case on IClassFile, I guess Martin meant IClassFile.becomeWorkingCopy(IProblemRequestor, WorkingCopyOwner, IProgressMonitor). If we deprecate this one, then we must add becomeWorkingCopy(WorkingCopyOwner, IProgressMonitor), otherwise it won't be possible for a class file to become a working copy (even if I think this functionality is not used right now). Can a class file really 'become' a working copy? Or is 'getWorkingCopy(WorkingCopyOwner...)' what is does? (In reply to comment #8) > Can a class file really 'become' a working copy? Or is > 'getWorkingCopy(WorkingCopyOwner...)' what is does? > You are right. I had forgotten that IClassFile#becomeWorkingCopy(...) returns a ICompilationUnit handle and should really be called getWorkingCopy(...). So if we deprecate IClassFile#becomeWorkingCopy(...), we should just put a reference to ITypeRoot#getWorkingCopy(WorkingCopyOwner, IProgressMonitor). Created attachment 60492 [details]
Complete proposed patch
This patch deprecates following methods:
- ICompilationUnit
#becomeWorkingCopy(IProblemRequestor, IProgressMonitor
#getWorkingCopy(WorkingCopyOwner, IProblemRequestor, IProgressMonitor)
- IClassFile
#becomeWorkingCopy(IProblemRequestor, WorkingCopyOwner, IProgressMonitor)
- WorkingCopyOwner
#newWorkingCopy(String,IClasspathEntry[],IProblemRequestor,IProgressMonitor)
Note that migration from deprecated methods to new API ones (ie. without problem requestor as parameter) was not really trivial and can represent a big amount of work for existing clients. I'll complete this patch by one or several FAQ in JDT porting guide of org.eclise.jdt.doc.isv to give some advice on how to use the new API methods without changing the behavior of the existing code... Created attachment 60544 [details]
Proposed FAQ entry in porting guide documentation
'null' is spec'ed as a valid argument to WorkingCopyOwner.getProblemRequestor. Is that necessary? From the usages it seems that this doesn't happen. Other than that: The API looks good. In the spec I wouldn't write 'is reported twice' but rather 'is reported on the passed problem requester as well as on the problem requestor returned by the working copy owner (if not null)' Agree that it looks good and agree with comment #13. Otherwise minor issues: - typo in ICompilationUnit#becomeWorkingCopy(IProgressMonitor): "Uses the primary owner *is* none" - changes to IJavaElementDelta's Javadoc are not part of the solving of this issue - why is the phrasing changed in WorkingCopyOwner#newWorkingCopy(String, IClasspathEntry[], IProblemRequestor, IProgressMonitor) ? It feels like a change in behavior Created attachment 60688 [details] New proposed patch based on v_741 (In reply to comment #13) > 'null' is spec'ed as a valid argument to WorkingCopyOwner.getProblemRequestor. > Is that necessary? From the usages it seems that this doesn't happen. > Removed in this new patch > Other than that: The API looks good. In the spec I wouldn't write 'is reported > twice' but rather > 'is reported on the passed problem requester as well as on the problem > requestor returned by the working copy owner (if not null)' > Fixed in this new patch (In reply to comment #14) > Agree that it looks good and agree with comment #13. > Otherwise minor issues: > - typo in ICompilationUnit#becomeWorkingCopy(IProgressMonitor): "Uses the > primary owner *is* none" > Fixed in this new patch > - changes to IJavaElementDelta's Javadoc are not part of the solving of this > issue > I've released this change independently => no longer in new patch > - why is the phrasing changed in WorkingCopyOwner#newWorkingCopy(String, > IClasspathEntry[], IProblemRequestor, IProgressMonitor) ? It feels like a > change in behavior > Put back old phrasing in this new patch Patches (code and doc) released for 3.3 M6 in HEAD stream. Verified for 3.3 M6 using build I20070320-0010 |