Merge pull request #399 from wader/xml-more-ns

xml: Even more namespace fixes

format/xml/testdata/test.svg.fqtest

@@ -1,4 +1,4 @@
-$ fq . test.svg
+$ fq -r '. as $a | ., (toxml({indent: 2}) | ., (fromxml | ., (diff($a; .) // "no diff")))' test.svg
 {
   "svg": {
     "-height": "2500",
@@ -22,7 +22,35 @@ $ fq . test.svg
     }
   }
 }
-$ fq -o array=true . test.svg
+<svg height="2500" inkscape:version="1.0 (4035a4f, 2020-05-01)" sodipodi:docname="ffclippy.svg" version="1.1" viewBox="0 0 192.756 192.756" width="2500" xmlns="http://www.w3.org/2000/svg" xmlns:inkscape="http://www.inkscape.org/namespaces/inkscape" xmlns:sodipodi="http://sodipodi.sourceforge.net/DTD/sodipodi-0.dtd" xmlns:svg="http://www.w3.org/2000/svg">
+  <g clip-rule="evenodd" fill-rule="evenodd" id="g863"></g>
+  <sodipodi:namedview pagecolor="#ffffff" showgrid="false"></sodipodi:namedview>
+</svg>
+{
+  "svg": {
+    "-height": "2500",
+    "-inkscape:version": "1.0 (4035a4f, 2020-05-01)",
+    "-sodipodi:docname": "ffclippy.svg",
+    "-version": "1.1",
+    "-viewBox": "0 0 192.756 192.756",
+    "-width": "2500",
+    "-xmlns": "http://www.w3.org/2000/svg",
+    "-xmlns:inkscape": "http://www.inkscape.org/namespaces/inkscape",
+    "-xmlns:sodipodi": "http://sodipodi.sourceforge.net/DTD/sodipodi-0.dtd",
+    "-xmlns:svg": "http://www.w3.org/2000/svg",
+    "g": {
+      "-clip-rule": "evenodd",
+      "-fill-rule": "evenodd",
+      "-id": "g863"
+    },
+    "sodipodi:namedview": {
+      "-pagecolor": "#ffffff",
+      "-showgrid": "false"
+    }
+  }
+}
+no diff
+$ fq -r -o array=true '. as $a | ., (toxml({indent: 2}) | ., (fromxml | ., (diff($a; .) // "no diff")))' test.svg
 [
   "svg",
   {
@@ -57,3 +85,42 @@ $ fq -o array=true . test.svg
     ]
   ]
 ]
+<svg height="2500" inkscape:version="1.0 (4035a4f, 2020-05-01)" sodipodi:docname="ffclippy.svg" version="1.1" viewBox="0 0 192.756 192.756" width="2500" xmlns="http://www.w3.org/2000/svg" xmlns:inkscape="http://www.inkscape.org/namespaces/inkscape" xmlns:sodipodi="http://sodipodi.sourceforge.net/DTD/sodipodi-0.dtd" xmlns:svg="http://www.w3.org/2000/svg">
+  <sodipodi:namedview pagecolor="#ffffff" showgrid="false"></sodipodi:namedview>
+  <g clip-rule="evenodd" fill-rule="evenodd" id="g863"></g>
+</svg>
+[
+  "svg",
+  {
+    "height": "2500",
+    "inkscape:version": "1.0 (4035a4f, 2020-05-01)",
+    "sodipodi:docname": "ffclippy.svg",
+    "version": "1.1",
+    "viewBox": "0 0 192.756 192.756",
+    "width": "2500",
+    "xmlns": "http://www.w3.org/2000/svg",
+    "xmlns:inkscape": "http://www.inkscape.org/namespaces/inkscape",
+    "xmlns:sodipodi": "http://sodipodi.sourceforge.net/DTD/sodipodi-0.dtd",
+    "xmlns:svg": "http://www.w3.org/2000/svg"
+  },
+  [
+    [
+      "sodipodi:namedview",
+      {
+        "pagecolor": "#ffffff",
+        "showgrid": "false"
+      },
+      []
+    ],
+    [
+      "g",
+      {
+        "clip-rule": "evenodd",
+        "fill-rule": "evenodd",
+        "id": "g863"
+      },
+      []
+    ]
+  ]
+]
+no diff

format/xml/xml.go

@@ -3,7 +3,8 @@ package xml
 // object mode inspired by https://www.xml.com/pub/a/2006/05/31/converting-between-xml-and-json.html
 
 // TODO: keep <?xml>? root #desc?
