Created attachment 19258 [details]
Proposed class supporting the ANT->Swing->ANT conversion and storing to EditableProperties

Created attachment 19259 [details]
Patch of the arch.xml int ant/project with added usecase

I would like to propose to add the API (see Ant.java) for support for
creating Swing models from ANT properties and for storing them back to
the project. For the short howto on using the API please see the
arch.xml.diff attachement. 

Please ingnore the surrounding Ant class. It won't be there when the
decision about the target package will be made. The question (very
likely for Jesse) is what the target package and or package should be.

Options:
1) ant/project org.netbeans.spi.project.support.ant 
  - Will mix plain ant support and swing models
  - possible ant/project dependency on project/libraries see 2)
2) ant/project org.netbeans.spi.project.support.ant.ui
  - I'm thinking about adding more methods into the StoreGroup class
    e.g. methods for creating TableModel from classpath this would
    require dependency of ant/project on project/libraries
3) new module ant/projectui same package as in 2)
  - This module could depend on ant/project and on project/libraries


  


  

Anoter aspect to consider when deciding about the place where to put
the api could be issue #49650 : Support SPI: artifact chooser.
In fact this is AntArtifactChooser and relies on AntArtifact and
AntAtifactQueryClasses and as such should probably not reside in
ProjectUIAPI module. (Project UIAPI module should not depend on ant
ANT secific implementation, I guess)

Re. location: ant/project org.netbeans.spi.project.support.ant.ui
sounds OK to me. I don't see any problem with ant/project ->
project/libraries. If we change our mind later, it is easy to split
the .ui package into its own module (using ModuleAutoDeps).


arch.xml patch is nice but (1) don't use the <h3>; (2) create valid
XHTML: <p></p> is evil, and text should be inside a <p>. Also, do we
really need the use case in arch.xml where it is hard to find?
Couldn't you just make up some sample code and paste it right into a
<pre> block in the StoreGroup Javadoc, where people would expect to
look for usage information? I think it is more appropriate to use
arch.xml to list *links* to appropriate and helpful documentation, not
as the main source of that documentation.


Now for the patch itself:


Need Javadoc for StoreGroup().


Class Javadoc for StoreGroup should be more descriptive, I guess.


Looks like a unit test for this stuff would be straightforward and
helpful. After all, you're using Swing models, which are much easier
to handle inside a test than the real component.


'models' is a Map<String,Object[]|Document>, not a
Map<String,Object[][]> as its comment implies. Right? And it seems
that the code in store(...) which casts models.get(...) to Object[] is
wrong as createStringDocument calls models.put(String,Document).


Javadoc for createBooleanButtonModel and various pieces of impl say
"enabled/disabled", yet cBBM Javadoc also says "on/off". Which is it?
Note that Ant accepts "on/off" but not "enabled/disabled" so I don't
know where "enabled/disabled" is coming from.


Remove the System.out.println.


Why do you do

Document d = new PlainDocument();
d.remove(0, d.getLength());

If the document has just been created, surely you do not need to
remove anything from it.


You can throw an AssertionError from the BadLocationException catches,
since it should be impossible.


s/BOOLAN/BOOLEAN/g


There is no particular reason to require an EditableProperties to be
passed in; any Map<String,String> would do. However if you do use it,
please call setProperty, not put.


Document and enforce threading: must store(...) be called from EQ?


Would like to see a diff of j2seproject using the new API, as well as
positive comments from at least one other potential user.

Usecase in arch.xml is displayed at 
http://www.netbeans.org/download/dev/javadoc/usecases.html
should be written in a way to be meaningful on that page. Of course
(other) usecase in javadoc is useful as well. 

Test should of course be a must in a regular api development.

"Usecase in arch.xml is displayed at 
http://www.netbeans.org/download/dev/javadoc/usecases.html
should be written in a way to be meaningful on that page. Of course
(other) usecase in javadoc is useful as well." - I don't follow the
logic here. If there is one known use case for a particular class,
surely it is best to put the actual sample code directly into the
class Javadoc, where it is obvious if you are looking at the Javadoc;
and add an href to the use cases page for people looking there.

usecase in class javadoc + link to it is sufficient. I was referening
to the fact that usecases.html should become entry point for main
nbdev tasks. This would not work without the link.

Created attachment 19601 [details]
Modified version of the SourceGroup

Created attachment 19602 [details]
Unit test for the store group

Created attachment 19603 [details]
Demo of the StoreGroup usage in the J2SE project

The required changes done. And attached. Test written and attached.
Also attached demo of the usaage in J2SE project. The useage in the
panels should be staight forward e.g. someComponent.setModel(). See
the test.

Would like to check in tomorrow. (Jan 12 2005)

In the unit test, you could avoid the need for the
PlainPropertyEvaluator nested class; see PropertyUtils methods
fixedPropertyProvider and sequentialPropertyEvaluator.


Don't forget SPL in StoreGroup.java (and 2005 copyrights everywhere).


In J2SEUIProperties: s/Propeties/Properties/g


Catch of BadLocationException in retrieving text from a Document
should I guess throw an assertion error - should never happen.


J2SEUIProperties: various static fields could be final. Also, probably
many of the public constants are already defined elsewhere? Some are
unused, too: SRC_DIR and TEST_SRC_DIR at least.

Seems to me that your implementation does not allow two models for  
the same property in one StoreGroup. Even not very likely, I guess  
that this is not the desired behaviour.  
 
I suggest to add link to CVS to show sample usage in J2SE to the 
javadoc of StoreGroup. 

What would you do with two models for the same property? Which one
would you store from??

At least throw exception.

Agreed; assuming that it is forbidden to have >1 model for a single
property, the create* methods should throw IAE if it is attempted (and
Javadoc and the throws clause should both mention this).

Proposed changes checked in.