feat: treat empty catalog as jbanghub/jbang-catalog aliases #1738
Conversation
|
hmm - test pass but have some other bugs :) |
|
fixed it so it now works from command line - lets see if all other tests passes. |
| if (names.length > 2) { | ||
| org = names[0]; | ||
| } | ||
| org = "jbangapp"; // TODO: should global default be controllable by config? |
There was a problem hiding this comment.
I think we should just make it a constant, just like Catalog.JBANG_CATALOG_JSON
|
Should we accept h2@ to be equal to h2@/ and h2@jbangapp ? |
|
ive updated so: h2@ is treated as blank catalog and thus maps to h2@jbangapp and sqlline@/sqlline works too should we error on h2@/ or be lenient ? (we do already accept |
| @@ -52,9 +52,10 @@ static CatalogRef get(String catalogName) { | |||
| if (catalog != null) { | |||
| catalogRef = catalog.catalogs.get(catalogName); | |||
| } | |||
| if (catalogRef == null && Util.isValidPath(catalogName)) { | |||
| if (catalogRef == null && (!catalogName.startsWith("/") || !catalogName.equals("")) | |||
There was a problem hiding this comment.
this is causing the test failure...we are for some reason resolving local paths first to support somealias@/var/path/locally/ondisk ?
@quintesse what usecase is that for ?
There was a problem hiding this comment.
#1363 is what introduced bar@/some/file/on/disk
There was a problem hiding this comment.
I've updated the code to make it so paths that exist on disk will be honorored.
But I'm not too happy about it.
its only needed for when you use absolute paths...relative paths would be different.
I could also let it run through first and then if fails lookup in jbanghub as a fallback - to avoid scanning multiple times.
There was a problem hiding this comment.
Yeah, I'm guessing it basically because we also allow foo@http://url/to/jbang-catalog.json so to be consistent we also allow file paths.
There was a problem hiding this comment.
could accept file:// instead of raw path ?
Because as it is now a local dir will actually override an implicit catalog.
Jbang myapp@jbanghub and you have jbanghub locally will override jbanghub catalog.
There was a problem hiding this comment.
could accept file:// instead of raw path ?
We could do that, yes. It does go somewhat against the fact that generally speaking a catalog reference is also a name@resourceref thing, so almost anything you should be able to pass to for example a //FILE tag was possible to use in a catalog reference. We'd now be limiting that. Now, I don't think anyone was actually using this, so we most likely won't be breaking anything, but just be aware that we will be making a breaking change.
There was a problem hiding this comment.
Ok, I just checked the URI spec, it seems that file: doesn't allow relative paths, so in that respect using file: is not a good replacement. And that does seem somewhat wrong. You'd be forced to always use absolute paths.
There was a problem hiding this comment.
hmm - I thought file:/./mydir was allowed but yeah does not seem to be "official" syntax.
URI.create("file:./pom.xml").getSchemeSpecficiPart() does let you fetch it though... but yeah not great.
…h2@ and sqlline@/sqlline
maxandersen
changed the title
prototype for #1733
lets you do:
jbang sqlline@/sqllineand
jbang h2@/basically treating "empty catalog" as a referring to "jbanghub"