elharo commented on code in PR #7:
URL: https://github.com/apache/xerces-j/pull/7#discussion_r2173731012


##########
tests/loc/XercesDOMLocationTest.java:
##########
@@ -0,0 +1,150 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package loc;
+
+import junit.framework.TestCase;
+import org.apache.xerces.parsers.DOMParser;
+
+import static 
org.apache.xerces.parsers.XML11Configuration.LOCATION_INFO_FEATURE;
+
+import org.apache.xerces.dom.NodeImpl;
+
+import org.w3c.dom.DOMLocator;
+import org.w3c.dom.Document;
+import org.w3c.dom.Element;
+import org.w3c.dom.Node;
+import org.w3c.dom.NodeList;
+import org.xml.sax.SAXException;
+import org.xml.sax.SAXNotRecognizedException;
+import org.xml.sax.SAXNotSupportedException;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+
+public class XercesDOMLocationTest extends TestCase {
+
+    private static final String TEST_XML_PATH = "tests/loc/test.xml";
+
+    private static List<Element> getElementChildren(Element parent) {
+        List<Element> result = new ArrayList<>();
+        NodeList children = parent.getChildNodes();
+        for (int i = 0; i < children.getLength(); i++) {
+            Node node = children.item(i);
+            if (node.getNodeType() == Node.ELEMENT_NODE) {
+                result.add((Element) node);
+            }
+        }
+        return result;
+    }
+
+    private Document parseXML(Boolean enableLocationFeature) throws 
SAXException, IOException {
+        DOMParser parser = new DOMParser();
+        if (enableLocationFeature != null)
+            parser.setFeature(LOCATION_INFO_FEATURE, enableLocationFeature);
+        parser.parse(new File(TEST_XML_PATH).toURI().toString());
+        Document doc = parser.getDocument();
+        return doc;
+    }
+
+    private DOMLocator getLocator(Node n) {
+        return ((NodeImpl) n).getLocator();
+    }
+
+    /**
+     * This test verifies that the XML tree we get from the parse is
+     * the expected one.
+     *
+     * The other tests focus on the location info such as line numbers
+     * and they do not have to repeat this verification.
+     */
+    public void testXMLIsExpectedTree() throws IOException, SAXException {
+        Document doc = parseXML(null); // Feature defaults
+        Element root = doc.getDocumentElement();
+        String name = root.getTagName();
+        assertEquals("root", name);
+        List<Element> ec = getElementChildren(root);

Review Comment:
   ec --> children



##########
src/org/apache/xerces/parsers/XML11Configuration.java:
##########
@@ -503,7 +507,7 @@ public XML11Configuration(
                        EXTERNAL_GENERAL_ENTITIES,  
                        EXTERNAL_PARAMETER_ENTITIES,
                        PARSER_SETTINGS,
-                       
+                LOCATION_INFO_FEATURE

Review Comment:
   indent looks off



##########
tests/loc/XercesDOMLocationTest.java:
##########
@@ -0,0 +1,150 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package loc;
+
+import junit.framework.TestCase;
+import org.apache.xerces.parsers.DOMParser;
+
+import static 
org.apache.xerces.parsers.XML11Configuration.LOCATION_INFO_FEATURE;
+
+import org.apache.xerces.dom.NodeImpl;
+
+import org.w3c.dom.DOMLocator;
+import org.w3c.dom.Document;
+import org.w3c.dom.Element;
+import org.w3c.dom.Node;
+import org.w3c.dom.NodeList;
+import org.xml.sax.SAXException;
+import org.xml.sax.SAXNotRecognizedException;
+import org.xml.sax.SAXNotSupportedException;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+
+public class XercesDOMLocationTest extends TestCase {
+
+    private static final String TEST_XML_PATH = "tests/loc/test.xml";
+
+    private static List<Element> getElementChildren(Element parent) {
+        List<Element> result = new ArrayList<>();
+        NodeList children = parent.getChildNodes();
+        for (int i = 0; i < children.getLength(); i++) {
+            Node node = children.item(i);
+            if (node.getNodeType() == Node.ELEMENT_NODE) {
+                result.add((Element) node);
+            }
+        }
+        return result;
+    }
+
+    private Document parseXML(Boolean enableLocationFeature) throws 
SAXException, IOException {
+        DOMParser parser = new DOMParser();
+        if (enableLocationFeature != null)
+            parser.setFeature(LOCATION_INFO_FEATURE, enableLocationFeature);
+        parser.parse(new File(TEST_XML_PATH).toURI().toString());
+        Document doc = parser.getDocument();
+        return doc;
+    }
+
+    private DOMLocator getLocator(Node n) {
+        return ((NodeImpl) n).getLocator();
+    }
+
+    /**
+     * This test verifies that the XML tree we get from the parse is
+     * the expected one.
+     *
+     * The other tests focus on the location info such as line numbers
+     * and they do not have to repeat this verification.
+     */
+    public void testXMLIsExpectedTree() throws IOException, SAXException {
+        Document doc = parseXML(null); // Feature defaults
+        Element root = doc.getDocumentElement();
+        String name = root.getTagName();
+        assertEquals("root", name);
+        List<Element> ec = getElementChildren(root);
+        assertEquals(3, ec.size());
+        Element n1 = ec.get(0);
+        Element n2 = ec.get(1);
+        Element n3 = ec.get(2);
+        assertEquals("child", n1.getTagName());
+        assertEquals("selfClosing", n2.getTagName());
+        assertEquals("another", n3.getTagName());
+    }
+
+    public void testFeatureDisabledByDefault() throws IOException, 
SAXException {
+        Document doc = parseXML(null); // Feature defaults

Review Comment:
   Weird. I expect this to throw a NullPointerException



##########
tests/loc/XercesDOMLocationTest.java:
##########
@@ -0,0 +1,150 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package loc;
+
+import junit.framework.TestCase;
+import org.apache.xerces.parsers.DOMParser;
+
+import static 
org.apache.xerces.parsers.XML11Configuration.LOCATION_INFO_FEATURE;
+
+import org.apache.xerces.dom.NodeImpl;
+
+import org.w3c.dom.DOMLocator;
+import org.w3c.dom.Document;
+import org.w3c.dom.Element;
+import org.w3c.dom.Node;
+import org.w3c.dom.NodeList;
+import org.xml.sax.SAXException;
+import org.xml.sax.SAXNotRecognizedException;
+import org.xml.sax.SAXNotSupportedException;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+
+public class XercesDOMLocationTest extends TestCase {
+
+    private static final String TEST_XML_PATH = "tests/loc/test.xml";
+
+    private static List<Element> getElementChildren(Element parent) {
+        List<Element> result = new ArrayList<>();
+        NodeList children = parent.getChildNodes();
+        for (int i = 0; i < children.getLength(); i++) {
+            Node node = children.item(i);
+            if (node.getNodeType() == Node.ELEMENT_NODE) {
+                result.add((Element) node);
+            }
+        }
+        return result;
+    }
+
+    private Document parseXML(Boolean enableLocationFeature) throws 
SAXException, IOException {
+        DOMParser parser = new DOMParser();
+        if (enableLocationFeature != null)
+            parser.setFeature(LOCATION_INFO_FEATURE, enableLocationFeature);
+        parser.parse(new File(TEST_XML_PATH).toURI().toString());
+        Document doc = parser.getDocument();
+        return doc;
+    }
+
+    private DOMLocator getLocator(Node n) {
+        return ((NodeImpl) n).getLocator();
+    }
+
+    /**
+     * This test verifies that the XML tree we get from the parse is
+     * the expected one.
+     *
+     * The other tests focus on the location info such as line numbers
+     * and they do not have to repeat this verification.
+     */
+    public void testXMLIsExpectedTree() throws IOException, SAXException {
+        Document doc = parseXML(null); // Feature defaults
+        Element root = doc.getDocumentElement();
+        String name = root.getTagName();
+        assertEquals("root", name);
+        List<Element> ec = getElementChildren(root);
+        assertEquals(3, ec.size());
+        Element n1 = ec.get(0);

Review Comment:
   n1 is what? don't abbreviate variable names



##########
tests/loc/XercesDOMLocationTest.java:
##########
@@ -0,0 +1,150 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package loc;
+
+import junit.framework.TestCase;
+import org.apache.xerces.parsers.DOMParser;
+
+import static 
org.apache.xerces.parsers.XML11Configuration.LOCATION_INFO_FEATURE;
+
+import org.apache.xerces.dom.NodeImpl;
+
+import org.w3c.dom.DOMLocator;
+import org.w3c.dom.Document;
+import org.w3c.dom.Element;
+import org.w3c.dom.Node;
+import org.w3c.dom.NodeList;
+import org.xml.sax.SAXException;
+import org.xml.sax.SAXNotRecognizedException;
+import org.xml.sax.SAXNotSupportedException;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+
+public class XercesDOMLocationTest extends TestCase {
+
+    private static final String TEST_XML_PATH = "tests/loc/test.xml";
+
+    private static List<Element> getElementChildren(Element parent) {
+        List<Element> result = new ArrayList<>();
+        NodeList children = parent.getChildNodes();
+        for (int i = 0; i < children.getLength(); i++) {
+            Node node = children.item(i);
+            if (node.getNodeType() == Node.ELEMENT_NODE) {
+                result.add((Element) node);
+            }
+        }
+        return result;
+    }
+
+    private Document parseXML(Boolean enableLocationFeature) throws 
SAXException, IOException {
+        DOMParser parser = new DOMParser();
+        if (enableLocationFeature != null)
+            parser.setFeature(LOCATION_INFO_FEATURE, enableLocationFeature);
+        parser.parse(new File(TEST_XML_PATH).toURI().toString());
+        Document doc = parser.getDocument();

Review Comment:
   doc --> document



##########
docs/features.xml:
##########
@@ -588,6 +588,21 @@ catch (SAXException e) {
     This feature is relevant only when the grammar is <strong>DTD</strong>. 
    </note>
   </feature>
+  <feature name='http://apache.org/xml/features/dom/include-location-info'
+           id='dom.include-location-info'>
+   <true>
+    Include a DOMLocator for each DOM Element giving its line and column
+    information.
+   </true>
+   <false>Do not include a DOMLocator for each DOM Element.</false>
+   <default value='false'/>
+   <note>
+    DOM Elements must be downcast to <code>NodeImpl</code> to access
+    the <code>getLocator():DOMLocator</code> method since it is not part of 
the standard w3c API.

Review Comment:
   W3C



##########
tests/loc/XercesDOMLocationTest.java:
##########
@@ -0,0 +1,150 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package loc;
+
+import junit.framework.TestCase;
+import org.apache.xerces.parsers.DOMParser;
+
+import static 
org.apache.xerces.parsers.XML11Configuration.LOCATION_INFO_FEATURE;
+
+import org.apache.xerces.dom.NodeImpl;
+
+import org.w3c.dom.DOMLocator;
+import org.w3c.dom.Document;
+import org.w3c.dom.Element;
+import org.w3c.dom.Node;
+import org.w3c.dom.NodeList;
+import org.xml.sax.SAXException;
+import org.xml.sax.SAXNotRecognizedException;
+import org.xml.sax.SAXNotSupportedException;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+
+public class XercesDOMLocationTest extends TestCase {
+
+    private static final String TEST_XML_PATH = "tests/loc/test.xml";
+
+    private static List<Element> getElementChildren(Element parent) {
+        List<Element> result = new ArrayList<>();
+        NodeList children = parent.getChildNodes();
+        for (int i = 0; i < children.getLength(); i++) {
+            Node node = children.item(i);
+            if (node.getNodeType() == Node.ELEMENT_NODE) {
+                result.add((Element) node);
+            }
+        }
+        return result;
+    }
+
+    private Document parseXML(Boolean enableLocationFeature) throws 
SAXException, IOException {

Review Comment:
   rename this method. As is, it sounds like the XML to parse should be passed 
in as an argument. Maybe loadTestData or something like that.



##########
src/org/apache/xerces/parsers/AbstractDOMParser.java:
##########
@@ -933,6 +942,18 @@ public void startElement (QName element, XMLAttributes 
attributes, Augmentations
                 return;
             }
             Element el = createElementNode (element);
+            // Extract location info if feature is enabled
+            if (fIncludeLocationInfo) {
+                DOMLocator elemLoc = new DOMLocatorImpl(

Review Comment:
   elemLoc --> elementLocation



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: j-dev-unsubscr...@xerces.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: j-dev-unsubscr...@xerces.apache.org
For additional commands, e-mail: j-dev-h...@xerces.apache.org

Reply via email to