Allow embedded comments in newline-comma lists

src/main/java/me/itzg/helpers/McImageHelper.java

@@ -107,12 +107,6 @@ public class McImageHelper {
     //language=RegExp
     public static final String SPLIT_COMMA_NL = "\\n|\\s*,\\s*";
     public static final String SPLIT_SYNOPSIS_COMMA_NL = ",|<nl>";
-    /**
-     * Also works for newline delimited values.
-     */
-    //language=RegExp
-    public static final String SPLIT_COMMA_WS = "\\s+|\\s*,\\s*";
-    public static final String SPLIT_SYNOPSIS_COMMA_WS = ",|<ws>";
     //language=RegExp
     public static final String VERSION_REGEX = "\\d+(\\.\\d+)+";
 
@@ -243,4 +237,5 @@ public Integer call() throws Exception {
             return ExitCode.OK;
         }
     }
+
 }

src/main/java/me/itzg/helpers/curseforge/CurseForgeFilesCommand.java

@@ -2,6 +2,7 @@
 
 import static me.itzg.helpers.curseforge.CurseForgeApiClient.*;
 import static me.itzg.helpers.curseforge.ModFileRefResolver.idsFrom;
+import static me.itzg.helpers.singles.NormalizeOptions.normalizeOptionList;
 
 import java.nio.file.Files;
 import java.nio.file.Path;
@@ -13,6 +14,7 @@
 import java.util.concurrent.Callable;
 import java.util.stream.Collectors;
 import lombok.extern.slf4j.Slf4j;
+import me.itzg.helpers.McImageHelper;
 import me.itzg.helpers.cache.ApiCaching;
 import me.itzg.helpers.cache.ApiCachingDisabled;
 import me.itzg.helpers.cache.ApiCachingImpl;
