bodewig commented on code in PR #111:
URL: https://github.com/apache/ant-ivy/pull/111#discussion_r2807852653
##########
src/java/org/apache/ivy/plugins/parser/xml/XmlModuleDescriptorParser.java:
##########
@@ -722,6 +716,17 @@ public void mergeExcludes(ExcludeRule[] excludeRules) {
}
}
+ private void mergeNamespaces(Map<String, String> namespaces) {
+ if (namespaces != null && !namespaces.isEmpty()) {
+ for (Map.Entry<String, String> entry : namespaces.entrySet()) {
+ Message.debug("Merging extra attribute namesapce: " +
entry);
+ if
(getMd().getExtraAttributesNamespaces().get(entry.getKey()) == null) {
Review Comment:
this looks the wrong way around to me.
XML namespace prefixes are completely irrelevant to the XML parser. So if
the parent POM defines a different prefix to the same namespace URI, than
nothing needs to be done (only make sure the attributes use the prefix used by
the merged module descriptor).
You probably want to add an entry with a prefix of your choosing if there is
no entry for the namespace URI - i.e. if there is no entry.getValue() is not
present as value in the namespaces.
##########
src/java/org/apache/ivy/plugins/parser/xml/XmlModuleDescriptorUpdater.java:
##########
@@ -409,13 +410,22 @@ public void startElement(String uri, String localName,
String qName, Attributes
// copy
write("<" + qName);
- for (int i = 0; i < attributes.getLength(); i++) {
- write(" " + attributes.getQName(i) + "=\""
- + substitute(settings, attributes.getValue(i)) +
"\"");
+ if (options.isMerge() && path.equals("ivy-module")) {
+ for (int i = 0, n = attributes.getLength(); i < n; i += 1)
{
+ if (attributes.getQName(i).startsWith("xmlns:"))
continue;
+ write(" " + attributes.getQName(i) + "=\"" +
substitute(settings, attributes.getValue(i)) + "\"");
+ }
+ Map<String, String> namespaces =
options.getMergedDescriptor().getExtraAttributesNamespaces();
+ for (Map.Entry<String, String> namespace :
namespaces.entrySet()) {
+ write(" xmlns:" + namespace.getKey() + "=\"" +
substitute(settings, namespace.getValue()) + "\"");
+ }
+ } else {
+ for (int i = 0, n = attributes.getLength(); i < n; i += 1)
{
Review Comment:
the `i += 1` rather than `i++` makes me itch :-)
##########
src/java/org/apache/ivy/plugins/parser/xml/XmlModuleDescriptorUpdater.java:
##########
@@ -409,13 +410,22 @@ public void startElement(String uri, String localName,
String qName, Attributes
// copy
write("<" + qName);
- for (int i = 0; i < attributes.getLength(); i++) {
- write(" " + attributes.getQName(i) + "=\""
- + substitute(settings, attributes.getValue(i)) +
"\"");
+ if (options.isMerge() && path.equals("ivy-module")) {
+ for (int i = 0, n = attributes.getLength(); i < n; i += 1)
{
+ if (attributes.getQName(i).startsWith("xmlns:"))
continue;
+ write(" " + attributes.getQName(i) + "=\"" +
substitute(settings, attributes.getValue(i)) + "\"");
+ }
+ Map<String, String> namespaces =
options.getMergedDescriptor().getExtraAttributesNamespaces();
+ for (Map.Entry<String, String> namespace :
namespaces.entrySet()) {
Review Comment:
I haven't looked at the code completely but I believe this is run for every
node beneath ivy-module/info.Do we really want to write the namespace
attributes to every element here? Wouldn't it be easier to output them at the
root element of the module?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
