Remove extra indentation text nodes

gastaldi · gastaldi · commit 54eb5fc78df0 · 2024-03-21T14:55:30.000-03:00
When removing an element, extra text nodes may be still left over, this ensures these extra text nodes are removed
diff --git a/src/main/java/io/fabric8/maven/MavenJDOMWriter.java b/src/main/java/io/fabric8/maven/MavenJDOMWriter.java
@@ -12,6 +12,7 @@
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Iterator;
+import java.util.List;
 import java.util.ListIterator;
 import java.util.Properties;
 
@@ -224,6 +225,7 @@ protected void findAndReplaceSimpleLists(Counter counter, Element parent, Collec
                     elIt.next();
                     elIt.remove();
                 }
+                removeExtraIndents(element.getContent());
             }
         }
     }
@@ -321,6 +323,7 @@ protected void iterateContributor(Counter counter, Element parent, Collection<Co
                     elIt.next();
                     elIt.remove();
                 }
+                removeExtraIndents(element.getContent());
             }
         }
     }
@@ -360,9 +363,29 @@ protected void iterateDependency(Counter counter, Element parent, Collection<Dep
             }
             if (elIt != null) {
                 while (elIt.hasNext()) {
-                    elIt.next();
+                    Element toRemove = elIt.next();
                     elIt.remove();
                 }
+                removeExtraIndents(element.getContent());
+            }
+        }
+    }
+
+    /**
+     * When elements are removed from the JDOM tree, there may be Text nodes (used for indentation) that are left behind.
+     * This method removes these nodes.
+     *
+     * @param content The list of content to remove text nodes from
+     */
+    private void removeExtraIndents(List<Content> content) {
+        // We don't want to remove the first or last text node
+        for (int i = content.size() - 2; i > 0; i--) {
+            Content current = content.get(i);
+            if (current instanceof Text) {
+                content.remove(i);
+            } else {
+                // If the current element is not a text node, we can stop
+                break;
             }
         }
     }
@@ -405,6 +428,7 @@ protected void iterateDeveloper(Counter counter, Element parent, Collection<Deve
                     elIt.next();
                     elIt.remove();
                 }
+                removeExtraIndents(element.getContent());
             }
         }
     }
@@ -447,6 +471,7 @@ protected void iterateExclusion(Counter counter, Element parent, Collection<Excl
                     elIt.next();
                     elIt.remove();
                 }
+                removeExtraIndents(element.getContent());
             }
         }
     }
@@ -489,6 +514,7 @@ protected void iterateExtension(Counter counter, Element parent, Collection<Exte
                     elIt.next();
                     elIt.remove();
                 }
+                removeExtraIndents(element.getContent());
             }
         }
     }
@@ -531,6 +557,7 @@ protected void iterateLicense(Counter counter, Element parent, Collection<Licens
                     elIt.next();
                     elIt.remove();
                 }
+                removeExtraIndents(element.getContent());
             }
         }
     }
@@ -573,6 +600,7 @@ protected void iterateMailingList(Counter counter, Element parent, Collection<Ma
                     elIt.next();
                     elIt.remove();
                 }
+                removeExtraIndents(element.getContent());
             }
         }
     }
@@ -615,6 +643,7 @@ protected void iterateNotifier(Counter counter, Element parent, Collection<Notif
                     elIt.next();
                     elIt.remove();
                 }
+                removeExtraIndents(element.getContent());
             }
         }
     }
@@ -657,6 +686,7 @@ protected void iteratePlugin(Counter counter, Element parent, Collection<Plugin>
                     elIt.next();
                     elIt.remove();
                 }
+                removeExtraIndents(element.getContent());
             }
         }
     }
@@ -699,6 +729,7 @@ protected void iteratePluginExecution(Counter counter, Element parent, Collectio
                     elIt.next();
                     elIt.remove();
                 }
+                removeExtraIndents(element.getContent());
             }
         }
     }
