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

Allow flags for commands that have subcommands #258

Closed
taqtiqa-mark opened this issue Sep 27, 2022 · 20 comments · Fixed by #271
Closed

Allow flags for commands that have subcommands #258

taqtiqa-mark opened this issue Sep 27, 2022 · 20 comments · Fixed by #271
Labels
enhancement New feature or request

Comments

@taqtiqa-mark
Copy link

Thanks for the effort put into Bashly and for making it open source.

We have a legacy app that has 'root flags'. Is there a workaround for this?

@DannyBen
Copy link
Owner

DannyBen commented Sep 27, 2022

You mean having both subcommands and flags?
How does this work? The root flags are managed by which subcommand?

Share more details, and possibly a command line example so I can help.

@DannyBen DannyBen added the question Further information is requested label Sep 27, 2022
@taqtiqa-mark
Copy link
Author

taqtiqa-mark commented Sep 28, 2022

Thanks for the prompt response. Perhaps the best motivating example is a Docker style CLI, where there are flags at the root and command[0], etc. levels:

$ docker --help                                      
                                                         
Usage:  docker [OPTIONS] COMMAND  
                                            
A self-sufficient runtime for containers   
                                           
Options:                                      
      --config string      Location of client config files (default "/home/dev/.docker")
  -c, --context string     Name of the context to use to connect to the daemon (overrides DOCKER_HOST
                           env var and default context set with "docker context use")      
  -D, --debug              Enable debug mode                                 
  -H, --host list          Daemon socket(s) to connect to  
  -l, --log-level string   Set the logging level ("debug"|"info"|"warn"|"error"|"fatal") (default "info")
      --tls                Use TLS; implied by --tlsverify
      --tlscacert string   Trust certs signed only by this CA (default "/home/dev/.docker/ca.pem")
      --tlscert string     Path to TLS certificate file (default "/home/dev/.docker/cert.pem")
      --tlskey string      Path to TLS key file (default "/home/dev/.docker/key.pem")
      --tlsverify          Use TLS and verify the remote                          
  -v, --version            Print version information and quit
                                                              
Management Commands:                                                         
  builder     Manage builds                                                             
  buildx*     Docker Buildx (Docker Inc., 0.9.1+azure-1)    
....

Likewise, the sub-commands have flags/options

$ docker attach --help
Usage:  docker attach [OPTIONS] CONTAINER

Attach local standard input, output, and error streams to a running container

Options:
      --detach-keys string   Override the key sequence for detaching a container
      --no-stdin             Do not attach STDIN
      --sig-proxy            Proxy all received signals to the process (default true)

and also sub-sub-commands have flags/options

$ docker builder build --help                                                    
                                              
Usage:  docker builder build [OPTIONS] PATH | URL | -
                                                
Build an image from a Dockerfile                                                  
                                                                    
Options:                                                   
      --add-host list           Add a custom host-to-IP mapping (host:ip)
      --build-arg list          Set build-time variables
...

I had thought to try use env variables in place of flags but that becomes cumbersome for sub-sub- etc. commands. Example:

DOCKER_BUILDER_BUILD_ADDHOST=list, etc.

Appreciate any thoughts.

@DannyBen
Copy link
Owner

Right. Everything you mentioned can be done with bashly, with the exception of flags that do not belong to any command.

Sub commands, and sub-sub(-sub...)-commands - can have their own arguments and flags of course.

Personally, I find these general flags confusing (I mean why is docker --debug info better than docker info --debug), but I can see a use case where you have flags that influence ALL the commands, and are therefore desired to be available to all commands.

At this point, this is not possible - I need to dig deeper into the code to see if this limitation can be lifted without complications.

@taqtiqa-mark
Copy link
Author

taqtiqa-mark commented Sep 28, 2022

I can see a use case where you have flags that influence ALL the commands

Thanks for taking this into consideration. There is also an elegance to allowing a parent flag to apply to all child commands? Maybe inherits: [build-arg,...] with inherits: parent and inherits: root as special cases? The default being inherits: none for backwards compatibility.

Am I correct in saying this behavior does not apply to the environment configuration?

@DannyBen
Copy link
Owner

DannyBen commented Sep 28, 2022

There is also an elegance to allowing a parent flag to apply to all child commands?

While there may be elegance to it, you need to remember that this is a Ruby script that generates a bash script.... :)

There are limits to what can be done while still keeping the code clean and maintainable. After all, you must agree that the generated bashly script already supports quite a few nuances, that are sometimes even hard to achieve in a "normal" programming language.

What I am considering is allowing things like this:

$ cli --global-flags subcommand --subcommand-flags
# and maybe:
$ cli --global-flags subcommand --subcommand-global-flags sub-subcommand --sub-sub-flags

In other words, flags that are defined on a command that has subcommands, will cascade down to everything below it,

This will be in line with what the docker cli provides. I think that this can be achieved with an acceptable amount of effort.

The default being inherits: none for backwards compatibility.

Unless I misunderstand this statement, I don't think we will have a problem at all, nor will we need inherit.
The state today is that flags are not allowed on commands that have subcommands. If I succeed in implementing this, flags will be allowed, and they will be converted internally to "global flags" in case the command has subcommands. So we do not break any compatibility, just add another mode of operation.

Am I correct in saying this behavior does not apply to the environment configuration?

