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.

Bug 61412 - Tag Based Editor Support API review
Summary: Tag Based Editor Support API review
Status: VERIFIED FIXED
Alias: None
Product: xml
Classification: Unclassified
Component: Code (show other bugs)
Version: 5.x
Hardware: All All
: P2 blocker (vote)
Assignee: Marek Fukala
URL:
Keywords: API_REVIEW
Depends on: 61622 61623 61624 61625 61626
Blocks:
  Show dependency tree
 
Reported: 2005-07-26 15:44 UTC by Marek Fukala
Modified: 2006-08-22 15:51 UTC (History)
6 users (show)

See Also:
Issue Type: TASK
Exception Reporter:


Attachments
The arch document answers - init phase (50.08 KB, text/html)
2005-07-26 15:46 UTC, Marek Fukala
Details
The opinions document (4.25 KB, text/html)
2005-08-01 18:37 UTC, Andrei Badea
Details
The arch document. (48.80 KB, text/html)
2005-08-11 15:06 UTC, Marek Fukala
Details
Zipped javadoc (121.16 KB, application/x-compressed)
2005-08-11 15:06 UTC, Marek Fukala
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Marek Fukala 2005-07-26 15:44:07 UTC
This is an umbrella issue for tag based editor support module related work
targeted to 4.2 release.
Comment 1 Marek Fukala 2005-07-26 15:46:10 UTC
Attaching arch questions document with answered "init" questions...
Comment 2 Marek Fukala 2005-07-26 15:47:00 UTC
Created attachment 23296 [details]
The arch document answers - init phase
Comment 3 Marek Fukala 2005-07-26 15:49:39 UTC
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.
Comment 4 Marek Fukala 2005-07-26 15:53:00 UTC
I would like to kindly ask reviewers (ppisl, abadea, mroskanin and mmetelka) for
the review.
Comment 5 Petr Pisl 2005-08-01 13:25:22 UTC
Marek, could you attach the javadoc?
Comment 6 Marek Fukala 2005-08-01 13:34:49 UTC
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.
Comment 7 Miloslav Metelka 2005-08-01 14:24:09 UTC
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.
Comment 8 Martin Roskanin 2005-08-01 14:41:14 UTC
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.



Comment 9 Andrei Badea 2005-08-01 18:37:47 UTC
Created attachment 23404 [details]
The opinions document
Comment 10 Marek Fukala 2005-08-11 15:03:20 UTC
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.
Comment 11 Marek Fukala 2005-08-11 15:06:04 UTC
Created attachment 23727 [details]
The arch document.
Comment 12 Marek Fukala 2005-08-11 15:06:39 UTC
Created attachment 23728 [details]
Zipped javadoc
Comment 13 Marek Fukala 2005-08-11 15:09:02 UTC
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.
Comment 14 Marek Fukala 2005-08-11 15:29:26 UTC
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.
Comment 15 Andrei Badea 2005-08-11 15:40:54 UTC
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.
Comment 16 Marek Fukala 2005-08-11 16:10:16 UTC
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.
Comment 17 Petr Pisl 2005-08-11 16:25:08 UTC
Now I don't have any objections as well. 
Comment 18 Martin Roskanin 2005-08-12 12:45:55 UTC
No objections.
Comment 19 Miloslav Metelka 2005-08-12 14:14:39 UTC
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.
Comment 20 Marek Fukala 2005-08-16 13:36:14 UTC
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.
Comment 21 Marek Fukala 2005-08-16 13:37:14 UTC
Thanks to all reviewers for the good work!
Comment 22 Dan Kolar 2006-08-22 15:51:39 UTC
v.