@@ -741,6 +772,7 @@ protected void iterateProfile(Counter counter, Element parent, Collection<Profil
                     elIt.next();
                     elIt.remove();
                 }
+                removeExtraIndents(element.getContent());
             }
         }
     }
@@ -783,6 +815,7 @@ protected void iterateReportPlugin(Counter counter, Element parent, Collection<R
                     elIt.next();
                     elIt.remove();
                 }
+                removeExtraIndents(element.getContent());
             }
         }
     }
@@ -825,6 +858,7 @@ protected void iterateReportSet(Counter counter, Element parent, Collection<Repo
                     elIt.next();
                     elIt.remove();
                 }
+                removeExtraIndents(element.getContent());
             }
         }
     }
@@ -870,6 +904,7 @@ protected void iterateRepository(Counter counter, Element parent, Collection<Rep
                     elIt.next();
                     elIt.remove();
                 }
+                removeExtraIndents(element.getContent());
             }
         }
     }
@@ -915,6 +950,7 @@ protected void iterateResource(Counter counter, Element parent, Collection<Resou
                     elIt.next();
                     elIt.remove();
                 }
+                removeExtraIndents(element.getContent());
             }
         }
     }
diff --git a/src/test/approvals/io/fabric8/maven/MavenTest.should_not_add_extra_line_break.approved.txt b/src/test/approvals/io/fabric8/maven/MavenTest.should_not_add_extra_line_break.approved.txt
@@ -15,29 +15,11 @@
 
     <dependencyManagement>
         <dependencies>
-            <dependency>
-                <groupId>org.springdoc</groupId>
-                <artifactId>springdoc-openapi-ui</artifactId>
-                <version>${springdoc-openapi.version}</version>
-            </dependency>
             <dependency>
                 <groupId>org.springframework.boot</groupId>
                 <artifactId>spring-boot-dependencies</artifactId>
                 <version>${spring-boot.version}</version>
-                <scope>import</scope>
                 <type>pom</type>
-            </dependency>
-            <dependency>
-                <groupId>org.springframework.boot</groupId>
-                <artifactId>spring-boot-dependencies</artifactId>
-                <version>${spring-boot.version}</version>
-                <type>pom</type>
-                <scope>import</scope>
-            </dependency>
-            <dependency>
-                <groupId>org.springframework.boot</groupId>
-                <artifactId>spring-boot-dependencies</artifactId>
-                <version>${spring-boot.version}</version>
                 <scope>import</scope>
             </dependency>
         </dependencies>
diff --git a/src/test/java/io/fabric8/maven/MavenTest.java b/src/test/java/io/fabric8/maven/MavenTest.java
@@ -17,7 +17,6 @@
 import java.util.stream.IntStream;
 
 import org.apache.maven.model.Dependency;
-import org.apache.maven.model.DependencyManagement;
 import org.apache.maven.model.Model;
 import org.apache.maven.model.Scm;
 import org.approvaltests.Approvals;
@@ -338,25 +337,9 @@ void should_write_attributes() throws Exception {
     void should_not_add_extra_line_break() throws Exception {
         Path pom = Paths.get(getClass().getResource("extra-line-pom.xml").toURI());
         Model model = Maven.readModel(pom);
-        DependencyManagement dependencyManagement = model.getDependencyManagement();
-        Dependency dep = new Dependency();
-        dep.setGroupId("org.springframework.boot");
-        dep.setArtifactId("spring-boot-dependencies");
-        dep.setVersion("${spring-boot.version}");
-        dep.setScope("import");
-        dep.setType("pom");
-        dependencyManagement.addDependency(dep);
-
-        Dependency dep2 = new Dependency();
-        dep2.setGroupId("org.springframework.boot");
-        dep2.setArtifactId("spring-boot-dependencies");
-        dep2.setVersion("${spring-boot.version}");
-        dep2.setScope("import");
-        dependencyManagement.addDependency(dep2);
-
+        model.getDependencyManagement().getDependencies().remove(0);
         StringWriter sw = new StringWriter();
         Maven.writeModel(model, sw);
         Approvals.verify(sw.toString());
     }
-
 }