Environment variables can be defined in any command (regardless of if they have subcommands or not).
Note that - as opposed to flags and args - the environment_variables definition is mainly decorative, so it an be displayed in the usage help, with the added value of being able to define a default and a required argument.

Does this make sense?

@DannyBen DannyBen changed the title root cannot have both commands and flags: Workaround Allow flags for commands that have subcommands Sep 28, 2022
@DannyBen DannyBen added enhancement New feature or request and removed question Further information is requested labels Sep 28, 2022
@taqtiqa-mark
Copy link
Author

flags that are defined on a command that has subcommands, will cascade down to everything below it

Sounds reasonable. Order of precedence would be that child defaults and values override any parent values?

@DannyBen
Copy link
Owner

DannyBen commented Sep 28, 2022

Sounds reasonable. Order of precedence would be that child defaults and values override any parent values?

I am unsure yet, but as I understand it now:

  1. It is not intended to have the same flag name in both global and specific flags (cli --debug download --debug does not make sense)
  2. In case the developer defines flags with the same name in both the global section and the subcommand section, and the user provides both, the last one (in subcommand) will prevail.

From your questions, I am not 100% we envision the same thing, so to clarify. Consider this YAML (which is not possible today):

name: cli
help: Sample application
version: 0.1.0

# This section might be called `global_flags` - not sure yet.
flags:
- long: --force
  short: -f
  help: Overwrite existing files

commands:
- name: download
  alias: d
  help: Download a file

  args:
  - name: source
    help: URL to download from

  flags:
  - long: --user
    short: -u
    help: Username to use for logging in

In this case, running cli --force download --user will call the code in download_command.sh, and will pass both --force and --user.

Running just cli --force will result in an error. Meaning, once you define commands on a command, it means that the user must run one of the commands. There is no handler for the "root" command in such a case.

@taqtiqa-mark
Copy link
Author

taqtiqa-mark commented Sep 28, 2022

Thanks for engaging.

.... cli --debug download --debug does not make sense

Fair enough. However, cli --debug=true download --debug=false or cli --debug=info download --debug=trace seem natural - or any use case where the child uses a specialized variant of the parent logic?

@taqtiqa-mark
Copy link
Author

taqtiqa-mark commented Sep 28, 2022

I am not 100% we envision the same thing

I believe we do.

@DannyBen
Copy link
Owner

cli --debug=info download --debug=trace

I feel this is a bad form, but I believe the implementation I have in mind will accommodate this mutation.

@taqtiqa-mark
Copy link
Author

Running just cli --force will result in an error. ...There is no handler for the "root" command in such a case.

Interesting: git and docker return different status codes. Both show help to stdout or stderr as appropriate.

Could this be configurable?

@DannyBen
Copy link
Owner

Could this be configurable?

Unlikely. I feel this is pushing it way too far.
Running docker --debug (which is a valid flag) just shows usage with exit code 0.
Running docker --asdasd shows usage, with a non zero code.

However - I cannot say for certain how (and at this point, if) it will be implemented.
Will try to get to it over the weekend.

@taqtiqa-mark
Copy link
Author

cli --debug=info download --debug=trace

I feel this is a bad form, but I believe the implementation I have in mind will accommodate this mutation.

Agreed it is contrived. But, cli cmda --opt=a subcmdb --opt=b will make sense in some domains.

@taqtiqa-mark
Copy link
Author

taqtiqa-mark commented Sep 28, 2022

Running docker --debug (which is a valid flag) just shows usage with exit code 0.

Yet, running git -P (which is a valid flag) just shows usage with exit code 1.
Return codes and whether things are sent to stdout or stderr makes a big difference to monitoring and alerting.... and each user will have a different setup :)

@DannyBen
Copy link
Owner

I never followed which output is sent to stdout and which to stderr.
If you feel there needs to be a change in how bashly outputs, and what are the exit codes - feel free to open a separate feature request.

What I meant in my previous comment, is that since there is nothing that handles cli --global-flag when a command is not provided, it will be considered a non starter, and therefore either show usage or some sort of constructive error.

I, for one, do not like that docker --debug just shows a long usage as if I entered docker --help instead of just telling me that this syntax is unacceptable.

The fact that other CLIs do something does not make it right.

@taqtiqa-mark
Copy link
Author

The fact that other CLIs do something does not make it right.

Yeah, unfortunately we don't have the freedom to choose and must fit into the existing/surrounding infra, practices and conventions.

@DannyBen
Copy link
Owner

Yeah, unfortunately we don't have the freedom to choose and must fit into the existing/surrounding infra, practices and conventions.

Or be the pioneers in changing them :)

@taqtiqa-mark
Copy link
Author

I believe option handling would be aided by this:

ralish/bash-script-template#10

@DannyBen
Copy link
Owner

This is now implemented in the master branch and will be released soon.

If you need this immediately or wish to test the functionality:

  • If you are using the docker version, you can use the dannyben/bashly:edge image instead of the lastest
  • If you are using the Ruby version, you can install the gem from GitHub (let me know if you need help with that).

Thanks for bringing this up.

@DannyBen
Copy link
Owner

DannyBen commented Oct 1, 2022

This is now implemented in bashly 0.8.7

The relevant documentation bit was added to the Configuration / Command / flags documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants