Bugzilla – Full Text Bug Listing
| Summary: | DocumentSetupParticipants (easily) interfere with each other | ||
|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | David Williams <david_williams> |
| Component: | Ant | Assignee: | Darin Wright <darin.eclipse> |
| Status: | VERIFIED FIXED | QA Contact: | |
| Severity: | major | ||
| Priority: | P3 | CC: | Darin_Swanson, kai-uwe_maetzel, thatnitind, wbeckwith |
| Version: | 3.0 | ||
| Target Milestone: | 3.0 RC2 | ||
| Hardware: | PC | ||
| OS: | Windows XP | ||
| Whiteboard: | |||
|
Description
David Williams
to be investigated I'll update Javadoc. Please keep in mind that Javadoc is not yet complete. The extension point is for allowing plug-ins to participate in the process of setting up documents. Emphasis on "participate". Thus the same sharing rule applies as for every other oridinary extension point: Add, don't replace. This means that every participant has to setup the document in a way that does not conflict with the interest of others. Thus, the only valid way approach is to work with the contract introduced by IDocumentExtension3 (which actually has been invented for exactly this case). Thus, when your setup participant fails in the described scenario, this is the expected behavior. This also means that you as well as Ant have to adapt the contributed setup participants. Ok, thanks Kai. I'll reopen this under "ant" component, to be sure that particular setup participant get's changed (to either not setup partitioning there, or to use the "addPartitioner" approach. [Darin, sorry, if this has already been done, but I'm not in a position to check head today, and just wanted to be sure this didn't get lost]. And, BTW, we will make similar changes. [Alternatively, I susppose the setup participant in this case could just check to see if the instanceof document is one that its really interested in?] Also ... the use case for using setup partitioner at all is still unclear to me, I'd appreciate some mention/javadoc for that. In the past, we've always initialized partioner when the document was created. Is there a reason not to do that? Thanks David and Kai. So just to ensure I am on the right page:
The AntDocumentSetupParticipant setup(IDocument) method should look like:
public void setup(IDocument document) {
if (document != null) {
IDocumentPartitioner partitioner = createDocumentPartitioner();
if (document instanceof IDocumentExtension3) {
IDocumentExtension3 extension3= (IDocumentExtension3) document;
extension3.setDocumentPartitioner(ANT_PARTITIONING, partitioner);
} else {
document.setDocumentPartitioner(partitioner);
}
partitioner.connect(document);
}
}
I have released changes for the Ant component to head and will be in the 200406040800 build Changes to: AbstractAntSourceViewerConfiguration: overrode getConfiguredDocumentPartitioning and set the partioning for the reconciler XmlFormatter: changed the partitioning for the MultiPassContentFormatter AntDocumentSetupParticipant: made a participant :-) by using the IDocumentExtension3 functionality and defining the partitioning AntEditorDocumentProvider: changed to use a forwarding doc provider with the correct partitioning. AntStorageDocumentProvider: changed setup to make use of the IDocumentExtension3 functionality *** Bug 64910 has been marked as a duplicate of this bug. *** Please verify DarinW. I'd like to re-open. Sorry I haven't looked closely before, but tonight noticed
that your fix still interferes with non-DocumentExtension3 documents.
For non-DocumentExention3 documents, an assert is still thrown in the "else"
clause below. My thought would be to do nothing if its not a DocumentExention3
document. I'm re-opening with slightly lowered severity, since we are due to
change our document/partitioner too, but currently this is preventing us from
using "setup" extension. And, I'll remind that this is related to 64683 since
your "ant setup" participates in ALL xml documents.
if (document instanceof IDocumentExtension3) {
IDocumentExtension3 extension3= (IDocumentExtension3) document;
extension3.setDocumentPartitioner(ANT_PARTITIONING, partitioner);
} else {
document.setDocumentPartitioner(partitioner);
}
Seems reasonable...I will look at it more tomorrow. I was following the coding of the other participants...so others may want to revisit there code as well :-) I changed the AntDocumentSetupParicipant to only be concerned with IDocumentExtension3 documents. Back to verify. Note comment #7 for a test case. Verified. |