Bug 136595 - DOM utility methods to add to org.openide.xml.XMLUtil
DOM utility methods to add to org.openide.xml.XMLUtil
Status: RESOLVED FIXED
Product: platform
Classification: Unclassified
Component: -- Other --
6.x
All All
: P2 (vote)
: 6.x
Assigned To: Jesse Glick
issues@platform
: API, API_REVIEW_FAST
Depends on: 184377
Blocks: 186930
  Show dependency treegraph
 
Reported: 2008-06-05 20:19 UTC by Jesse Glick
Modified: 2011-03-07 23:46 UTC (History)
4 users (show)

See Also:
Issue Type: ENHANCEMENT
:


Attachments
Changes to XMLUtil.java (8.00 KB, patch)
2010-03-05 17:57 UTC, mvfranz
Details | Diff
Unit test for new methods on XMLUtil.java (7.03 KB, patch)
2010-03-05 18:01 UTC, mvfranz
Details | Diff
ant.freeform refactor diff (79.48 KB, patch)
2010-03-08 07:05 UTC, mvfranz
Details | Diff
apisupport.project refactor diff (49.92 KB, patch)
2010-03-08 07:06 UTC, mvfranz
Details | Diff
j2ee.archive refactor diff (3.92 KB, patch)
2010-03-08 07:06 UTC, mvfranz
Details | Diff
j2ee.earproject refactor diff (6.43 KB, patch)
2010-03-08 07:07 UTC, mvfranz
Details | Diff
j2ee.ejbjarproject refactor diff (13.72 KB, patch)
2010-03-08 07:08 UTC, mvfranz
Details | Diff
java.freeform refactor diff (85.71 KB, patch)
2010-03-08 07:08 UTC, mvfranz
Details | Diff
java.j2seproject refactor diff (3.66 KB, patch)
2010-03-08 07:09 UTC, mvfranz
Details | Diff
maven refactor diff (4.07 KB, patch)
2010-03-08 07:09 UTC, mvfranz
Details | Diff
o.n.bluj refactor diff (3.42 KB, patch)
2010-03-08 07:10 UTC, mvfranz
Details | Diff
openide.loaders refactor diff (1.11 KB, patch)
2010-03-08 07:10 UTC, mvfranz
Details | Diff
profiler.nbmodule refactor diff (2.62 KB, patch)
2010-03-08 07:11 UTC, mvfranz
Details | Diff
project.ant refactor diff (40.84 KB, patch)
2010-03-08 07:12 UTC, mvfranz
Details | Diff
projectapi refactor diff (2.71 KB, patch)
2010-03-08 07:12 UTC, mvfranz
Details | Diff
projectimport.eclipse.core refactor diff (12.23 KB, patch)
2010-03-08 07:13 UTC, mvfranz
Details | Diff
projectimport.eclipse.web refactor diff (5.68 KB, patch)
2010-03-08 07:13 UTC, mvfranz
Details | Diff
projectimport.jbuilder refactor diff (8.82 KB, application/octet-stream)
2010-03-08 07:14 UTC, mvfranz
Details
ruby.project refactor diff (3.33 KB, patch)
2010-03-08 07:15 UTC, mvfranz
Details | Diff
ruby.rakeproject refactor diff (12.56 KB, patch)
2010-03-08 07:16 UTC, mvfranz
Details | Diff
web.freeform refactor diff (18.38 KB, patch)
2010-03-08 07:17 UTC, mvfranz
Details | Diff
web.project refactor diff (8.59 KB, patch)
2010-03-08 07:17 UTC, mvfranz
Details | Diff
j2ee.clientproject refactor diff (5.89 KB, patch)
2010-03-08 07:18 UTC, mvfranz
Details | Diff
XMLUtil patches for refactored methods (19.84 KB, patch)
2010-03-28 00:24 UTC, mvfranz
Details | Diff
Patch for modules to use new XMLUtil methods (395.09 KB, patch)
2010-03-29 11:11 UTC, mvfranz
Details | Diff
All changes for XMLUtil (438.66 KB, patch)
2010-04-04 13:36 UTC, mvfranz
Details | Diff
Refactored code for XMLUtil (438.62 KB, application/octet-stream)
2010-04-05 23:38 UTC, mvfranz
Details
XMLUtil Refactoring patch (443.13 KB, patch)
2010-04-07 04:48 UTC, mvfranz
Details | Diff
XMLUtil Refactoring patch (443.13 KB, patch)
2010-04-08 02:15 UTC, mvfranz
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jesse Glick 2008-06-05 20:19:55 UTC
Over time a number of very common DOM-related utility methods have cropped up in the code base and multiplied:

