Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix skipping one program during check #318

Merged
merged 3 commits into from
Jan 7, 2024
Merged

Fix skipping one program during check #318

merged 3 commits into from
Jan 7, 2024

Conversation

b3nj5m1n
Copy link
Owner

When there's only one file in the programs/ directory, it does not get picked up by xdg-ninja.

Reproducing:

git clone git@github.com:b3nj5m1n/xdg-ninja.git && cd xdg-ninja
mv programs/abook.json . && rm programs/* && mv abook.json programs
./xdg-ninja.sh -v

This doesn't produce any output for abook.json on the main branch.

Since I found this accidentally during testing #271 and "fixed" it using ChatGPT, I'm not sure if this may introduce other unexpected side effects.

@Ballasi
Copy link
Contributor

Ballasi commented Sep 28, 2023

From my tests, the bug actually seems to be a bit more critical.

It seems like it skips the first check in all cases, not just when there's one file in programs/. In the current program, this means skipping the check of FreeCAD.

From what it seems, the issue comes from the inputs tag in jq, I quickly did a Google search but couldn't find anything in 10s. By replacing it to . (what I've often seen in jq as being "the root of the JSON file"), it works, and it happens to be one of the things ChatGPT did. We would then have something similar to this:

jq '. as $input | $input.files[] as $file | $input.name, $file.path, $file.movable, $file.help' "$XN_PROGRAMS_DIR"/*

One other thing ChatGPT did is completely get rid of the $input variable, since it is now defined as . you can just replace the instances of $input to virtually nothing (you don't write $input.name->..name but just .name), so you would have something like this:

jq '.files[] as $file | .name, $file.path, $file.movable, $file.help' "$XN_PROGRAMS_DIR"/*

That would be what I would tend to put in the end.

For some reason, ChatGPT checks if the JSON object is an object (if type == "object" then) and spits it out as raw if not (else . end). That's probably not a good idea to keep that as it is then given to the read command which might mess things up because it might not just return four lines.

It's better to have a catch case in here in case there's an issue (without it, jq should just return an error, we need to check that but I believe it makes the app itself crash, so it's already a "catch case" without that change).

I would tend to go with the last line I gave you, not the one ChatGPT gave you. I'll check what's up with that inputs thing and what it's used for in jq, but that's what causing the issue here.

@Ballasi
Copy link
Contributor

Ballasi commented Sep 28, 2023

Related stackoverflow issue concerning inputs: https://stackoverflow.com/a/55996042

xdg-ninja.sh Outdated Show resolved Hide resolved
@b3nj5m1n
Copy link
Owner Author

b3nj5m1n commented Jan 7, 2024

Sorry for taking so long to respond again, I've implemented your suggestion and the issue seems to be fixed, so I think we're good to go.

@b3nj5m1n b3nj5m1n merged commit ff5ea3a into main Jan 7, 2024
4 checks passed
@b3nj5m1n b3nj5m1n deleted the fix-missing-program branch January 7, 2024 07:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants