Catalog command improvements

[quintesse]

fix: properly listing the catalog names (fixes #1678) feat: added --force options to catalog add commands (related to the above) feat: added import feature to catalog refs

The last feature is something that we discussed (not sure if there's an issue for it, couldn't find it): an option that imports the items defined in a catalog and makes them available to the user without having to specify the full name, eg. using @somecatalog.

Right now this only works for local catalogs, so if you add a catalog reference with jbang catalog add --import maxandersen it will make all aliases, templates and catalog refs in that catalog available to the user without having to specify @maxandersen.

NB: The reason for the local catalog limitation (for now?) is to prevent a cascade. Otherwise you could import a catalog that references a bunch of other catalogs and imports them all and suddenly you have a gazillion aliases and templates available. But perhaps this is a restriction that could be relaxed in the future.

Btw, with this feature you could almost implement something like the jbanghub with very little changes. If you import the Hub's catalog and if it would have a list of catalog references, like for example "sqlline", then we would have a simple way to manage "named catalogs". You could do jbang foo@sqlline and it would work.

[maxandersen]

functionallity wise looks good. 2 questions:

1. not sure I follow what "only local" means ...got an example of where using remote vs local?

2. the imported catalog still "pollutes" my default catalog names available. I didn't expect that. i.e. I put this in a jbang-catalog:

"catalogs": {
    "jbanghubby": {
      "catalog-ref": "https://github.com/jbanghub/jbang-catalog/blob/HEAD/jbang-catalog.json",
      "import": true
    }
  },

now jbang mima works - great, but jbang mima@jbanghubby works too... meaning that added catalogs wether implicit or explicit can "rewire" what catalogs I have on my system...that is not good from a security and predictability wise.

i.e. lets say someone adds this to their public catalog:

"catalogs": {
    "jbangdev": {
      "catalog-ref": "https://github.com/jbanghub/jbang-catalog/blob/HEAD/jbang-catalog.json",
      "import": true
    }
  },

now jbang env@jbangdev is failing but imagine i add a env command that just did some random operation.

This side-effect is not caused by this change but. a problem we have already today and I honestly think we need to fix/break that behavior - at least by default.

[quintesse]

Sure:

1. it only works for catalogs found on your file system, it doesn't work for remote catalogs. Or in other words you must at some point have used an --import flag while adding a catalog manually. It's not that you added a catalog and then they decide to add an "import: true" and suddenly you get a whole bunch of extra stuff. (On the other hand, if you did use --import on an untrusted catalog they could decide to add a whole bunch of stuff you don't want).
2. "now jbang mima works - great, but jbang mima@jbanghubby works too..", sure, the catalog still has an explicit name, nothing you can do about that. The --import just means that you can now also use all the items in the catalog without the fully qualified name. (Btw, I'd say that the import is the major "polluter", the catalog name is just a single item which is probably pretty unique-ish)

The overriding... well, yes, that could probably happen. I can think of 2 solutions: make jbangedv/jbanghub special (not my favorite) or prioritize "toplevel" catalogs over "sub" catalogs. That way any catalogs that you explicitly added or used yourself will always have preference.

[quintesse]

I've updated the code to prioritize local catalogs over imported catalog refs. Still need to fix an integration test though.

[quintesse]

@maxandersen this is now in a testable state again. It won't show aliases/templates recursively anymore. And you can now also do things like jbang catalog add microsoft@jbanghub (you could do jbang catalog add --import jbanghub but then you'd get all catalog refs and perhaps you're only interested in the MS one).

While creating this PR I started wondering if we should actually take into account the catalogs that get added to implicit-catalog.json while listing catalog items. Right now whenever you do anything like jbang script@somecatalog and somecatalog is some implicit catalog reference, we resolve it to an actual URL and store the result in that file. But what happens is that immediately all the aliases and templates (and catalog refs) in that catalog start showing up in the alias list and template list commands. And I've started wondering if that's actually a good idea. It's at least surprising, I think.

Edit: reworded

Edit2: I actually added a commit that does this to try it out and personally I like it. Because we're often testing all kinds of stuff my implicit-catalog was filled with loads of things that at some point in the past I had tried out. Which made my jbang alias list very long and mostly filled with things that didn't actually interest me. With this commit it will only show stuff that I explicitly added to the catalog. We'd only need to add the jbanghub by default now and we'd have a clean list filled with (hopefully) interesting items.

[maxandersen]

Totally agree we shouldn't list from implicit catalogs by default.

[maxandersen]

not sure if this is expecte:

jbang catalog add jbanghub
jbang catalog add --import --force jbanghub

result in duplicated catalog list - one from the first one then the other set.

jbang catalog remove jbanghub remove them all.

[maxandersen]

it might not list content recursively but if I do:

jbang cache clear --all

then do:

jbang catalog add --import jbanghub

I have ~14 jbang-catalog.json files in the cache.

I'm assuming that is also cause of why jbang catalog list is slow.

Is that expected?

I was hoping it would not incur a cost before users actually add or reference an alias in that catalog?

[quintesse]

not sure if this is expecte: ... result in duplicated catalog list - one from the first one then the other set.

jbang catalog remove jbanghub remove them all.

Could you try and see if only running jbang catalog add --import --force jbanghub gives you the same result? Because then it might be expected. Because the add will import the jbanghub "alias", import or not. The import just has the added effect of making its contents available locally as well. That would also explain why "both" disappear after a remove.

I have ~14 jbang-catalog.json files in the cache.

I'm assuming that is also cause of why jbang catalog list is slow.

Is that expected?

I'll look into it because we definitely don't want that, no.

does that need to be a tri-state Boolean?

It doesn't need to be, no. I simply tend to like it because it gives us a way to check if the value was actually added by the user or not. But if you prefer I can change it to a simple boolean.

[maxandersen]

Could you try and see if only running jbang catalog add --import --force jbanghub gives you the same result?

it does ...which seems wrong imo. the duplication is "noise" compared to what --import is supposed to do (make the aliases available)

[quintesse]

Could you try and see if only running jbang catalog add --import --force jbanghub gives you the same result?

it does ...which seems wrong imo. the duplication is "noise" compared to what --import is supposed to do (make the aliases available)

I understand, we could try to do this but I think there are some big problems.

So first of all any catalogs that you add (imported or not) result in an named entry to be added to the catalogs section of the jbang-catalog.json, that's just how it's implemented, not much we can do about that now (nor do I think that other alternatives would improve the situation).

If you don't specify a name yourself we detect one from the name/URL you specify.

So running jbang catalog add --import jbanghub would result in a "jbanghub" entry being added but NOT being shown in the list of catalogs.

Problem: so how can the user delete it again? They won't know that there's this hidden catalog named "jbanghub" and that they should run jbang catalog remove jbanghub.

Another problem is that both jbang catalog add jbanghub and jbang catalog add --import jbanghub would result in a "jbanghub" entry being added. Which means that one would override the other. Which is not good, because what if I want to have both? I would be obliged to manually given one a different name from the other.

I could imagine simply generating some unique name for the imported catalog because we're not showing it anyway, but we're still left with the first problem of not being able to delete the catalog.

Edit: the only solution I can see is:

1. generate a unique name for imported catalogs (perhaps simply add "_import" to their name)
2. don't show imported catalogs by default
3. add an option --show-imported to catalog list

That way you can actually see the names of imported catalogs and therefore you can delete them. But it does make it harder for users to discover. They'd probably go like "hey, I just added a catalog but it doesn't show up in the list! So how do I delete it??" and we'd have to go "RTFM!" :-)

Edit2: well, my suggestion would actually be that the best solution is the current one :-) It's simpler and it just works.

Edit3: a clarification for why I don't think the current situation is that bad. I think the most often used list commands will be for alias list and template list which don't have this "noise" problem, so the issue is only for catalog list, which I think is way less used. Still, perhaps we could improve upon it? Perhaps by marking imported catalogs with "(imported)" in the list or something?

Edi4: Oh, I see now that all imported items are being duplicated, if that was what you were getting at, that would indeed be wrong. Looking into it.

[quintesse]

@maxandersen this is now in a testable state again. Let me know what you think.

[maxandersen]

jbang catalog remove karatelabs seem to update the cached version so it stays removed until clear cache.

[maxandersen]

@quintesse areyø ypu able to look Into this one or should I just make it error if it tried to edit file in cache?

[quintesse]

@quintesse areyø ypu able to look Into this one or should I just make it error if it tried to edit file in cache?

Sorry, got side-tracked, I will fix this first thing tomorrow!

[quintesse]

Ok, fixed