@@ -96,14 +98,19 @@ public void setSlugCategory(String defaultCategory) {
     @ArgGroup(exclusive = false)
     SharedFetchArgs sharedFetchArgs = new SharedFetchArgs();
 
-    @Parameters(split = ",", paramLabel = "REF",
+    @Parameters(paramLabel = "REF",
+        split = McImageHelper.SPLIT_COMMA_NL, splitSynopsisLabel = McImageHelper.SPLIT_SYNOPSIS_COMMA_NL,
         description = "Can be <project ID>|<slug>':'<file ID>,"
             + " <project ID>|<slug>'@'<filename matcher>,"
             + " <project ID>|<slug>,"
             + " project page URL, file page URL,"
             + " '@'<filename with ref per line>"
-            + "%nIf not specified, any previous mod/plugin files are removed")
-    List<String> modFileRefs;
+            + "%nIf not specified, any previous mod/plugin files are removed."
+            + "%Embedded comments are allowed")
+    public void setModFileRefs(List<String> modFileRefs) {
+        this.modFileRefs = normalizeOptionList(modFileRefs);
+    }
+    private List<String> modFileRefs = Collections.emptyList();
 
     @Override
     public Integer call() throws Exception {
@@ -114,7 +121,6 @@ public Integer call() throws Exception {
         final Map<ModFileIds, FileEntry> previousFiles = buildPreviousFilesFromManifest(oldManifest);
 
         final CurseForgeFilesManifest newManifest;
-
         if (modFileRefs != null && !modFileRefs.isEmpty()) {
             try (
                 final ApiCaching apiCaching = disableApiCaching ? new ApiCachingDisabled()
@@ -128,37 +134,23 @@ public Integer call() throws Exception {
             ) {
                 newManifest =
                     apiClient.loadCategoryInfo(Arrays.asList(CATEGORY_MC_MODS, CATEGORY_BUKKIT_PLUGINS))
-                    .flatMap(categoryInfo ->
+                        .flatMap(categoryInfo ->
 
-                        processModFileRefs(categoryInfo, previousFiles, apiClient)
-                            .map(entries -> CurseForgeFilesManifest.builder()
-                                .entries(entries)
-                                .build()))
+                            processModFileRefs(categoryInfo, previousFiles, apiClient)
+                                .map(entries -> CurseForgeFilesManifest.builder()
+                                    .entries(entries)
+                                    .build()))
                         .doOnError(InvalidApiKeyException.class,
                             throwable -> ApiKeyHelper.logKeyIssues(log, apiKey))
                         .block();
             }
         }
         else {
-            // nothing to install or requesting full cleanup
             newManifest = null;
         }
 
-        if (oldManifest != null) {
-            Manifests.cleanup(outputDir,
-                mapFilePathsFromEntries(oldManifest),
-                mapFilePathsFromEntries(newManifest),
-                s -> log.info("Removing old file {}", s)
-            );
-        }
-        if (newManifest != null && newManifest.getEntries() != null
-            && !newManifest.getEntries().isEmpty()
-        ) {
-            Manifests.save(outputDir, CurseForgeFilesManifest.ID, newManifest);
-        }
-        else {
-            Manifests.remove(outputDir, CurseForgeFilesManifest.ID);
-        }
+        Manifests.cleanup(outputDir, oldManifest, newManifest, log);
+        Manifests.apply(outputDir, CurseForgeFilesManifest.ID, newManifest);
 
         return ExitCode.OK;
     }
@@ -167,7 +159,7 @@ public Integer call() throws Exception {
     private Mono<List<FileEntry>> processModFileRefs(CategoryInfo categoryInfo,
         Map<ModFileIds, FileEntry> previousFiles, CurseForgeApiClient apiClient
     ) {
-        if (modFileRefs == null || modFileRefs.isEmpty()) {
+        if (modFileRefs.isEmpty()) {
             return Mono.just(Collections.emptyList());
         }
 
@@ -289,11 +281,4 @@ private Map<ModFileIds, FileEntry> buildPreviousFilesFromManifest(CurseForgeFile
             : Collections.emptyMap();
     }
 
-    private static List<String> mapFilePathsFromEntries(CurseForgeFilesManifest oldManifest) {
-        return
-            oldManifest != null ?
-            oldManifest.entries.stream().map(FileEntry::getFilePath).collect(Collectors.toList())
-            : null;
-    }
-
 }

src/main/java/me/itzg/helpers/curseforge/CurseForgeFilesManifest.java

@@ -1,6 +1,9 @@
 package me.itzg.helpers.curseforge;
 
+import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
+import java.util.stream.Collectors;
 import lombok.Builder;
 import lombok.Data;
 import lombok.Getter;
@@ -24,4 +27,12 @@ public static class FileEntry {
     }
 
     List<FileEntry> entries;
+
+    @Override
+    public Collection<String> getFiles() {
+        return entries != null ? entries.stream()
+            .map(FileEntry::getFilePath)
+            .collect(Collectors.toList())
+            : Collections.emptyList();
+    }
 }

src/main/java/me/itzg/helpers/curseforge/InstallCurseForgeCommand.java

@@ -1,6 +1,7 @@
 package me.itzg.helpers.curseforge;
 
 import static me.itzg.helpers.http.Fetch.fetch;
+import static me.itzg.helpers.singles.NormalizeOptions.normalizeOptionSet;
 
 import java.io.BufferedWriter;
 import java.io.IOException;
@@ -103,19 +104,24 @@ static class ExcludeIncludeArgs {
         PathOrUri excludeIncludeFile;
 
         static class Listed {
-            @Option(names = "--exclude-mods", paramLabel = "PROJECT_ID|SLUG",
-                split = McImageHelper.SPLIT_COMMA_WS, splitSynopsisLabel = McImageHelper.SPLIT_SYNOPSIS_COMMA_WS,
+            @Option(names = {"--exclude-mods", "--excludes"}, paramLabel = "PROJECT_ID|SLUG",
+                split = McImageHelper.SPLIT_COMMA_NL, splitSynopsisLabel = McImageHelper.SPLIT_SYNOPSIS_COMMA_NL,
                 description = "For mods that need to be excluded from server deployments, such as those that don't label as client"
             )
-            Set<String> excludedMods;
+            public void setExcludedMods(Set<String> excludedMods) {
+                this.excludedMods = normalizeOptionSet(excludedMods);
+            }
+            private Set<String> excludedMods;
 
-            @Option(names = "--force-include-mods", paramLabel = "PROJECT_ID|SLUG",
-                split = McImageHelper.SPLIT_COMMA_WS, splitSynopsisLabel = McImageHelper.SPLIT_SYNOPSIS_COMMA_WS,
+            @Option(names = {"--force-include-mods", "--force-includes"}, paramLabel = "PROJECT_ID|SLUG",
+                split = McImageHelper.SPLIT_COMMA_NL, splitSynopsisLabel = McImageHelper.SPLIT_SYNOPSIS_COMMA_NL,
                 description = "Some mods incorrectly declare client-only support, but still need to be included in a server deploy."
                     + "%nThis can also be used to selectively override exclusions."
             )
+            public void setForceIncludeMods(Set<String> forceIncludeMods) {
+                this.forceIncludeMods = normalizeOptionSet(forceIncludeMods);
+            }
             Set<String> forceIncludeMods;
-
         }
     }
 

src/main/java/me/itzg/helpers/files/Manifests.java

@@ -5,6 +5,7 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Objects;
@@ -59,11 +60,12 @@ public static void cleanup(Path baseDir, BaseManifest oldManifest,
      * @param oldManifest can be null
      * @param removeListener passed the path of a file being removed. Useful for debug logging as removed.
      */
-    public static void cleanup(Path baseDir, BaseManifest oldManifest, BaseManifest newManifest,
+    public static void cleanup(Path baseDir, @Nullable BaseManifest oldManifest, @Nullable BaseManifest newManifest,
         Consumer<String> removeListener
     ) throws IOException {
         if (oldManifest != null && oldManifest.getFiles() != null) {
-            cleanup(baseDir, oldManifest.getFiles(), newManifest.getFiles(), removeListener);
+            cleanup(baseDir, oldManifest.getFiles(), newManifest != null ? newManifest.getFiles() : Collections.emptyList(),
+                removeListener);
         }
     }
 
@@ -158,12 +160,30 @@ public static Path buildManifestPath(Path outputDir, String id) {
         ));
     }
 
-    public static <M extends BaseManifest> Path save(Path outputDir, String id, M manifest) {
+    /**
+     * Given a resulting ("new") manifest, it will save it when the file listing is non-empty or remove
+     * an existing manifest file if the listing is empty.
+     * @param outputDir directory to place the manifest file
+     * @param id manifest identifier, such as "fabric"
+     * @param manifest the resulting/new manifest
+     */
+    public static <M extends BaseManifest> void apply(Path outputDir, String id, M manifest) {
+        if (manifest == null) {
+            remove(outputDir, id);
+        }
+        else if (manifest.getFiles() == null || manifest.getFiles().isEmpty()) {
+            remove(outputDir, id);
+        }
+        else {
+            save(outputDir, id, manifest);
+        }
+    }
+
+    public static <M extends BaseManifest> void save(Path outputDir, String id, M manifest) {
         final Path manifestPath = buildManifestPath(outputDir, id);
 
         try {
             ObjectMappers.defaultMapper().writeValue(manifestPath.toFile(), manifest);
-            return manifestPath;
         } catch (IOException e) {
             throw new ManifestException("Failed to save manifest", e);
         }

src/main/java/me/itzg/helpers/get/GetCommand.java

@@ -2,6 +2,7 @@
 
 import static me.itzg.helpers.McImageHelper.OPTION_SPLIT_COMMAS;
 import static me.itzg.helpers.http.Fetch.fetch;
+import static me.itzg.helpers.singles.NormalizeOptions.normalizeStream;
 
 import java.io.IOException;
 import java.io.PrintWriter;
@@ -271,9 +272,7 @@ private void readUris() throws IOException {
 
         final LenientUriConverter uriConverter = new LenientUriConverter();
 
-        Files.readAllLines(urisFile).stream()
-            .filter(line -> !line.startsWith("#"))
-            .filter(line -> !line.trim().isEmpty())
+        normalizeStream(Files.readAllLines(urisFile).stream())
             .map(line -> {
                 try {
                     return uriConverter.convert(line);

src/main/java/me/itzg/helpers/modrinth/InstallModrinthModpackCommand.java

@@ -1,7 +1,10 @@
 package me.itzg.helpers.modrinth;
 
+import static me.itzg.helpers.singles.NormalizeOptions.normalizeOptionList;
+
 import java.io.IOException;
 import java.nio.file.Path;
+import java.util.Collections;
 import java.util.List;
 import java.util.concurrent.Callable;
 import lombok.AccessLevel;
@@ -90,23 +93,35 @@ public class InstallModrinthModpackCommand implements Callable<Integer> {
     @Option(names = "--exclude-files",
         split = McImageHelper.SPLIT_COMMA_NL, splitSynopsisLabel = McImageHelper.SPLIT_SYNOPSIS_COMMA_NL,
         description = "Files to exclude, such as improperly declared client mods. It will match any part of the file's name/path."
+            + "%nEmbedded comments are allowed."
     )
-    List<String> excludeFiles;
+    public void setExcludeFiles(List<String> excludeFiles) {
+        this.excludeFiles = normalizeOptionList(excludeFiles);
+    }
+    private List<String> excludeFiles = Collections.emptyList();
 
     @Option(names = "--force-include-files",
         split = McImageHelper.SPLIT_COMMA_NL, splitSynopsisLabel = McImageHelper.SPLIT_SYNOPSIS_COMMA_NL,
         description = "Files to force include that were marked as non-server mods. It will match any part of the file's name/path."
+            + "%nEmbedded comments are allowed."
     )
-    List<String> forceIncludeFiles;
+    public void setForceIncludeFiles(List<String> forceIncludeFiles) {
+        this.forceIncludeFiles = normalizeOptionList(forceIncludeFiles);
+    }
+    private List<String> forceIncludeFiles = Collections.emptyList();
 
     @Option(names = "--overrides-exclusions",
         split = "\n|,", splitSynopsisLabel = "NL or ,",
         description = "Excludes files from the overrides that match these ant-style patterns\n"
             + "*  : matches any non-slash characters\n"
             + "** : matches any characters\n"
             + "?  : matches one character"
+            + "%nEmbedded comments are allowed."
     )
-    List<String> overridesExclusions;
+    public void setOverridesExclusions(List<String> overridesExclusions) {
+        this.overridesExclusions = normalizeOptionList(overridesExclusions);
+    }
+    private List<String> overridesExclusions = Collections.emptyList();
 
     @Option(names = "--default-exclude-includes", paramLabel = "FILE|URI",
         description = "A JSON file that contains global and per modpack exclude/include declarations. "
@@ -159,7 +174,7 @@ public Integer call() throws IOException {
                                 sharedFetch
                             )
                         )
-                        .setOverridesExclusions(overridesExclusions)
+                        .setOverridesExclusions(normalizeOptionList(overridesExclusions))
                         .setMaxConcurrentDownloads(maxConcurrentDownloads)
                         .processModpack(sharedFetch)
                         .flatMap(installation -> {

src/main/java/me/itzg/helpers/modrinth/ModrinthCommand.java

@@ -3,6 +3,7 @@
 import static me.itzg.helpers.McImageHelper.SPLIT_COMMA_NL;
 import static me.itzg.helpers.McImageHelper.SPLIT_SYNOPSIS_COMMA_NL;
 import static me.itzg.helpers.http.Fetch.fetch;
+import static me.itzg.helpers.singles.NormalizeOptions.normalizeOptionList;
 
 import com.fasterxml.jackson.databind.ObjectMapper;
 import java.io.IOException;
@@ -54,12 +55,16 @@ public class ModrinthCommand implements Callable<Integer> {
             + "%nExamples: fabric-api, fabric:fabric-api, fabric:fabric-api:0.76.1+1.19.2,"
             + " datapack:terralith, @/path/to/modrinth-mods.txt"
             + "%nValid release types: release, beta, alpha"
-            + "%nValid loaders: fabric, forge, paper, datapack, etc.",
+            + "%nValid loaders: fabric, forge, paper, datapack, etc."
+            + "%nEmbedded comments are allowed.",
         split = SPLIT_COMMA_NL,
         splitSynopsisLabel = SPLIT_SYNOPSIS_COMMA_NL,
         paramLabel = "[loader:]id|slug[:version]"
     )
-    List<String> projects;
+    public void setProjects(List<String> projects) {
+        this.projects = normalizeOptionList(projects);
+    }
+    private List<String> projects = Collections.emptyList();
 
     @Option(names = "--game-version", description = "Applicable Minecraft version", required = true)
     String gameVersion;
@@ -111,14 +116,7 @@ public enum DownloadDependencies {
     public Integer call() throws Exception {
         Files.createDirectories(outputDirectory);
 
-        // Unlike @Parameters, the argument of @Option could be a list with a single empty string
-        // when --projects="". So, trim away any blanks.
-        final List<String> trimmedProjects = projects == null ? Collections.emptyList() :
-            projects.stream()
-                .filter(s -> !s.trim().isEmpty())
-                .collect(Collectors.toList());
-
-        final List<Path> outputFiles = processProjects(trimmedProjects);
+        final List<Path> outputFiles = processProjects(projects);
 
         final ModrinthManifest newManifest = ModrinthManifest.builder()
             .files(Manifests.relativizeAll(outputDirectory, outputFiles))
@@ -127,13 +125,7 @@ public Integer call() throws Exception {
 
         final ModrinthManifest prevManifest = loadManifest();
         Manifests.cleanup(outputDirectory, prevManifest, newManifest, log);
-
-        if (!outputFiles.isEmpty()) {
-            Manifests.save(outputDirectory, ModrinthManifest.ID, newManifest);
-        }
-        else {
-            Manifests.remove(outputDirectory, ModrinthManifest.ID);
-        }
+        Manifests.apply(outputDirectory, ModrinthManifest.ID, newManifest);
 
         return ExitCode.OK;
     }
@@ -148,7 +140,6 @@ private List<Path> processProjects(List<String> projects) {
             return
                 modrinthApiClient.bulkGetProjects(
                     expandProjectListings(projects).stream()
-                        .filter(s -> !s.trim().isEmpty())
                         .map(ProjectRef::parse)
                 )
                 .defaultIfEmpty(Collections.emptyList())