elharo commented on code in PR #337:
URL:
https://github.com/apache/maven-javadoc-plugin/pull/337#discussion_r1845446776
##########
src/main/java/org/apache/maven/plugins/javadoc/AbstractJavadocMojo.java:
##########
@@ -5955,7 +5955,7 @@ protected final JavadocOptions buildJavadocOptions()
throws IOException {
options.setBootclasspathArtifacts(toList(bootclasspathArtifacts));
options.setDocfilesSubdirsUsed(docfilessubdirs);
options.setDocletArtifacts(toList(docletArtifact, docletArtifacts));
- options.setExcludedDocfilesSubdirs(excludedocfilessubdir);
+ options.setExcludedDocfilesSubdirs(excludedocfilessubdir == null ?
null : excludedocfilessubdir.trim());
Review Comment:
This was hard to read. Consider renaming excludedDocfilesSubdirs
##########
src/test/java/org/apache/maven/plugins/javadoc/JavadocUtilTest.java:
##########
@@ -675,4 +680,23 @@ private void stopSilently(Server server) {
// ignored
}
}
+
+ public void testQuotedArgument() throws Exception {
+
+ String value = " org.apache.uima.analysis_component:\n
org.apache.uima.analysis_engine\n";
+
+ String arg = JavadocUtil.quotedArgument(value);
+
assertThat(arg).isEqualTo("'org.apache.uima.analysis_component:org.apache.uima.analysis_engine'");
+
+ value =
"org.apache.uima.analysis_component:org.apache.uima.analysis_engine";
Review Comment:
This should be two test methods so they don't interfere with eech other
##########
src/test/java/org/apache/maven/plugins/javadoc/JavadocUtilTest.java:
##########
@@ -675,4 +680,23 @@ private void stopSilently(Server server) {
// ignored
}
}
+
+ public void testQuotedArgument() throws Exception {
+
+ String value = " org.apache.uima.analysis_component:\n
org.apache.uima.analysis_engine\n";
+
+ String arg = JavadocUtil.quotedArgument(value);
+
assertThat(arg).isEqualTo("'org.apache.uima.analysis_component:org.apache.uima.analysis_engine'");
Review Comment:
assertEquals is simpler and more direct
##########
src/main/java/org/apache/maven/plugins/javadoc/JavadocUtil.java:
##########
@@ -221,14 +221,17 @@ protected static List<String> getExcludedPackages(
* @return quoted option-argument
*/
protected static String quotedArgument(String value) {
+ if (value == null) {
+ return null;
+ }
String arg = value;
+ List<String> list =
Arrays.stream(arg.split("\n")).map(String::trim).collect(Collectors.toList());
+ arg = String.join("", list);
Review Comment:
try to avoid reusing variables as it's bug prone
Also, this is combining lines with no space in between one line and the
next. Is that what you want?
--
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: issues-unsubscr...@maven.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