Element findElement(Element parent, String name, String namespace)
String findText(Element parent)
List<Element> findSubElements(Element parent)
ErrorHandler defaultErrorHandler()
void appendChildElement(Element parent, Element el, String[] order)
Element translateXML(Element from, String namespace)

Only one instance so far but seems like it *ought* to be used more often:

void validate(Element data, Schema schema)

Source files from which common code could be extracted:

ant.freeform/src/org/netbeans/modules/ant/freeform/spi/support/Util.java
apisupport.project/src/org/netbeans/modules/apisupport/project/Util.java
compapp.projects.base/src/org/netbeans/modules/compapp/projects/base/ui/customizer/IcanproProjectProperties.java
compapp.projects.jbi/src/org/netbeans/modules/compapp/projects/jbi/ui/customizer/JbiProjectProperties.java
j2ee.clientproject/src/org/netbeans/modules/j2ee/clientproject/classpath/ClassPathSupportCallbackImpl.java
j2ee.earproject/src/org/netbeans/modules/j2ee/earproject/UpdateProjectImpl.java
j2ee.earproject/src/org/netbeans/modules/j2ee/earproject/classpath/ClassPathSupportCallbackImpl.java
j2ee.ejbjarproject/src/org/netbeans/modules/j2ee/ejbjarproject/UpdateProjectImpl.java
j2ee.ejbjarproject/src/org/netbeans/modules/j2ee/ejbjarproject/classpath/ClassPathSupportCallbackImpl.java
java.api.common/src/org/netbeans/modules/java/api/common/classpath/j2ee/BaseClassPathSupport.java
java.api.common/src/org/netbeans/modules/java/api/common/classpath/j2ee/J2EEClassPathSupport.java
project.ant/src/org/netbeans/modules/project/ant/Util.java
projectimport.eclipse.core/src/org/netbeans/modules/projectimport/eclipse/core/Util.java
projectimport.eclipse.web/src/org/netbeans/modules/projectimport/eclipse/web/Util.java
projectimport.jbuilder/src/org/netbeans/modules/projectimport/jbuilder/parsing/Util.java
projectui/test/unit/src/org/netbeans/modules/project/ui/actions/TestSupport.java
ruby.rakeproject/src/org/netbeans/modules/ruby/modules/project/rake/Util.java
web.project/src/org/netbeans/modules/web/project/UpdateProjectImpl.java
web.project/src/org/netbeans/modules/web/project/classpath/ClassPathSupportCallbackImpl.java
contrib/autoproject.core/src/org/netbeans/modules/autoproject/core/AuxiliaryConfigImpl.java [see separate API review]
contrib/contrib/latex/guiproject/src/org/netbeans/modules/latex/guiproject/LaTeXAuxiliaryConfigurationImpl.java
contrib/erlang.makeproject/src/org/netbeans/modules/erlang/makeproject/modules/Util.java
contrib/erlang.project/src/org/netbeans/modules/erlang/project/Util.java
contrib/visualweb.project.importpage/src/org/netbeans/modules/visualweb/project/importpage/Import.java [not a copy]

Can also synch overlapping method impls with:

nbbuild/antsrc/org/netbeans/nbbuild/XMLUtil.java
Comment 1 David Konecny 2008-06-05 21:58:21 UTC
Cool! Thanks. I did not forget about this. I'm happy to help by updating java/web/j2ee/import modules to this new API.
Comment 2 Jesse Glick 2008-06-05 22:52:03 UTC
Since I have the list of usages, it would be easiest for me to replace all of them in one sweep, I think. Will probably
file API review first with XMLUtil.java patch (coming soon), and if it seems acceptable to people, file a proposed patch
of other modules a couple days later for additional review.
Comment 3 Jesse Glick 2008-07-02 22:38:14 UTC
AuxiliaryConfigImpl now in projectapi.
Comment 4 Jesse Glick 2008-07-03 21:06:00 UTC
validate to be implemented separately in issue #42686.
Comment 5 Jesse Glick 2008-08-28 21:08:00 UTC
(also check: ProjectXMLCatalogReader)
Comment 6 Jesse Glick 2008-09-29 20:32:16 UTC
findElement should throw IAE, not return null, in case >1 element of the given name can be found among children of the
specified parent. See issue #141277 for background.
Comment 7 mvfranz 2010-03-05 17:57:04 UTC
Created attachment 94822 [details]
Changes to XMLUtil.java
Comment 8 mvfranz 2010-03-05 17:59:25 UTC
Here are the proposed changes to XMLUtil.  I added 6 methods and 6 unit tests.  These methods were extracted from 21 module and 95 files.