Original file line number	Diff line number	Diff line change
`@@ -12,6 +12,7 @@`
`12`	`12`	`import java.util.Arrays;`
`13`	`13`	`import java.util.Collection;`
`14`	`14`	`import java.util.Iterator;`
	`15`	`+import java.util.List;`
`15`	`16`	`import java.util.ListIterator;`
`16`	`17`	`import java.util.Properties;`
`17`	`18`
`@@ -224,6 +225,7 @@ protected void findAndReplaceSimpleLists(Counter counter, Element parent, Collec`
`224`	`225`	`elIt.next();`
`225`	`226`	`elIt.remove();`
`226`	`227`	`}`
	`228`	`+ removeExtraIndents(element.getContent());`
`227`	`229`	`}`
`228`	`230`	`}`
`229`	`231`	`}`
`@@ -321,6 +323,7 @@ protected void iterateContributor(Counter counter, Element parent, Collection<Co`
`321`	`323`	`elIt.next();`
`322`	`324`	`elIt.remove();`
`323`	`325`	`}`
	`326`	`+ removeExtraIndents(element.getContent());`
`324`	`327`	`}`
`325`	`328`	`}`
`326`	`329`	`}`
`@@ -360,9 +363,29 @@ protected void iterateDependency(Counter counter, Element parent, Collection<Dep`
`360`	`363`	`}`
`361`	`364`	`if (elIt != null) {`
`362`	`365`	`while (elIt.hasNext()) {`
`363`		`- elIt.next();`
	`366`	`+ Element toRemove = elIt.next();`
`364`	`367`	`elIt.remove();`
`365`	`368`	`}`
	`369`	`+ removeExtraIndents(element.getContent());`
	`370`	`+ }`
	`371`	`+ }`
	`372`	`+ }`
	`373`	`+`
	`374`	`+ /**`
	`375`	`+ * When elements are removed from the JDOM tree, there may be Text nodes (used for indentation) that are left behind.`
	`376`	`+ * This method removes these nodes.`
	`377`	`+ *`
	`378`	`+ * @param content The list of content to remove text nodes from`
	`379`	`+ */`
	`380`	`+ private void removeExtraIndents(List<Content> content) {`
	`381`	`+ // We don't want to remove the first or last text node`
	`382`	`+ for (int i = content.size() - 2; i > 0; i--) {`
	`383`	`+ Content current = content.get(i);`
	`384`	`+ if (current instanceof Text) {`
	`385`	`+ content.remove(i);`
	`386`	`+ } else {`
	`387`	`+ // If the current element is not a text node, we can stop`
	`388`	`+ break;`
`366`	`389`	`}`
`367`	`390`	`}`
`368`	`391`	`}`
`@@ -405,6 +428,7 @@ protected void iterateDeveloper(Counter counter, Element parent, Collection<Deve`
`405`	`428`	`elIt.next();`
`406`	`429`	`elIt.remove();`
`407`	`430`	`}`
	`431`	`+ removeExtraIndents(element.getContent());`
`408`	`432`	`}`
`409`	`433`	`}`
`410`	`434`	`}`
`@@ -447,6 +471,7 @@ protected void iterateExclusion(Counter counter, Element parent, Collection<Excl`
`447`	`471`	`elIt.next();`
`448`	`472`	`elIt.remove();`
`449`	`473`	`}`
	`474`	`+ removeExtraIndents(element.getContent());`
`450`	`475`	`}`
`451`	`476`	`}`
`452`	`477`	`}`
`@@ -489,6 +514,7 @@ protected void iterateExtension(Counter counter, Element parent, Collection<Exte`
`489`	`514`	`elIt.next();`
`490`	`515`	`elIt.remove();`
`491`	`516`	`}`
	`517`	`+ removeExtraIndents(element.getContent());`
