From 0311125c0355e32a45c196d6e9fbfe1751c9def9 Mon Sep 17 00:00:00 2001 From: Tako Schotanus Date: Fri, 7 May 2021 17:49:36 +0200 Subject: [PATCH] Implemented all items from issue #854 (#858) --- itests/catalog.feature | 17 +++--- itests/run.feature | 2 +- readme.adoc | 6 +-- src/main/java/dev/jbang/catalog/Catalog.java | 25 +++++---- .../java/dev/jbang/catalog/CatalogRef.java | 9 ++-- .../java/dev/jbang/catalog/CatalogUtil.java | 2 +- src/main/java/dev/jbang/cli/Alias.java | 31 ++++------- src/main/java/dev/jbang/cli/Catalog.java | 52 +++++++++++++++---- src/main/java/dev/jbang/cli/Template.java | 21 ++------ .../java/dev/jbang/util/ConsoleOutput.java | 25 +++++++++ src/test/java/dev/jbang/cli/TestApp.java | 2 +- src/test/java/dev/jbang/cli/TestCatalog.java | 2 +- 12 files changed, 112 insertions(+), 82 deletions(-) create mode 100644 src/main/java/dev/jbang/util/ConsoleOutput.java diff --git a/itests/catalog.feature b/itests/catalog.feature index c5dfd10b40241..bafa34df1fc90 100644 --- a/itests/catalog.feature +++ b/itests/catalog.feature @@ -1,29 +1,30 @@ Feature: catalog testing Scenario: java catalog list - When command('jbang catalog add --global tc jbang-catalog.json') + When command('jbang catalog add --global --name averylongcatalogname jbang-catalog.json') And command('jbang catalog list') - Then match out contains "tc = JBang test scripts" + Then match out contains "averylongcatalogname" + Then match out contains "JBang test scripts" Scenario: add catalog and run catalog named reference - When command('jbang catalog add --global tc jbang-catalog.json') + When command('jbang catalog add --global --name tc jbang-catalog.json') Then command('jbang echo@tc tako') Then match out == "0:tako\n" Scenario: add catalog and remove - When command('jbang catalog add --global tc jbang-catalog.json') + When command('jbang catalog add --global --name tc jbang-catalog.json') Then command('jbang catalog remove tc') Then command('jbang echo@tc tako') Then match err contains "Unknown catalog 'tc'" Scenario: add catalog twice with same name - When command('jbang catalog add --global tc jbang-catalog.json') - Then command('jbang catalog add --global tc jbang-catalog.json') + When command('jbang catalog add --global --name tc jbang-catalog.json') + Then command('jbang catalog add --global --name tc jbang-catalog.json') Then match exit == 0 Scenario: add catalog twice with different name - When command('jbang catalog add --global tc jbang-catalog.json') - Then command('jbang catalog add --global ct jbang-catalog.json') + When command('jbang catalog add --global --name tc jbang-catalog.json') + Then command('jbang catalog add --global --name ct jbang-catalog.json') Then command('jbang echo@tc tako') And match exit == 0 Then command('jbang echo@ct tako') diff --git a/itests/run.feature b/itests/run.feature index 10a911caff472..a151e44d20842 100644 --- a/itests/run.feature +++ b/itests/run.feature @@ -32,7 +32,7 @@ Scenario: java run multiple files using alias Then match out contains "hello properties" Scenario: java run multiple files using remote alias - When command('jbang catalog add test https://raw.githubusercontent.com/jbangdev/jbang/master/itests/jbang-catalog.json') + When command('jbang catalog add --name test https://raw.githubusercontent.com/jbangdev/jbang/master/itests/jbang-catalog.json') Then command('jbang trust add https://raw.githubusercontent.com') Then command('jbang resource@test') Then match out contains "hello properties" diff --git a/readme.adoc b/readme.adoc index 5ad97a33a4614..ceb6895e780de 100644 --- a/readme.adoc +++ b/readme.adoc @@ -1286,11 +1286,11 @@ Catalogs are lists of Aliases as defined in the previous section, but while the within a catalog, the `catalog` command is for managing references to catalogs. This is mostly useful when dealing with remote catalogs. You can for example add a catalog like this: - jbang catalog add demo https://github.com/jbangdev/jbang-catalog/blob/master/jbang-catalog.json + jbang catalog add --name demo https://github.com/jbangdev/jbang-catalog/blob/master/jbang-catalog.json or simply by using the same "implicit" catalog system described in <>: - jbang catalog add demo jbangdev + jbang catalog add --name demo jbangdev The aliases in that catalog are now available by adding `@demo` to their names. For example: @@ -1303,7 +1303,7 @@ The aliases in that catalog are now available by adding `@demo` to their names. [jbang] Building jar... Hello World! -In fact it's possible to run the alias just by using `jbang run hello`, the `@demo` part is only necessary when trying to +In fact, it's possible to run the alias just by using `jbang run hello`, the `@demo` part is only necessary when trying to disambiguate between aliases with the same name from different catalogs. You can list the available catalogs by running: diff --git a/src/main/java/dev/jbang/catalog/Catalog.java b/src/main/java/dev/jbang/catalog/Catalog.java index d159a6b3fdaa7..7a1fd05d41e86 100644 --- a/src/main/java/dev/jbang/catalog/Catalog.java +++ b/src/main/java/dev/jbang/catalog/Catalog.java @@ -48,25 +48,23 @@ public class Catalog { public final String description; public transient ResourceRef catalogRef; - public Catalog(String baseRef, String description, ResourceRef catalogRef) { - this.baseRef = baseRef; - this.description = description; - this.catalogRef = catalogRef; - } - public Catalog(String baseRef, String description, ResourceRef catalogRef, Map catalogs, Map aliases, Map templates) { this.baseRef = baseRef; this.description = description; this.catalogRef = catalogRef; catalogs.forEach((key, c) -> this.catalogs.put(key, - new CatalogRef(c.catalogRef, c.description))); + new CatalogRef(c.catalogRef, c.description, this))); aliases.forEach((key, a) -> this.aliases.put(key, new Alias(a.scriptRef, a.description, a.arguments, a.properties, this))); templates.forEach((key, t) -> this.templates.put(key, new Template(t.fileRefs, t.description, this))); } + public static Catalog empty() { + return new Catalog(null, null, null, Collections.emptyMap(), Collections.emptyMap(), Collections.emptyMap()); + } + /** * Returns in all cases the absolute base reference that can be used to resolve * an Alias' script location. The result will either be a URL or an absolute @@ -136,7 +134,7 @@ void write() throws IOException { } /** - * Load a Catalog's aliases given the name of a previously registered Catalog + * Load a Catalog given the name of a previously registered Catalog * * @param catalogName The name of a registered * @return An Aliases object @@ -232,7 +230,7 @@ public static Catalog getMerged(boolean includeImplicits) { return false; }); - Catalog result = new Catalog(null, null, null); + Catalog result = Catalog.empty(); Collections.reverse(catalogs); for (Catalog catalog : catalogs) { if (!includeImplicits @@ -274,7 +272,7 @@ static Catalog findNearestCatalogWith(Path dir, Function accep } else if (accept.apply(getBuiltin())) { return getBuiltin(); } - return new Catalog(null, null, null); + return Catalog.empty(); } public static Catalog get(Path catalogPath) { @@ -316,7 +314,7 @@ public static String findImplicitName(Catalog catalog) { // This returns the built-in Catalog that can be found in the resources public static Catalog getBuiltin() { - Catalog catalog = new Catalog(null, null, null); + Catalog catalog = Catalog.empty(); if (Util.isFresh() || !catalogCache.containsKey(CACHE_BUILTIN)) { String res = "classpath:/" + JBANG_CATALOG_JSON; ResourceRef catRef = ResourceRef.forResource(res); @@ -338,7 +336,7 @@ public static void clearCache() { static Catalog read(Path catalogPath) { Util.verboseMsg(String.format("Reading catalog from %s", catalogPath)); - Catalog catalog = new Catalog(null, null, null); + Catalog catalog = Catalog.empty(); if (Files.isRegularFile(catalogPath)) { try (Reader in = Files.newBufferedReader(catalogPath)) { catalog = read(in); @@ -365,6 +363,7 @@ private static Catalog read(Reader in) { } for (String catName : catalog.catalogs.keySet()) { CatalogRef cat = catalog.catalogs.get(catName); + cat.catalog = catalog; check(cat.catalogRef != null, "Missing required attribute 'catalogs.catalogRef'"); } for (String aliasName : catalog.aliases.keySet()) { @@ -379,7 +378,7 @@ private static Catalog read(Reader in) { check(!tpl.fileRefs.isEmpty(), "Attribute 'templates.file-refs' has no elements"); } } else { - catalog = new Catalog(null, null, null); + catalog = Catalog.empty(); } return catalog; } diff --git a/src/main/java/dev/jbang/catalog/CatalogRef.java b/src/main/java/dev/jbang/catalog/CatalogRef.java index 527f8fb186fe3..72904087c5203 100644 --- a/src/main/java/dev/jbang/catalog/CatalogRef.java +++ b/src/main/java/dev/jbang/catalog/CatalogRef.java @@ -13,12 +13,13 @@ import dev.jbang.cli.ExitException; import dev.jbang.util.Util; -public class CatalogRef { +public class CatalogRef extends CatalogItem { @SerializedName(value = "catalog-ref", alternate = { "catalogRef" }) public final String catalogRef; public final String description; - CatalogRef(String catalogRef, String description) { + CatalogRef(String catalogRef, String description, Catalog catalog) { + super(catalog); this.catalogRef = catalogRef; this.description = description; } @@ -33,7 +34,7 @@ public class CatalogRef { public static CatalogRef createByRefOrImplicit(String catalogRef) { if (Util.isAbsoluteRef(catalogRef) || Files.isRegularFile(Paths.get(catalogRef))) { Catalog cat = Catalog.getByRef(catalogRef); - return new CatalogRef(catalogRef, cat.description); + return new CatalogRef(catalogRef, cat.description, cat); } else { Optional url = ImplicitCatalogRef.getImplicitCatalogUrl(catalogRef); if (!url.isPresent()) { @@ -41,7 +42,7 @@ public static CatalogRef createByRefOrImplicit(String catalogRef) { "Unable to locate catalog: " + catalogRef); } Catalog cat = Catalog.getByRef(url.get()); - return new CatalogRef(url.get(), cat.description); + return new CatalogRef(url.get(), cat.description, cat); } } diff --git a/src/main/java/dev/jbang/catalog/CatalogUtil.java b/src/main/java/dev/jbang/catalog/CatalogUtil.java index 1789ecf7caff2..2a8a45b91812a 100644 --- a/src/main/java/dev/jbang/catalog/CatalogUtil.java +++ b/src/main/java/dev/jbang/catalog/CatalogUtil.java @@ -231,7 +231,7 @@ public static CatalogRef addCatalogRef(Path catalogFile, String name, String cat } catch (InvalidPathException ex) { // Ignore } - CatalogRef ref = new CatalogRef(catalogRef, description); + CatalogRef ref = new CatalogRef(catalogRef, description, catalog); catalog.catalogs.put(name, ref); try { catalog.write(); diff --git a/src/main/java/dev/jbang/cli/Alias.java b/src/main/java/dev/jbang/cli/Alias.java index 46cd0b982f744..30438f2d33276 100644 --- a/src/main/java/dev/jbang/cli/Alias.java +++ b/src/main/java/dev/jbang/cli/Alias.java @@ -13,6 +13,7 @@ import dev.jbang.source.ResourceRef; import dev.jbang.source.RunContext; import dev.jbang.source.Source; +import dev.jbang.util.ConsoleOutput; import dev.jbang.util.Util; import picocli.CommandLine; @@ -144,7 +145,7 @@ static void printAliasesWithOrigin(PrintStream out, String catalogName, Catalog Collectors.groupingBy( e -> e.getValue().catalog.catalogRef)); groups.forEach((ref, entries) -> { - out.println(ref.getOriginalResource()); + out.println(ConsoleOutput.bold(ref.getOriginalResource())); entries.stream().map(Map.Entry::getKey).sorted().forEach(k -> printAlias(out, catalogName, catalog, k, 3)); }); } @@ -161,38 +162,24 @@ private static void printAlias(PrintStream out, String catalogName, Catalog cata } out.print(Util.repeat(" ", indent)); if (alias.description != null) { - out.println(yellow(fullName) + " = " + alias.description); + out.println(ConsoleOutput.yellow(fullName) + " = " + alias.description); if (Util.isVerbose()) - out.println(Util.repeat(" ", fullName.length() + indent) + faint(" (" + scriptRef + ")")); + out.println( + Util.repeat(" ", fullName.length() + indent) + ConsoleOutput.faint(" (" + scriptRef + ")")); } else { - out.println(yellow(fullName) + " = " + scriptRef); + out.println(ConsoleOutput.yellow(fullName) + " = " + scriptRef); } if (alias.arguments != null) { out.println( - Util.repeat(" ", fullName.length() + indent) + cyan(" Arguments: ") + Util.repeat(" ", fullName.length() + indent) + ConsoleOutput.cyan(" Arguments: ") + String.join(" ", alias.arguments)); } if (alias.properties != null) { out.println( - Util.repeat(" ", fullName.length() + indent) + magenta(" Properties: ") + alias.properties); + Util.repeat(" ", fullName.length() + indent) + ConsoleOutput.magenta(" Properties: ") + + alias.properties); } } - - private static String yellow(String text) { - return CommandLine.Help.Ansi.AUTO.new Text("@|fg(yellow) " + text + "|@").toString(); - } - - private static String cyan(String text) { - return CommandLine.Help.Ansi.AUTO.new Text("@|fg(cyan) " + text + "|@").toString(); - } - - private static String magenta(String text) { - return CommandLine.Help.Ansi.AUTO.new Text("@|fg(magenta) " + text + "|@").toString(); - } - - private static String faint(String text) { - return CommandLine.Help.Ansi.AUTO.new Text("@|faint " + text + "|@").toString(); - } } @CommandLine.Command(name = "remove", description = "Remove existing alias.") diff --git a/src/main/java/dev/jbang/cli/Catalog.java b/src/main/java/dev/jbang/cli/Catalog.java index e18bba66544fe..44e59efcc7840 100644 --- a/src/main/java/dev/jbang/cli/Catalog.java +++ b/src/main/java/dev/jbang/cli/Catalog.java @@ -4,10 +4,15 @@ import java.io.PrintWriter; import java.nio.file.Files; import java.nio.file.Path; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; import dev.jbang.Settings; import dev.jbang.catalog.CatalogRef; import dev.jbang.catalog.CatalogUtil; +import dev.jbang.source.ResourceRef; +import dev.jbang.util.ConsoleOutput; import dev.jbang.util.Util; import picocli.CommandLine; @@ -57,18 +62,21 @@ class CatalogAdd extends BaseCatalogCommand { "-d" }, description = "A description for the catalog") String description; - @CommandLine.Parameters(paramLabel = "name", index = "0", description = "A name for the catalog", arity = "1") + @CommandLine.Option(names = { "--name" }, description = "A name for the alias") String name; - @CommandLine.Parameters(paramLabel = "urlOrFile", index = "1", description = "A file or URL to a catalog file", arity = "1") + @CommandLine.Parameters(paramLabel = "urlOrFile", index = "0", description = "A file or URL to a catalog file", arity = "1") String urlOrFile; @Override public Integer doCall() { - if (!name.matches("^[a-zA-Z][-.\\w]*$")) { + if (name != null && !dev.jbang.catalog.Catalog.isValidName(name)) { throw new IllegalArgumentException( "Invalid catalog name, it should start with a letter followed by 0 or more letters, digits, underscores, hyphens or dots"); } + if (name == null) { + name = CatalogUtil.nameFromRef(urlOrFile); + } CatalogRef ref = CatalogRef.createByRefOrImplicit(urlOrFile); Path catFile = getCatalog(false); if (catFile != null) { @@ -76,7 +84,7 @@ public Integer doCall() { } else { catFile = CatalogUtil.addNearestCatalogRef(name, ref.catalogRef, ref.description); } - info(String.format("Catalog added to %s", catFile)); + info(String.format("Catalog '%s' added to '%s'", name, catFile)); return EXIT_OK; } } @@ -105,6 +113,9 @@ public Integer doCall() { @CommandLine.Command(name = "list", description = "Show currently defined catalogs.") class CatalogList extends BaseCatalogCommand { + @CommandLine.Option(names = { "--show-origin" }, description = "Show the origin of the catalog") + boolean showOrigin; + @CommandLine.Parameters(paramLabel = "name", index = "0", description = "The name of a catalog", arity = "0..1") String name; @@ -119,7 +130,11 @@ public Integer doCall() { } else { catalog = dev.jbang.catalog.Catalog.getMerged(true); } - printCatalogs(out, name, catalog); + if (showOrigin) { + printCatalogsWithOrigin(out, name, catalog); + } else { + printCatalogs(out, name, catalog); + } } else { dev.jbang.catalog.Catalog catalog = dev.jbang.catalog.Catalog.getByName(name); if (!catalog.aliases.isEmpty()) { @@ -147,21 +162,38 @@ static void printCatalogs(PrintStream out, String catalogName, dev.jbang.catalog .stream() .sorted() .forEach(nm -> { - printCatalog(out, catalogName, catalog, nm); + printCatalog(out, catalogName, catalog, nm, 0); }); } + static void printCatalogsWithOrigin(PrintStream out, String catalogName, dev.jbang.catalog.Catalog catalog) { + Map>> groups = catalog.catalogs + .entrySet() + .stream() + .collect( + Collectors.groupingBy( + e -> e.getValue().catalog.catalogRef)); + groups.forEach((ref, entries) -> { + out.println(ConsoleOutput.bold(ref.getOriginalResource())); + entries .stream() + .map(Map.Entry::getKey) + .sorted() + .forEach(k -> printCatalog(out, catalogName, catalog, k, 3)); + }); + } + private static void printCatalog(PrintStream out, String catalogName, dev.jbang.catalog.Catalog catalog, - String name) { + String name, int indent) { + out.print(Util.repeat(" ", indent)); String fullName = catalogName != null ? name + "@" + catalogName : name; CatalogRef ref = catalog.catalogs.get(name); if (ref.description != null) { - out.println(fullName + " = " + ref.description); - out.println(Util.repeat(" ", fullName.length()) + " (" + out.println(ConsoleOutput.yellow(fullName) + " = " + ref.description); + out.println(Util.repeat(" ", fullName.length() + indent) + " (" + ref.catalogRef + ")"); } else { - out.println(fullName + " = " + ref.catalogRef); + out.println(ConsoleOutput.yellow(fullName) + " = " + ref.catalogRef); } } } diff --git a/src/main/java/dev/jbang/cli/Template.java b/src/main/java/dev/jbang/cli/Template.java index 9f0b14191dd2a..26b4e1805654a 100644 --- a/src/main/java/dev/jbang/cli/Template.java +++ b/src/main/java/dev/jbang/cli/Template.java @@ -15,6 +15,7 @@ import dev.jbang.catalog.Catalog; import dev.jbang.catalog.CatalogUtil; import dev.jbang.source.ResourceRef; +import dev.jbang.util.ConsoleOutput; import dev.jbang.util.Util; import picocli.CommandLine; @@ -242,9 +243,9 @@ private static void printTemplate(PrintStream out, String catalogName, Catalog c String fullName = catalogName != null ? name + "@" + catalogName : name; out.print(Util.repeat(" ", indent)); if (template.description != null) { - out.println(yellow(fullName) + " = " + template.description); + out.println(ConsoleOutput.yellow(fullName) + " = " + template.description); } else { - out.println(yellow(fullName) + " = "); + out.println(ConsoleOutput.yellow(fullName) + " = "); } if (showFiles) { for (String dest : template.fileRefs.keySet()) { @@ -262,22 +263,6 @@ private static void printTemplate(PrintStream out, String catalogName, Catalog c } } } - - private static String yellow(String text) { - return CommandLine.Help.Ansi.AUTO.new Text("@|fg(yellow) " + text + "|@").toString(); - } - - private static String cyan(String text) { - return CommandLine.Help.Ansi.AUTO.new Text("@|fg(cyan) " + text + "|@").toString(); - } - - private static String magenta(String text) { - return CommandLine.Help.Ansi.AUTO.new Text("@|fg(magenta) " + text + "|@").toString(); - } - - private static String faint(String text) { - return CommandLine.Help.Ansi.AUTO.new Text("@|faint " + text + "|@").toString(); - } } @CommandLine.Command(name = "remove", description = "Remove existing template.") diff --git a/src/main/java/dev/jbang/util/ConsoleOutput.java b/src/main/java/dev/jbang/util/ConsoleOutput.java new file mode 100644 index 0000000000000..659afba123c85 --- /dev/null +++ b/src/main/java/dev/jbang/util/ConsoleOutput.java @@ -0,0 +1,25 @@ +package dev.jbang.util; + +import picocli.CommandLine; + +public class ConsoleOutput { + public static String yellow(String text) { + return CommandLine.Help.Ansi.AUTO.new Text("@|fg(yellow) " + text + "|@").toString(); + } + + public static String cyan(String text) { + return CommandLine.Help.Ansi.AUTO.new Text("@|fg(cyan) " + text + "|@").toString(); + } + + public static String magenta(String text) { + return CommandLine.Help.Ansi.AUTO.new Text("@|fg(magenta) " + text + "|@").toString(); + } + + public static String faint(String text) { + return CommandLine.Help.Ansi.AUTO.new Text("@|faint " + text + "|@").toString(); + } + + public static String bold(String text) { + return CommandLine.Help.Ansi.AUTO.new Text("@|bold " + text + "|@").toString(); + } +} diff --git a/src/test/java/dev/jbang/cli/TestApp.java b/src/test/java/dev/jbang/cli/TestApp.java index a0830d6bbd30a..56a71cf989b74 100644 --- a/src/test/java/dev/jbang/cli/TestApp.java +++ b/src/test/java/dev/jbang/cli/TestApp.java @@ -245,7 +245,7 @@ void testAppInstallAlias() throws IOException { void testAppInstallAliasFromRepo() throws IOException { String src = examplesTestFolder.resolve("helloworld.java").toString(); checkedRun(null, "alias", "add", "-g", "--name=apptest", src); - checkedRun(null, "catalog", "add", "-g", "testrepo", + checkedRun(null, "catalog", "add", "-g", "--name=testrepo", jbangTempDir.resolve("jbang-catalog.json").toString()); ExecutionResult result = checkedRun(null, "app", "install", "apptest@testrepo"); assertThat(result.err, containsString("Command installed: apptest")); diff --git a/src/test/java/dev/jbang/cli/TestCatalog.java b/src/test/java/dev/jbang/cli/TestCatalog.java index dcad4eb12b884..514e902cf6333 100644 --- a/src/test/java/dev/jbang/cli/TestCatalog.java +++ b/src/test/java/dev/jbang/cli/TestCatalog.java @@ -60,7 +60,7 @@ void testAddSucceeded() throws IOException { @Test void testAddInvalidName() throws IOException { Jbang jbang = new Jbang(); - new CommandLine(jbang).execute("catalog", "add", "invalid!", "dummy"); + new CommandLine(jbang).execute("catalog", "add", "--name=invalid!", "dummy"); } @Test