I can added the diffs to specific modules if required.  The module list is:
ant.freeform
apisupport.project
j2ee.archive
j2ee.clientproject
j2ee.earproject
j2ee.ejbjarproject
java.freeform
java.j2seproject
maven
o.n.bluej
openide.loaders
profiler.nbmodule
project.ant
projectapi
projectimport.eclipse.core
projectimport.eclipse.web
projectimport.jbuilder
ruby.project
ruby.rakeproject
web.freeform
web.project
Comment 9 mvfranz 2010-03-05 18:01:20 UTC
Created attachment 94823 [details]
Unit test for new methods on XMLUtil.java
Comment 10 Petr Jiricka 2010-03-08 03:01:03 UTC
> I can added the diffs to specific modules if required.

Yes, this would be great. Thanks!
Comment 11 mvfranz 2010-03-08 07:05:12 UTC
Created attachment 94844 [details]
ant.freeform refactor diff
Comment 12 mvfranz 2010-03-08 07:06:05 UTC
Created attachment 94845 [details]
apisupport.project refactor diff
Comment 13 mvfranz 2010-03-08 07:06:49 UTC
Created attachment 94846 [details]
j2ee.archive refactor diff
Comment 14 mvfranz 2010-03-08 07:07:32 UTC
Created attachment 94847 [details]
j2ee.earproject refactor diff
Comment 15 mvfranz 2010-03-08 07:08:09 UTC
Created attachment 94848 [details]
j2ee.ejbjarproject refactor diff
Comment 16 mvfranz 2010-03-08 07:08:50 UTC
Created attachment 94849 [details]
java.freeform refactor diff
Comment 17 mvfranz 2010-03-08 07:09:24 UTC
Created attachment 94850 [details]
java.j2seproject refactor diff
Comment 18 mvfranz 2010-03-08 07:09:54 UTC
Created attachment 94851 [details]
maven refactor diff
Comment 19 mvfranz 2010-03-08 07:10:25 UTC
Created attachment 94852 [details]
o.n.bluj refactor diff
Comment 20 mvfranz 2010-03-08 07:10:57 UTC
Created attachment 94853 [details]
openide.loaders refactor diff
Comment 21 mvfranz 2010-03-08 07:11:36 UTC
Created attachment 94854 [details]
profiler.nbmodule refactor diff
Comment 22 mvfranz 2010-03-08 07:12:03 UTC
Created attachment 94855 [details]
project.ant refactor diff
Comment 23 mvfranz 2010-03-08 07:12:45 UTC
Created attachment 94856 [details]
projectapi refactor diff
Comment 24 mvfranz 2010-03-08 07:13:26 UTC
Created attachment 94857 [details]
projectimport.eclipse.core refactor diff
Comment 25 mvfranz 2010-03-08 07:13:56 UTC
Created attachment 94858 [details]
projectimport.eclipse.web refactor diff
Comment 26 mvfranz 2010-03-08 07:14:33 UTC
Created attachment 94859 [details]
projectimport.jbuilder refactor diff
Comment 27 mvfranz 2010-03-08 07:15:29 UTC
Created attachment 94860 [details]
ruby.project refactor diff
Comment 28 mvfranz 2010-03-08 07:16:47 UTC
Created attachment 94861 [details]
ruby.rakeproject refactor diff
Comment 29 mvfranz 2010-03-08 07:17:16 UTC
Created attachment 94862 [details]
web.freeform refactor diff
Comment 30 mvfranz 2010-03-08 07:17:43 UTC
Created attachment 94863 [details]
web.project refactor diff
Comment 31 mvfranz 2010-03-08 07:18:15 UTC
Created attachment 94864 [details]
j2ee.clientproject refactor diff
Comment 32 Jaroslav Tulach 2010-03-09 00:37:12 UTC
I am not sure I can see the whole picture in such amount of diffs. I'd suggest to create a named branch and integrate all these changesets into it. Now you shall be able to do it in your own team repository too. See http://wiki.netbeans.org/PrototypesRepo for details.

