This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.
This is an umbrella issue for tag based editor support module related work targeted to 4.2 release.
Attaching arch questions document with answered "init" questions...
Created attachment 23296 [details] The arch document answers - init phase
The existing code is placed in cvs module xml/tageditorsupport in tag_editor_support branch. An implementation of the provider SPI for XML files (along with navigator and folding implementations) is in xml/text-edit module in the same branch.
I would like to kindly ask reviewers (ppisl, abadea, mroskanin and mmetelka) for the review.
Marek, could you attach the javadoc?
I am sorry, but since this is an inception review I have no javadoc prepared yet. Please look into sources - they are divided into API, SPI and util classes.
MM01) There is duplicate EditorLibrary and EditorLib in the list of the imported APIs in the arch doc. MM02) IMHO the org.netbeans.editor.ext.tags.api is not the optimal package placement as we plan to deprecate the org.netbeans.editor and org.netbeans.editor.ext contents in the future completely. Maybe org.netbeans.modules.editor.tags.api or org.netbeans.lib.editor.tags.api would be better. MM03) BaseDocument should not appear in your API e.g. DocumentModel or DocumentModelProvider. Use just j.s.t.Document. MM04) DocumentModelProvider is not final. Is it supposed to be? What about DocumentModel, should it be final? There are some protected methods though. MM05) How will the model's lock methods coexist with the document's locking mechanism? Why is the writeLock() in the API (though protected but would be accessible through subclassing DocumentModel) when in the arch doc the model is stated to be read-only? MM06) Did you look at the j.s.t.Element and the related classes? Maybe they would suite your needs too without introducing an extra API and have your custom root element available through Document.getRootElements(). The arch doc is terse about how the model will be updated - just saying "model is incrementally updated based on changes of the document content (e.g. when user edits the document)" so I'm not able to investigate this more.
MR01: DocumentModelGeneratorFactoryProvider could use MimeLookup for obtaining mime type sensitive factories. MR02: DocumentElement class if final. It is not necessary to provide protected method fireDocumentElementEvent.
Created attachment 23404 [details] The opinions document
I have finished the arch questions so the document is now complete and ready for final review. I will attach the arch document here as well as zipped javadoc.
Created attachment 23727 [details] The arch document.
Created attachment 23728 [details] Zipped javadoc
Reviewers, since all TCR and TCA are resolved, I would like to ask you for the final review. If noone has a strong objections against the changes done in the API or against the solutions of the TCR and TCA I suggest to make the final review only by issuezilla.
Oops, I forgot to say that the packages will be changed to org.netbeans.modules.editor.structure to better adhere the future editor's plans.
AB01: I notice a new DocumentModelProviderFactory SPI has been added. It seems to me it it not necessary, since instances of DocumentModelProvider can be registered in the layer, similar to how the providers code completion API are. Other than this, I have no objections.
Thanks Andrei for catching this. The factory used to be necessary in the past when the DocumentModelProvider wasn't a singleton class. I forgot to remove the factory after the code change. Now the DocumentModelProvider instance is declared in the layer directly and instantionalized only once. fixed.
Now I don't have any objections as well.
No objections.
If MM01 and MM05 gets resolved I'm ok with the integration. BTW the packaging change to o.n.m.editor.structure seems not done yet in the branch but I suppose you are willing to do it as you've said in the earlier note.
1) the classes have been repackaged to org.netbeans.modules.editor.structure as agreed earlier 2) the arch document fixed so it doesn't contain the duplicate EditorLib imported API. 3) As for the document locking see my comment in Issue #61625 regarding performance. Since noone else complained I am going to merge the module into trunk tomorrow along with a XML provider implementation.
Thanks to all reviewers for the good work!
v.