-// TODO: xml default indent?
+// TODO: refactor to share more code
+// TODO: rewrite ns stack
 
 import (
 	"bytes"
@@ -74,24 +75,26 @@ type xmlNS struct {
 	url  string
 }
 
-func elmName(space, local string) string {
+// TODO: nss pop? attr not stack?
-	if space == "" {
-		return local
-	}
-	return space + ":" + local
-}
 
 // xmlNNStack is used to undo namespace url resolving, space is url not the "alias" name
 type xmlNNStack []xmlNS
 
 func (nss xmlNNStack) lookup(name xml.Name) string {
+	var s string
 	for i := len(nss) - 1; i >= 0; i-- {
 		ns := nss[i]
 		if name.Space == ns.url {
-			return ns.name
+			// first found or is default namespace
+			if s == "" || ns.name == "" {
+				s = ns.name
+			}
+			if s == "" {
+				break
 			}
 		}
-	return ""
+	}
+	return s
 }
 
 func (nss xmlNNStack) push(name string, url string) xmlNNStack {
@@ -100,57 +103,11 @@ func (nss xmlNNStack) push(name string, url string) xmlNNStack {
 	return xmlNNStack(n)
 }
 
-func fromXMLToArray(n xmlNode) any {
+func elmName(space, local string) string {
-	var f func(n xmlNode, nss xmlNNStack) []any
+	if space == "" {
-	f = func(n xmlNode, nss xmlNNStack) []any {
+		return local
-		attrs := map[string]any{}
-		for _, a := range n.Attrs {
-			local, space := a.Name.Local, a.Name.Space
-			name := local
-			if space != "" {
-				if space == "xmlns" {
-					nss = nss.push(local, a.Value)
-				} else {
-					space = nss.lookup(a.Name)
 	}
-				name = space + ":" + local
+	return space + ":" + local
-			} else if local == "xmlns" {
-				// track default namespace
-				nss = nss.push("", a.Value)
-			}
-			attrs[name] = a.Value
-		}
-		if attrs["#text"] == nil && !whitespaceRE.Match(n.Chardata) {
-			attrs["#text"] = strings.TrimSpace(string(n.Chardata))
-		}
-		if attrs["#comment"] == nil && !whitespaceRE.Match(n.Comment) {
-			attrs["#comment"] = strings.TrimSpace(string(n.Comment))
-		}
-
-		nodes := []any{}
-		for _, c := range n.Nodes {
-			nodes = append(nodes, f(c, nss))
-		}
-
-		local, space := n.XMLName.Local, n.XMLName.Space
-		if space != "" {
-			space = nss.lookup(n.XMLName)
-		}
-		name := elmName(space, local)
-
-		elm := []any{name}
-		if len(attrs) > 0 {
-			elm = append(elm, attrs)
-		} else {
-			// make attrs null if there were none, jq allows index into null
-			elm = append(elm, nil)
-		}
-		elm = append(elm, nodes)
-
-		return elm
-	}
-
-	return f(n, nil)
 }
 
 func fromXMLToObject(n xmlNode, xi format.XMLIn) any {
@@ -160,19 +117,25 @@ func fromXMLToObject(n xmlNode, xi format.XMLIn) any {
 
 		for _, a := range n.Attrs {
 			local, space := a.Name.Local, a.Name.Space
-			name := local
-			if space != "" {
 			if space == "xmlns" {
 				nss = nss.push(local, a.Value)
-				} else {
-					space = nss.lookup(a.Name)
-				}
-				name = space + ":" + local
 			} else if local == "xmlns" {
 				// track default namespace
 				nss = nss.push("", a.Value)
 			}
+		}
 
+		for _, a := range n.Attrs {
+			local, space := a.Name.Local, a.Name.Space
+			name := local
+			if space != "" {
+				if space == "xmlns" {
+					// nop
+				} else {
+					space = nss.lookup(a.Name)
+				}
+				name = elmName(space, local)
+			}
 			attrs["-"+name] = a.Value
 		}
 
@@ -224,6 +187,64 @@ func fromXMLToObject(n xmlNode, xi format.XMLIn) any {
 	return map[string]any{name: attrs}
 }
 
+func fromXMLToArray(n xmlNode) any {
+	var f func(n xmlNode, nss xmlNNStack) []any
+	f = func(n xmlNode, nss xmlNNStack) []any {
+		attrs := map[string]any{}
+
+		for _, a := range n.Attrs {
+			local, space := a.Name.Local, a.Name.Space
+			if space == "xmlns" {
+				nss = nss.push(local, a.Value)
+			} else if local == "xmlns" {
+				// track default namespace
+				nss = nss.push("", a.Value)
+			}
+		}
+
+		for _, a := range n.Attrs {
+			local, space := a.Name.Local, a.Name.Space
+			name := local
+			if space != "" {
+				if space == "xmlns" {
+					// nop
+				} else {
+					space = nss.lookup(a.Name)
+				}
+				name = elmName(space, local)
+			}
+			attrs[name] = a.Value
+		}
+
+		if attrs["#text"] == nil && !whitespaceRE.Match(n.Chardata) {
+			attrs["#text"] = strings.TrimSpace(string(n.Chardata))
+		}
+		if attrs["#comment"] == nil && !whitespaceRE.Match(n.Comment) {
+			attrs["#comment"] = strings.TrimSpace(string(n.Comment))
+		}
+
+		nodes := []any{}
+		for _, c := range n.Nodes {
+			nodes = append(nodes, f(c, nss))
+		}
+
+		name := elmName(nss.lookup(n.XMLName), n.XMLName.Local)
+
+		elm := []any{name}
+		if len(attrs) > 0 {
+			elm = append(elm, attrs)
+		} else {
+			// make attrs null if there were none, jq allows index into null
+			elm = append(elm, nil)
+		}
+		elm = append(elm, nodes)
+
+		return elm
+	}
+
+	return f(n, nil)
+}
+
 func decodeXML(d *decode.D, in any) any {
 	xi, _ := in.(format.XMLIn)
 
@@ -289,6 +310,20 @@ type ToXMLOpts struct {
 	Indent int
 }
 
+func xmlNameFromStr(s string) xml.Name {
+	return xml.Name{Local: s}
+}
+
+func xmlNameSort(a, b xml.Name) bool {
+	if a.Space != b.Space {
+		if a.Space == "" {
+			return true
+		}
+		return a.Space < b.Space
+	}
+	return a.Local < b.Local
+}
+
 func toXMLFromObject(c any, opts ToXMLOpts) any {
 	var f func(name string, content any) (xmlNode, int, bool)
 	f = func(name string, content any) (xmlNode, int, bool) {
@@ -319,10 +354,11 @@ func toXMLFromObject(c any, opts ToXMLOpts) any {
 					n.Comment = []byte(s)
 				case strings.HasPrefix(k, "-"):
 					s, _ := v.(string)
-					n.Attrs = append(n.Attrs, xml.Attr{
+					a := xml.Attr{
-						Name:  xml.Name{Local: k[1:]},
+						Name:  xmlNameFromStr(k[1:]),
 						Value: s,
-					})
+					}
+					n.Attrs = append(n.Attrs, a)
 				default:
 					switch v := v.(type) {
 					case []any:
@@ -360,8 +396,7 @@ func toXMLFromObject(c any, opts ToXMLOpts) any {
 		}
 
 		sort.Slice(n.Attrs, func(i, j int) bool {
-			a, b := n.Attrs[i].Name, n.Attrs[j].Name
+			return xmlNameSort(n.Attrs[i].Name, n.Attrs[j].Name)
-			return a.Space < b.Space || a.Local < b.Local
 		})
 
 		return n, seq, hasSeq
@@ -415,7 +450,7 @@ func toXMLFromArray(c any, opts ToXMLOpts) any {
 		}
 
 		n := xmlNode{
-			XMLName: xml.Name{Local: name},
+			XMLName: xmlNameFromStr(name),
 		}
 
 		for k, v := range attrs {
@@ -429,15 +464,14 @@ func toXMLFromArray(c any, opts ToXMLOpts) any {
 			default:
 				s, _ := v.(string)
 				n.Attrs = append(n.Attrs, xml.Attr{
-					Name:  xml.Name{Local: k},
+					Name:  xmlNameFromStr(k),
 					Value: s,
 				})
 			}
 		}
 
 		sort.Slice(n.Attrs, func(i, j int) bool {
-			a, b := n.Attrs[i].Name, n.Attrs[j].Name
+			return xmlNameSort(n.Attrs[i].Name, n.Attrs[j].Name)
-			return a.Space < b.Space || a.Local < b.Local
 		})
 
 		for _, c := range children {