Y01 Versioning information is missing. Increment API module spec version, add @since to new methods, write an apichange.xml entry. 

Y02 All modules that start to use the new API need to depend on new spec version of the API module.

Otherwise I like the work and the amount of duplicated code it eliminates.
Comment 33 Jesse Glick 2010-03-09 07:39:58 UTC
(In reply to comment #32)
> I am not sure I can see the whole picture in such amount of diffs. I'd suggest
> to create a named branch and integrate all these changesets into it. Now you
> shall be able to do it in your own team repository too.

Probably mvfranz has no push access, so this is not an option. Attaching diffs is fine, but it would be better to attach one diff covering the whole repository - people can always ignore parts they are not interested in.
Comment 34 mvfranz 2010-03-09 18:10:31 UTC
I can upload one patch for all changes (trust me, adding 21 attachments was not fun).  I can do this before or after I make the two changes for the API bump.
Comment 35 Jaroslav Tulach 2010-03-10 10:43:56 UTC
Address the advices and then attach new patch.
Comment 36 Jesse Glick 2010-03-10 10:45:49 UTC
(In reply to comment #34)
> I can do this before or after I make the two changes for the API bump.

After, so we can see a final diff.

Other than lack of @since, apichanges.xml, and versioning updates, everything looks good, I think - thanks for the hard work! You seem to be listed under https://sca.dev.java.net/CA_signatories.htm#m so that's no problem. To the code in XMLUtil.java:


[JG01] Use generics (List<String>), and for-loops in preference to Iterator. Easier to read. E.g.:

  for (Element e : findSubElements(parent)) { ... }


[JG02] Interpretation of 'order' param in appendChildElement is not obvious and should be clarified in Javadoc: that it is a list of local names; that it may have no duplicates; what happens if the given element is not listed at all; what happens if the order cannot be satisfied for this or that reason; etc.


[JG03] Use IllegalArgumentException rather than asserts for inappropriate input, such as index == -1 in appendChildElement.


[JG04] Clarify that findElement(parent, name, null) matches _any_ namespace, rather than only elements with no namespace.


[JG05] Javadoc phrases such as "DOM provides a similar method" can be replaced with an appropriate {@link ...}.


[JG06] translateXml missing Javadoc fields.


[JG07] 'doc' parameter to copyDocument could probably be omitted from signature; just use to.getOwnerDocument().


[JG08] translateXml and copyDocument could probably be merged as they do much the same thing, though with different implementations - translateXml's use of Node.cloneNode(true) is more general than the limited cases in copyDocument. Since it may be useful to have both methods for different caller use cases, consider making one be implemented in terms of the other.
Comment 37 mvfranz 2010-03-10 17:37:20 UTC
Yes, I have already signed the license stuff for the OpenJDK (BSD/Haiku).

I can make all the suggested changes, not a problem.  Just to be clear, I did not write these new methods, I extracted them from the code that is being removed.  The only thing I did write were the unit tests.
Comment 38 Jesse Glick 2010-03-10 18:10:24 UTC
(In reply to comment #37)
> I did not write these new methods, I extracted them from the code
> that is being removed.

That's fine, but we have higher standards for API code - if the interface is wrong, we cannot change it later.
Comment 39 mvfranz 2010-03-10 21:01:20 UTC
(In reply to comment #36)
> [JG02] Interpretation of 'order' param in appendChildElement is not obvious and
> should be clarified in Javadoc: that it is a list of local names; that it may
> have no duplicates; what happens if the given element is not listed at all;
> what happens if the order cannot be satisfied for this or that reason; etc.
It is confusing, I think I understand it, but explaining it might be hard.
    Append a child element to the parent at the specified location.
    
      The <code>desiredAppendOrder<code> is an unique list of local names of existing
      child elements and the new element to be appended.  <em>All</em> existing child
      elements must be specified.  Children are not re-ordered
     
     
      @param parent parent to which the child will be appended
      @param el element to be added
      @param desiredAppendOrder order of the elements which must be followed
      @throws IllegalArgumentException if the order cannot be followed, either
      a missing existing child or missing new child entry from the desiredAppendOrder

What is hard to explain, is that the 'order' does two things.  1. ensure that all existing and the new child elements are specified, and 2. the element that we append before (insert before).  The order does not really matter other then what follows the new child.

> [JG04] Clarify that findElement(parent, name, null) matches _any_ namespace,
> rather than only elements with no namespace.
This is not how the implementation I picked works.  Since it is using 'tagName' when the namespace is null and not 'localName' <h:table> is not the same as <table>.  If I change it to use localName then <f:table> and <h:table> are the same, and there are now multiple elements with the same name

These are the two implementations I picked from:
 if ((namespace == null && name.equals(el.getTagName()))
       || (namespace != null && name.equals(el.getLocalName()) && namespace.equals(el.getNamespaceURI())))

 if (name.equals(el.getLocalName())
       && ((namespace != null && namespace.equals(el.getNamespaceURI())) || namespace == null))
I don't care which implementation is used.  I just picked the first one, and that it how I wrote my tests.
Comment 40 Jesse Glick 2010-03-11 07:56:15 UTC
(In reply to comment #39)
> What is hard to explain, is that the 'order' does two things.  1. ensure that
> all existing and the new child elements are specified, and 2. the element that
> we append before (insert before).  The order does not really matter other then
> what follows the new child.

Something like this. #1 is a precondition; #2 is the useful part. The intent is to make it easy to insert new child elements when an XML schema (such as for project.xml#//data) specifies a particular order but some items are optional or can be repeated. The order here should match the <xsd:sequence> in the schema. It is assumed you are starting with a valid document, and you want to ensure that it is still valid after the insertion.

> I don't care which implementation is used.  I just picked the first one, and
> that it how I wrote my tests.

But you need to pick the behavior that matches what the existing clients rely on, or the IDE will be quite broken. (*) Setting namespace=null is used when searching for a similar child element using any of a number of historical schema versions. For example, http://www.netbeans.org/ns/freeform-project-java/1.xsd defines a <built-to> which is comparable to the <built-to> in .../2.xsd; if java.freeform were calling this method on a parent <compilation-unit>, and it could be starting with a project.xml defined with either schema version, it would pass null for the namespace to match only by local name.

(*) Be sure to run unit tests of modules you are editing (ant test-unit -Dtest-unit-sys-prop.ignore.random.failures=true). If the tests passed before - which they should at least for those listed in nbbuild/hudson/core-main - they ought to pass after the refactoring too.
Comment 41 mvfranz 2010-03-14 18:04:05 UTC
(In reply to comment #40)
> But you need to pick the behavior that matches what the existing clients rely
> on, or the IDE will be quite broken. (*) Setting namespace=null is used when
> searching for a similar child element using any of a number of historical
> schema versions. For example,
> http://www.netbeans.org/ns/freeform-project-java/1.xsd defines a <built-to>
> which is comparable to the <built-to> in .../2.xsd; if java.freeform were
> calling this method on a parent <compilation-unit>, and it could be starting
> with a project.xml defined with either schema version, it would pass null for
> the namespace to match only by local name.
> 
> (*) Be sure to run unit tests of modules you are editing (ant test-unit
> -Dtest-unit-sys-prop.ignore.random.failures=true). If the tests passed before -
> which they should at least for those listed in nbbuild/hudson/core-main - they
> ought to pass after the refactoring too.

I have run the unit tests on all the projects using both implementations and all projects give me the same errors as without my changes.  There seem to be more failures on OS X since Javac and rt.jar are different.
At this point, it seems that the unit tests do not test this functionality.  If null matching is the desired logic, I can change the new code and tests to match that.
Comment 42 mvfranz 2010-03-14 21:18:17 UTC
The usages that I found of passing null for a namespace are:
compares tagName and null namespace
 - apisupport.project (3)
 - projectimport.jbuilder (1)
comares localname and null namespace
 - projectimport.eclipse.core (7)
 - projectimport.eclipse.web (2)

All other projects specify the namespace.
Comment 43 Jesse Glick 2010-03-15 18:39:46 UTC
(In reply to comment #41)
> If null matching is the desired logic

Yes, passing null for namespace should mean "match any element with the given tag name, regardless of what namespace - if any - it is in". This ought to work whether the document was parsed with namespace support on or off, or created in memory using DOM 1 or DOM 2+ methods.
Comment 44 mvfranz 2010-03-28 00:24:01 UTC
Created attachment 96080 [details]
XMLUtil patches for refactored methods

Here are my latest changes for XMLUtil, sorry it took so long,  I had some issue after I did a pull -u and redid the changes.

I made the changes mentioned, all but the combing copyDocument and translateXML.  I looked at it, and made them even more similar, but when I tried to combine/re-use the logic something when horribly wrong.

The only additional change I did was to use localName and nodeName for findElement.  It seems that the parser using by projectimport.jbuilder does not set localName on nodes.  Not sure if this is the DOM1 vs DOM2 logic that was mentioned.

This should apply to main as of today.  I will upload the rest of the patch once requested.
Comment 45 mvfranz 2010-03-29 11:11:26 UTC
Created attachment 96166 [details]
Patch for modules to use new XMLUtil methods

This is the patch that removes the refactored methods and changes the usage to XMLUtil version.  It should also contain the bumping of the required openide.util module to 8.3.
Comment 46 Jesse Glick 2010-03-31 03:40:07 UTC
(Best to attach a single patch at a time, with all API & usage together.)


JG02 - appendChildElement Javadoc is still a bit mysterious; the summary in this issue was more helpful than the Javadoc I see in the patch. "the existing item" is unspecified, and it is not mentioned that elements of 'order' are element local names. But I guess I can fix this kind of thing up when applying the patch.


JG06 still open. Again not a blocker.


[JG09] Any reason you skipped defaultErrorHandler (e.g. from org.netbeans.modules.projectimport.eclipse.core.Util)? Something like this is used quite widely. Does not have to be done as part of this issue if you are out of time, of course, but seems to fall into the same category as the other methods.


[JG10] Junk in ant.freeform.Actions.


[JG11] Dependencies in client modules are not consistently updated, e.g. in apisupport.project and j2ee.archive (maybe others). In other cases (e.g. j2ee.clientproject) test dependencies are changed but not main module dependencies.

All client modules should have one dependency change in project.xml: to use the new rev of org.openide.util.
Comment 47 mvfranz 2010-03-31 10:47:02 UTC
(In reply to comment #46)
> (Best to attach a single patch at a time, with all API & usage together.)
> 
> 
> JG02 - appendChildElement Javadoc is still a bit mysterious; the summary in
> this issue was more helpful than the Javadoc I see in the patch. "the existing
> item" is unspecified, and it is not mentioned that elements of 'order' are
> element local names. But I guess I can fix this kind of thing up when applying
> the patch.
> 
> 
> JG06 still open. Again not a blocker.
> 
I will take another look at these.
> 
> [JG09] Any reason you skipped defaultErrorHandler (e.g. from
> org.netbeans.modules.projectimport.eclipse.core.Util)? Something like this is
> used quite widely. Does not have to be done as part of this issue if you are
> out of time, of course, but seems to fall into the same category as the other
> methods.
> 
There are a few defaultErrorHandler methods spread throughout the different projects.  I did not touch any of them.  I can look at including them as well.
> 
> [JG10] Junk in ant.freeform.Actions.
> 
> 
> [JG11] Dependencies in client modules are not consistently updated, e.g. in
> apisupport.project and j2ee.archive (maybe others). In other cases (e.g.
> j2ee.clientproject) test dependencies are changed but not main module
> dependencies.
> 
When changing some of the dependencies I got errors, here is one bug #178584, guess I will have to change the files manually.

> All client modules should have one dependency change in project.xml: to use the
> new rev of org.openide.util.
Comment 48 mvfranz 2010-04-01 02:37:33 UTC
(In reply to comment #46)
> (Best to attach a single patch at a time, with all API & usage together.)
> JG06 still open. Again not a blocker.
Added some descriptions to the parameters.
> 
> 
> [JG09] Any reason you skipped defaultErrorHandler (e.g. from
> org.netbeans.modules.projectimport.eclipse.core.Util)? Something like this is
> used quite widely. Does not have to be done as part of this issue if you are
> out of time, of course, but seems to fall into the same category as the other
> methods.
> 
Looked at this.  questions are:
Exceptions.attachMessage vs ErrorManager.getDefault().annotate
Logger.getLogger(Util.class.getName()).log vs ErrorManager.getDefault().notify

These are from diff -u projectimport.eclipse.web/src/org/netbeans/modules/projectimport/eclipse/web/Util.java ruby.rakeproject/src/org/netbeans/modules/ruby/modules/project/rake/Util.java

Ruby Rake is different from the other three instances I found in the way exceptions are logged.  Is one method preferred over another?

> 
> [JG10] Junk in ant.freeform.Actions.
Removed
Comment 49 Jesse Glick 2010-04-01 14:23:40 UTC
(In reply to comment #48)
>> [JG09] defaultErrorHandler
> Exceptions.attachMessage vs ErrorManager.getDefault().annotate
> Logger.getLogger(Util.class.getName()).log vs ErrorManager.getDefault().notify

ErrorManager is effectively deprecated; do not use it for new code. Use Logger.getLogger(Whatever.class.getName()) for logging, as in projectimport.eclipse.*.
Comment 50 mvfranz 2010-04-04 13:36:38 UTC
Created attachment 96651 [details]
All changes for XMLUtil

This has all the changes needed to refactor the XML methods into XMLUtil.  The only item that needs work is the appendChildElement documentation (AFAIK).

I manually changed the j2ee.archive dependency as the UI does not allow it.  I tried to recreate the dependencies, but there seem to be circular dependency with the j2ee modules.
Comment 51 Jesse Glick 2010-04-05 18:13:28 UTC
JG11 not fully addressed; see j2ee.clientproject/nbproject/project.xml. j2ee.earproject also looks incorrect, maven and profiler.nbmodule are not updated at all, and there may be others wrong. Please review the patch yourself and double-check all the dep changes.


Should be possible to get this patch applied this week in time for 6.9 Beta cutoff on Friday.
Comment 52 mvfranz 2010-04-05 23:38:42 UTC
Created attachment 96724 [details]
Refactored code for XMLUtil

I rechecked the project dependencies.  It seems that some hg pull -u I did undid my dependency changes.
Comment 53 Jesse Glick 2010-04-05 23:48:32 UTC
(In reply to comment #52)
> Created an attachment (id=96724) [details]
> Refactored code for XMLUtil

This is essentially the same patch as before, with the exception of line numbers and

+import java.util.logging.LogRecord;

Please, read your own diff before attaching. 'diffstat -p1 whatever.diff' is also helpful to get an overview of what files are modified.
Comment 54 mvfranz 2010-04-07 04:48:02 UTC
Created attachment 96832 [details]
XMLUtil Refactoring patch

I have rechecked the changes.  It seems that the UI cannot change the dependencies (project.xml) so I had to edit them manually.  I ran diffstat and scrolled through the patch.  Everything looks good (but I could be wrong).  I applied to an updated pull (different repo) and only the openide.util manifest has a conflict - version already been bump to 8.3.
Comment 55 Jesse Glick 2010-04-07 17:15:36 UTC
(In reply to comment #54)
> It seems that the UI cannot change the dependencies

Probably in a new dev build it can; you're on CC for the bug so you should see notification when the fix makes it into a nightly.

> applied to an updated pull (different repo) and only the openide.util manifest
> has a conflict - version already been bump to 8.3.

Which means that this patch needs to be updated to change 8.3 to 8.4, not just in openide.util/manifest.mf, but in every nbproject/project.xml which updates the dep on org.openide.util, and in @since tags and apichanges.xml. Sorry, but there is no clean way to merge API version increments, and someone else happened to be doing a change in org.openide.util recently (CharSequences).
Comment 56 mvfranz 2010-04-08 02:15:27 UTC
Created attachment 96877 [details]
XMLUtil Refactoring patch

Changed the version to 8.4,  API doc to say seven instead of six new methods, updated date to today.
Comment 57 Jesse Glick 2010-04-08 03:05:27 UTC
Minor patch problem: openide.util 8.2 -> 8.4 was rejected (should have been 8.3 -> 8.4).

Thank you for your hard work! core-main #44728f9b507e
Comment 58 Quality Engineering 2010-04-09 04:37:43 UTC
Integrated into 'main-golden', will be available in build *201004090201* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/44728f9b507e
User: mvfranz@netbeans.org
Log: #136595: DOM utility methods to add to org.openide.xml.XMLUtil.


By use of this website, you agree to the NetBeans Policies and Terms of Use. © 2012, Oracle Corporation and/or its affiliates. Sponsored by Oracle logo