`492`	`518`	`}`
`493`	`519`	`}`
`494`	`520`	`}`
`@@ -531,6 +557,7 @@ protected void iterateLicense(Counter counter, Element parent, Collection<Licens`
`531`	`557`	`elIt.next();`
`532`	`558`	`elIt.remove();`
`533`	`559`	`}`
	`560`	`+ removeExtraIndents(element.getContent());`
`534`	`561`	`}`
`535`	`562`	`}`
`536`	`563`	`}`
`@@ -573,6 +600,7 @@ protected void iterateMailingList(Counter counter, Element parent, Collection<Ma`
`573`	`600`	`elIt.next();`
`574`	`601`	`elIt.remove();`
`575`	`602`	`}`
	`603`	`+ removeExtraIndents(element.getContent());`
`576`	`604`	`}`
`577`	`605`	`}`
`578`	`606`	`}`
`@@ -615,6 +643,7 @@ protected void iterateNotifier(Counter counter, Element parent, Collection<Notif`
`615`	`643`	`elIt.next();`
`616`	`644`	`elIt.remove();`
`617`	`645`	`}`
	`646`	`+ removeExtraIndents(element.getContent());`
`618`	`647`	`}`
`619`	`648`	`}`
`620`	`649`	`}`
`@@ -657,6 +686,7 @@ protected void iteratePlugin(Counter counter, Element parent, Collection<Plugin>`
`657`	`686`	`elIt.next();`
`658`	`687`	`elIt.remove();`
`659`	`688`	`}`
	`689`	`+ removeExtraIndents(element.getContent());`
`660`	`690`	`}`
`661`	`691`	`}`
`662`	`692`	`}`
`@@ -699,6 +729,7 @@ protected void iteratePluginExecution(Counter counter, Element parent, Collectio`
`699`	`729`	`elIt.next();`
`700`	`730`	`elIt.remove();`
`701`	`731`	`}`
	`732`	`+ removeExtraIndents(element.getContent());`
`702`	`733`	`}`
`703`	`734`	`}`
`704`	`735`	`}`
`@@ -741,6 +772,7 @@ protected void iterateProfile(Counter counter, Element parent, Collection<Profil`
`741`	`772`	`elIt.next();`
`742`	`773`	`elIt.remove();`
`743`	`774`	`}`
	`775`	`+ removeExtraIndents(element.getContent());`
`744`	`776`	`}`
`745`	`777`	`}`
`746`	`778`	`}`
`@@ -783,6 +815,7 @@ protected void iterateReportPlugin(Counter counter, Element parent, Collection<R`
`783`	`815`	`elIt.next();`
`784`	`816`	`elIt.remove();`
`785`	`817`	`}`
	`818`	`+ removeExtraIndents(element.getContent());`
`786`	`819`	`}`
`787`	`820`	`}`
`788`	`821`	`}`
`@@ -825,6 +858,7 @@ protected void iterateReportSet(Counter counter, Element parent, Collection<Repo`
`825`	`858`	`elIt.next();`
`826`	`859`	`elIt.remove();`
`827`	`860`	`}`
	`861`	`+ removeExtraIndents(element.getContent());`
`828`	`862`	`}`
`829`	`863`	`}`
`830`	`864`	`}`
`@@ -870,6 +904,7 @@ protected void iterateRepository(Counter counter, Element parent, Collection<Rep`
`870`	`904`	`elIt.next();`
`871`	`905`	`elIt.remove();`
`872`	`906`	`}`
	`907`	`+ removeExtraIndents(element.getContent());`
`873`	`908`	`}`
`874`	`909`	`}`
`875`	`910`	`}`
`@@ -915,6 +950,7 @@ protected void iterateResource(Counter counter, Element parent, Collection<Resou`
`915`	`950`	`elIt.next();`
`916`	`951`	`elIt.remove();`
`917`	`952`	`}`
	`953`	`+ removeExtraIndents(element.getContent());`
`918`	`954`	`}`
`919`	`955`	`}`
`920`	`956`	`}`