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

Make command line GNU-compatible #1974

Closed
agross opened this issue Oct 23, 2016 · 11 comments
Closed

Make command line GNU-compatible #1974

agross opened this issue Oct 23, 2016 · 11 comments

Comments

@agross
Copy link
Contributor

agross commented Oct 23, 2016

While trying to implement zsh completion for Paket I found that the way Paket handles command line arguments is a bit off to what zsh regards as "the right way" which in turn makes implementing completion overly complex. I also found that some strings Paket accepts as arguments are (for reasons beyond my understanding) not written as such.

Take for example this line:

paket add nuget NUnit group Test project Foo.csproj --verbose --force

What's the command and what are parameters?

  • add nuget is the command
  • group is an argument
    • Test is a parameter to the group argument
  • project is an argument
    • Foo.csproj is a parameter to the group argument
  • --verbose is an argument that takes no parameters
  • --force is an argument that takes no parameters

So how should command line completion distinct between command and arguments (with or without parameters)? The short answer is that it can't. At least, I was not able to make it work with zsh without going great lengths and essentially reimplement the standard GNU-style arguments completion that ships with zsh (a 16KB function).

My suggestion is to align the command line to GNU-style:

paket add nuget NUnit --group Test --project Foo.csproj --verbose --force
paket add nuget NUnit --group=Test --project=Foo.csproj --verbose --force

Both should be regarded as the same. If the second line won't be parsed by Argu, that's fine with me as well.

I know this results in a breaking change, I suggest this to be planned for v4.

@forki
Copy link
Member

forki commented Oct 23, 2016

/cc @eiriktsarpalis

@matthid
Copy link
Member

matthid commented Oct 23, 2016

I couldn't agree more with this. It's understandable how it has historically grown to this but eventually we should sit down, do it properly and follow standards/best practices even when it hurts (as @agross has shown here standards have a good reason). Its even worse for FAKE ;)

@agross
Copy link
Contributor Author

agross commented Oct 23, 2016

While we're at it, I also suggest we have a hard look at the documented strings:

version <version>     Allows to specify version of the package.

Should read:

version <version>     version of the package
  • Imperative form instead of "allows" etc.
  • Lowercase all the strings (aligns better with Argu's hard-coded --help description)
  • Remove full stops if == 1 statement is made

@inosik
Copy link
Contributor

inosik commented Oct 24, 2016

I already suggested that kind of change for v2 in #1035.

On that note, paket add nuget Foo always bugged me. I think it would be nice if the package name would be a positional argument, instead of a named one. So it would be paket add Foo.

@agross
Copy link
Contributor Author

agross commented Oct 24, 2016

@inosik I think the rationale behind add nuget is that some day add http might be possible as well. One might argue that add-nuget and add-http might be a better fit, but git has similar subcommands, e.g. git bisect start.

@agross
Copy link
Contributor Author

agross commented Oct 24, 2016

@eiriktsarpalis
Copy link
Member

I believe Argu 3.0 supports the syntax that you describe.

@vbfox
Copy link
Contributor

vbfox commented Oct 27, 2016

On that note, paket add nuget Foo always bugged me. I think it would be nice if the package name would be a positional argument, instead of a named one. So it would be paket add Foo.

That's a case where it would be better to make it optional and default nuget. It's even worse from an user point of view in update or remove because paket can determine the source from the name (reading the lock file), except in the very rare case where 2 sources would have the package referenced with the same name.

Yarn support multiple sources and determine that from the passed in text already: https://yarnpkg.com/en/docs/cli/add

@agross
Copy link
Contributor Author

agross commented Jun 13, 2017

I spent some time to work on my zsh completion and GNU-style args for paket add. Feedback is appreciated!

https://asciinema.org/a/76d0gjmogm7f0vxntx05f2u94

@drusellers
Copy link

yas

@enricosada
Copy link
Collaborator

Fixed

USAGE: paket add [--help] [--version <version constraint>] [--project <path>] [--group <name>]
                 [--create-new-binding-files] [--force] [--interactive] [--redirects] [--clean-redirects]
                 [--no-install] [--no-resolve] [--keep-major] [--keep-minor] [--keep-patch] [--touch-affected-refs]
                 <package ID>

NUGET:

    <package ID>          NuGet package ID

OPTIONS:

    --version, -V <version constraint>
                          dependency version constraint
    --project, -p <path>  add the dependency to a single project only
    --group, -g <name>    add the dependency to a group (default: Main group)
    --create-new-binding-files
                          create binding redirect files if needed
    --force, -f           force download and reinstallation of all dependencies
    --interactive, -i     ask for every project whether to add the dependency
    --redirects           create binding redirects
    --clean-redirects     remove binding redirects that were not created by Paket
    --no-install          do not modify projects
    --no-resolve          do not resolve
    --keep-major          only allow updates that preserve the major version
    --keep-minor          only allow updates that preserve the minor version
    --keep-patch          only allow updates that preserve the patch version
    --touch-affected-refs touch project files referencing affected dependencies to help incremental build tools
                          detecting the change
    --silent, -s          suppress console output
    --verbose, -v         print detailed information to the console
    --log-file <path>     print output to a file
    --from-bootstrapper   call coming from the '--run' feature of the bootstrapper
    --help                display this list of options.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants