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

Suppress recipe not found error code #2460

Merged
merged 12 commits into from
Nov 23, 2024
Merged

Conversation

R3ZV
Copy link
Contributor

@R3ZV R3ZV commented Nov 10, 2024

Closes #2275. It adds a new verbosity version which is used to convert the error code of an UnknownRecipe to 0, due to it making an early return it will also suppress any error messages. This behaviour is the same as the one npm --if-present has as it was referenced by the issue author.

I choose to add another verbosity variant due to not wanting to add further fields in Config. I thought it fits the verbosity config since it affects the output of the program.

@laniakea64
Copy link
Contributor

The name --recipe-exists is neither descriptive nor intuitive for this functionality. Could this option be named --if-present, same as the npm option?

--if-present is also more natural considering how this option could mesh with optional imports/modules.

@R3ZV
Copy link
Contributor Author

R3ZV commented Nov 11, 2024

Fair enough, I will make the according naming changes, are there any changes that should be made, regarding either the implementation or the tests?

@casey
Copy link
Owner

casey commented Nov 11, 2024

Verbosity levels should only affect what's printed and not what error codes are returned, so I think this should be a flag. I'm not entirely satisfied with --if-present, but I could be swayed.

Other ideas:

  • --ignore-missing
  • --optional
  • --allow-missing

--optional is nice because it's short, and --allow-missing is nice because you can imagine other flags of the form --allow-X, to allow different errors.

@R3ZV
Copy link
Contributor Author

R3ZV commented Nov 12, 2024

I myself am a fan of either --ignore-missing or --allow-missing, I do have one question, by "this should be a flag" you mean to have another field in struct Config, or are there other mechanisms in the codebase? If it is the former perhaps I can add a field allow_missing to Config.

@casey
Copy link
Owner

casey commented Nov 17, 2024

Let's go with --allow-missing. Yup, it should be flag on Config.

@R3ZV R3ZV force-pushed the ignore-if-not-present branch from 5d9f9b6 to 4597aad Compare November 17, 2024 14:27
@R3ZV
Copy link
Contributor Author

R3ZV commented Nov 17, 2024

Changed everything to match the feedback, if there is anything else you would like me to change, please let me know!

Perhaps the description could be more specific than Suppress error code, maybe something like Suppresses 'UnknownRecipe' error code.

@casey
Copy link
Owner

casey commented Nov 23, 2024

I tweaked this a bit. The main change was to move the check for unknown recipes into Subcommand::run, which makes it only apply when running a recipe, and not other subcommands, like --show which still return an error if --allow-missing is supplied and the recipe is missing. I also extended it to missing submodules, so just --allow-missing foo::bar where there is no foo module is okay.

@casey casey enabled auto-merge (squash) November 23, 2024 20:19
@casey casey merged commit 40840a0 into casey:master Nov 23, 2024
5 checks passed
@R3ZV R3ZV deleted the ignore-if-not-present branch December 20, 2024 20:59
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.

Command line flag to exit without error if recipe not found
3 participants