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

Fails to fallback to the default command if no argument is passed #94

Closed
dhruvmanila opened this issue Aug 27, 2021 · 11 comments
Closed
Labels
question Further information is requested

Comments

@dhruvmanila
Copy link

I might've misinterpreted the default key, but here's my use case:

Assuming a CLI tool called foo, if I run ./foo without any arguments, it should run the default command as specified with the default key, but instead it outputs the usage and exit.

I think the reason is that in parse_requirements function, the default command comes after the fallback command, inverting the order fixes the issue:

# :command.command_fallback
"" )
ftp_usage
exit 1
;;
* )
action="upload"
ftp_upload_parse_requirements "$@"
shift $#
;;

@dhruvmanila
Copy link
Author

This is unrelated to this issue, but is there any reason for exiting with the exit code 1 when asking for help with -h/--help?

@DannyBen
Copy link
Owner

DannyBen commented Aug 27, 2021

Well, as for the fallback command, this is by design, and mentioned here:

Setting this to true on any command, will cause any unrecognized command line to be passed to this command.

The intention was to allow a shorthand syntax, and not to hijack the "no arguments" situation.

You can see a use case for it in rush, where you can run rush get <package> or rush <package>.

In other words, the default flag is designed only for commands with at least one mandatory argument. Perhaps I should mention it in the docs.

You can see how it behaves by adding default: true to the download command that is generated after running bashly init, and then just run ./cli filename on the generated script.

As for the help exiting with non zero, this is a side effect I need to check if can be changed.
The cause for this is that any unrecognized command shows usage, and exits with non zero. I need to check if it is easy to separate.

@dhruvmanila
Copy link
Author

Thanks for the quick response.

Right, understood. But is there any other way to create the "no argument" situation? I could convert all the commands to positional arguments and check if there were any arguments passed or not but that is not a good solution.

Another thing I noticed is we cannot pass multiple flags with one hyphen. So, -abc won't work and -a -b -c will.

@DannyBen
Copy link
Owner

DannyBen commented Aug 27, 2021

is there any other way to create the "no argument" situation?

No arguments, as in the big majority of command line tools, shows short usage.
Perhaps you can paste the most minimal YAML file of your use case so that I can better understand your intentions?

we cannot pass multiple flags with one hyphen. So, -abc won't work

Right. Not supported at this time. I believe parsing for such a case is more effort than its worth.

Also, don't forget that in some CLI apps, you can provide the argument of the short flag immediately after the short flag - for example --file, -f FILE enables the ability to provide -fREADME.md.

All these are - by design - out of scope, due to the high priority I assign to simplicity. If there is a simple way to do it, I am open to it.

@dhruvmanila
Copy link
Author

dhruvmanila commented Aug 27, 2021

Perhaps you can paste the most minimal YAML file of your use case so that I can better understand your intentions?

Maybe a description would help, if not then I will try to create a YAML file.

Assuming a CLI tool called foo which provides multiple commands such that ./foo bar, ./foo baz, ./foo -h and ./foo -v are the possible invocations. What I'm looking for is that if the user has not passed in any argument, invoking the tool with ./foo, the default would be to run the bar command. In other words, ./foo should be equivalent to ./foo bar.

Maybe, the default keyword could take in a string and check if that is a valid command in the configuration file. If it is, then that command will be the default behavior for the CLI tool, otherwise bashly should give an error that no such command exist.

One of my use case is that I have a script which upgrades all the tools and packages I have on my machine. If the script is called without any arguments, then everything should be upgraded, otherwise the specified tools/packages should be upgraded.

@DannyBen
Copy link
Owner

Thanks. I understand the feature, and your "script that upgrades tools" remark helps me undertsnad the use case.

Give me a few minutes to prepare an example or two of possible solutions. I will post a new comment when done.

In the meantime, I have changed the --help exit code to 0.
However, when running anything that invokes the short usage (including just the command name), it will still exit with 1, by design.

@DannyBen
Copy link
Owner

DannyBen commented Aug 27, 2021

Ok. Please take a look at https://github.com/DannyBen/bashly-support/tree/master/issue-94

There are two solutions there, that I think are intuitive, standards-compliant, and easy to write and maintain.

Solution 1

Treat your packages as arguments, not commands. It is semantically more correct I think. The sample code provided assumes that your tool is only responsible for upgrading stuff, but even if this is not the case, you can implement the same concept under your-tool upgrade command - the principles are exactly the same.

In this solution, the package name is optional, and if not provided, will "run all".

Solution 2

If, for whatever reason, you really prefer to have your package names as commands, and not arguments, then there is no choice but to have a your-tool all command.

Solution 3 - Use rush

Since I also faced your use case (or at least similar), in which I wanted an easy and reproducible way to install / upgrade everything on my system, I have created rush. This tool (created with bashly...) lets you write scripts that are doing whatever you want (in whatever language), and run them by doing rush ANYTHING,

The tool provides much more functionality, and you can see all the tools that I am personally installing with it in my personal rush-repo.

Thought I'd mention it, in case it is also of interest to you.

@DannyBen DannyBen added the question Further information is requested label Aug 27, 2021
@dhruvmanila
Copy link
Author

Thanks for providing comprehensive and multiple solutions.

I tried exactly the same technique as you have provided in solution 1 and I'm happy with that. I have also added the allowed keyword and might experiment with catch_all so that I can upgrade multiple packages, but not all.

rush seems like an amazing tool you have created and I will surely take a look at that.

I believe the issue can be closed now.

@DannyBen
Copy link
Owner

Excellent. And you are absolutely right - the allowed and catch_all directives are indeed good fit for this use case. I am glad the documentation was easy enough for you to find these.

If you need any help with bashly or rush, feel free to open an issue. My github issues philosophy, is that you can open support tickets :)

Thanks for opening it, and as for the --help exiting as 0, it is already implemented in master, and will be released in the next release. If this is something that you need urgently, I don't mind packing a release today, but if not, I will group it with the next feature or two.

@dhruvmanila
Copy link
Author

I am glad the documentation was easy enough for you to find these.

Yes, the documentation is well laid out.

Thanks for opening it, and as for the --help exiting as 0, it is already implemented in master, and will be released in the next release. If this is something that you need urgently, I don't mind packing a release today, but if not, I will group it with the next feature or two.

No, it's not urgent.

@DannyBen
Copy link
Owner

No, it's not urgent.

Well... something else came up, so version 0.6.4 is now live with the --help exit code fix as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants