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.
In present state the java module provides some 'not official' way how to extend java editor and work with guarded sections in java source files. This is used by the form module for example. Main goal of this task is to create regular API allowing to manipulate guarded sections without necessity to subclass JavaEditor and possibly SPI that will permit to implement sections even for other content types like xml, ... The new implementation must be able to read already stored sections. As a possible solution seems to be to create a new module editor/guards that will provide API and SPI plus an implementation presenting guarded sections in a StyledDocument. In order to support java guards there should be also the java/guards module that will implement reading and writing of sections from/to java source. Later new modules supporting other content types may be added.
I have added an initial prototype to cvs. You can get sources with cvs update -P -d -r guards_84239 editor/guards java/guards nbbuild/cluster.properties It is possible to build arch documents of both modules.
I would like to request a review of the new Guarded Sections API.
MM01) Could PositionRef be replaced by j.s.t.Position? Similarly could PositionBounds be replaced by getStart/EndPosition() in *Section ? I understand that this may problem for existing impls so we may discuss this further on the review. MM02) Could GuardedSections be renamed to GuardedSectionManager? (someone could expect it to be static utility class like e.g. for Collections or Lookups) MM03) I think that we do not need to prefix the interfaces by "I". The EditorSupport could possibly be GuardedEditorSupport. MM04) Why is there GuardedSection.openAt() when it's already deprecated and only throws exception? Please remove it from the API. MM05) Could GuardedSectionFactory and IGuardedSectionsProvider possibly be merged into one abstract class with the static find() method? MM06) Why is there the Utils class? Could not be the methods from it put directly into the appropriate classes SimpleSection and InteriorSection?
Y01 like (MM01) I can imagine there is a slight benefit in PositionRef, because they are persistent and can exist even if the document is closed. However I am not sure if this is what your API really needs. Maybe it works just on open documents. If that is so, I would prefer to completely remove dependency on openide/text and use the Swing's Position. If it is not possible to remove the dependency completely due to some implementation tricks, then still it is better to not show it in the APIs itself, so it could be potentially removed in future. Y02 like MM02 - classes named with plural are usually just factory classes which do not have instances. Rename as Mila suggested. Y03 It is not NetBeans convention to prefix interfaces "I". So please do not do it unless you want your API to be used in Eclipse, as well. Y04 In other APIs, we usually prefix the required/provided tokens with package name, so instead of "OpenIDE-Module-Requires: JAVA_GUARDED_SECTIONS" I would suggest "OpenIDE-Module-Requires: org.netbeans.api.editor.guards.sections.Java" or something like that. The precedence for this should be org.openide.modules.os.Unix & co. Y05 Can I see test coverage report? Can you generated it using "ant coverage" and attach here? Y06 like MM06 - either move the methods from the class somewhere else as Mila suggested or rename to GuardedSectionsUtilities, so this class does not interfere in code completion with other "util" classes. Y07 I do not understand the usecase "Plug guarded sections stuff into the editor" can I see a working test for it? Maybe you could like to the test from the usecase description... Y08 I suggest you to use hyperlinks in the code samples, at least for classes in your own API. Y09 Is any class from "org.netbeans.spi.editor.guards.providers" referenced from any other part of the API? E.g. is not this just a set of utility classes that people may or may not use? If so, project APIs started a convention to name such packages as support - e.g. "org.netbeans.spi.editor.guards.support" Y10 Do not use abbrevated names, for example rename "translateToCharBuff" Y11 I am not native speaker but I guess the language pairs top&bottom and header&footer, so you should not use header&bottom together I guess that is all from me right now, to summarize I would say that I understand the API part, but I need more info about the SPI one. Btw. I am adding API_REVIEW keyword and I'd like to know whether there will be a meeting to discuss and clear possible confusions?
The review was held already and the opinion document is at http://openide.netbeans.org/tutorial/reviews/opinions_84239.html Please feel free to add additional comments to this issue. Thanks.
Status update: ad MM02,Y02) GuardedSections renamed to GuardedSectionManager ad MM03) IEditorSupport renamed to GuardedEditorSupport ad MM04) GuardedSection.openAt() removed ad MM05) we agreed to not merge factory and provider ad MM06,Y06) Utils, IGuardedReader and IGuardedWriter merged to AbstractGuardedSectionsProvider ad Y03) "I" prefix removed ad Y04) better token name org.netbeans.api.editor.guards.Java ad Y09) package renamed to org.netbeans.spi.editor.guards.support ad Y10) abbreviated names removed ad Y11) "bottom" occurences renamed to "footer" TCA - GuardedSection.deleteSection() and removeSection throw ISE instead of return boolean; all setXXX() do not have boolean return type too TCA - GuardedSection.getBegin() renamed to getCaretPosition TCA - InteriorSection.getBody() added TCR - notifyDocumentModified removed
I forgot to mention: * TCA - rewritten to swing Positions * unit tested
The API meet requirements of Mobility team.
As there were no complaints I consider this review as approved. I have updated the opinion document.
Review is completed.