xdg-ninja: add shell pattern matching capability

[Ballasi]

Relaunching this to spark some discussion about #263.

The implementation wouldn't solve the first point talked about in the issue but would make up for the rest of the issue. I can also take a look at adding capability for multi-files.

I quickly tested it on my side but I'd rather have someone double check this works indeed fine, @Midblyte would you be interested in rebasing your patch on this one to test out #264?

I have also changed the check_file behavior to retrieve the file name so that $HOME/.test-* would be spelled out with the actual file name if it is fine (e.g., it would print /home/user/.test-2023).

Since calls to find is quite costy, I have also added a has_pattern function to use shell pattern maching in the case the string contains *, ? or [something].

[Midblyte]

I tried but due to my lack of capacity & time to waste on git, I couldn't rebase my patch on yours without messing up everything. I think I'll need some help to do it properly.
However, by manually applying your changes to my patch, yes, it seems to work fine.

In the meantime, pushing some other improvements to #264 now.

[Ballasi]

I am not even sure if GitHub has the capability to have a PR depend on another? What you did is probably just fine as long as my PR gets merged first.

I think I'll need some help to do it properly.

If you want to try to rebase your changes on top of mine, this should do the trick:

$ git remote add ballasi [email protected]:fourchettes/xdg-ninja.git
$ git fetch ballasi
$ git checkout main
$ git rebase shell-pattern-matching
$ git push -f origin main

We'll probably need to check if my commits doesn't get added in your PR. If that doesn't work as intended, you can go back to the initial state by doing the following:

$ git checkout 5c14da9 -B main
$ git push -f origin main

I am curious to know how GitHub would behave in that case, lmk how that went!

Edit: it's apparently impossible in GitHub, so you're better off just keeping it as is.

[Ballasi]

@b3nj5m1n Any updates on this? If there's anything wrong/that you don't like about this change, I'd happily discuss about it.

[b3nj5m1n]

I'm so sorry it took me this long to answer, I have no idea how I forgot about it.

One problem is that the haskell version of the client doesn't support this, but since I don't think anyone (myself included) uses that for anything other than adding configs, I think that's probably alright.

I'm also pretty sure find isn't always installed, so we're technically adding another dependency. We'll have to add that to the README, as well as the nix flake, possibly other package sources as well. Since it is find though and most people will have it installed, I don't think this is a problem either.

While trying this out, I noticed an apparent bug in the normal xdg-ninja script, it seems like if there is only one file in programs/ (rm programs/*), it doesn't read it. If you make a copy of that file (cp programs/ohmyzsh.json programs/ohmyzsh2.json), it now reads only one of them. This makes me believe that xdg-ninja is always silently swallowing one file. I believe this bug was introduced by one of the many optimisations, and nobody noticed since there are now so many programs supported. It seems like this can be fixed by changing the current line 221

xdg-ninja/xdg-ninja.sh

Line 221 in 4aac3fb

$(jq 'inputs as $input | $input.files[] as $file | $input.name, $file.path, $file.movable, $file.help' "$XN_PROGRAMS_DIR"/* | sed -e 's/^"//' -e 's/"$//')

to this:

$(jq 'if type == "object" then .files[] as $file | .name, $file.path, $file.movable, $file.help else . end' programs/* | sed -e 's/^"//' -e 's/"$//')

At least that's what ChatGPT gave me, it seems to work in the few tests I've done, but I'd have to look more into the jq syntax, it's been way too long since I've done anything with it.

---

TLDR: We need to add find as a dependency, and there's a bug in the current version of the xdg-ninja script which doesn't seem to have anything to do with this PR.
In general this looks good to me, I'm really sorry for taking so long to respond.

[b3nj5m1n]

I don't seem to be able to push to this PR, but this patch should do it for now:

From cfc1e56b7b3be5a0cccc9ebf51349c0e77afb462 Mon Sep 17 00:00:00 2001
From: b3nj5m1n <[email protected]>
Date: Tue, 26 Sep 2023 22:22:38 +0200
Subject: [PATCH] Add findutils as dependency

---
 README.md | 1 +
 flake.nix | 1 +
 2 files changed, 2 insertions(+)

diff --git a/README.md b/README.md
index 6c3bc5e..9af8c77 100644
--- a/README.md
+++ b/README.md
@@ -36,6 +36,7 @@ To install xdg-ninja with [Homebrew](https://brew.sh), run `brew install xdg-nin
 
 - your favorite POSIX-compliant shell ([bash](https://repology.org/project/bash/packages), [zsh](https://repology.org/project/zsh/packages), [dash](https://repology.org/project/dash-shell/packages), ...)
 - [jq](https://repology.org/project/jq/packages) for parsing the json files
+- [find](https://repology.org/project/findutils/versions)
 
 ### Optional
 
diff --git a/flake.nix b/flake.nix
index 8f3a5bf..5a1be2c 100644
--- a/flake.nix
+++ b/flake.nix
@@ -13,6 +13,7 @@
         runtimeDependencies = with pkgs; [
           glow
           jq
+          findutils
         ];
         overlays = [
           (self: super: {
-- 
2.41.0

[Ballasi]

I'm really sorry for taking so long to respond

No problem at all!

We need to add find as a dependency

That's true, for some reason since find is defined by the POSIX specifications, I expected it to be included by default, but you are right. I am force-pushing with your patch in a second.

[Ballasi]

By the way, you should probably make an issue on GitHub specific for what you found. I haven't tested it on my side yet, I will and I'll keep you posted there.