docopt / docopt.go

A command-line arguments parser that will make you smile.
http://docopt.org/
MIT License
1.43k stars 108 forks source link

Refactor and API improvements #41

Closed aviddiviner closed 6 years ago

aviddiviner commented 7 years ago

Hi there. I've been working on my own fork of this project for a while to clean up the API. I've reached a stage where I'm happy for my changes to be merged back in if you're interested in using them. You can see what's basically changed by reading the README: https://github.com/aviddiviner/docopt-go#api

But I'll list the salient points here for your summary:

And some other little fixes here and there. But mostly, it's about having a cleaner API, and being able to control the "exit and print usage" behaviour for testing.

Take a look. I hope you like it, and would want to merge it back in! Let me know 😄

aviddiviner commented 7 years ago

Oh, and I fixed the broken Travis build and all of that. The build for this PR will fail, because the examples all reference the new APIs at github.com/docopt/docopt-go (which aren't there yet). Take a look on my repo to see the build passing.

aviddiviner commented 7 years ago

I've added in a Bind API, which allows you to bind to a struct, assigning option values to the exported fields of that struct, all at once.

var config struct {
  Command string `docopt:"<cmd>"`
  Tries   int    `docopt:"-n"`
  Force   bool   // Gets the value of --force
}
opts.Bind(&config)
aviddiviner commented 7 years ago

By my count, this PR fixes these issues: https://github.com/docopt/docopt.go/issues/3, https://github.com/docopt/docopt.go/issues/4, https://github.com/docopt/docopt.go/issues/17, https://github.com/docopt/docopt.go/issues/31, https://github.com/docopt/docopt.go/issues/39

And provides the features requested by these PRs: https://github.com/docopt/docopt.go/pull/20, https://github.com/docopt/docopt.go/pull/30, https://github.com/docopt/docopt.go/pull/42

mboersma commented 6 years ago

@aviddiviner obviously this fell completely through the cracks--what can I say when something is ignored for over a year, other than I'm so sorry your good work was ignored.

I'm trying to rejuvanate interest and improve the health of docopt and docopt.go because I think @keleshev's idea is still brilliant. If you would be willing to revisit your improvements and rebase them against master, I promise they will not be ignored now.

aviddiviner commented 6 years ago

Cool. I've also pretty much forgotten about this, haven't looked at it for over a year. But I'll try dust it off some time this week and get the PR ready again.

aviddiviner commented 6 years ago

The last thing I was playing with before I stopped working on this repo was doing a bit of fuzz-testing. I found that the parser was a bit broken: https://github.com/aviddiviner/docopt-go/commit/ccbf38fb4394ca6bdabcc2083234fa3108f7eeb3

Are you interested in adding this commit? I suspect the whole parser will need a rewrite at some point to fix a bunch of the bugs that it picked up.

aviddiviner commented 6 years ago

Also, see my comment on Dec 1, 2016:

The build for this PR will fail, because the examples all reference the new APIs at github.com/docopt/docopt-go (which aren't there yet). Take a look on my repo to see the build passing.

aviddiviner commented 6 years ago

@mboersma This is ready to merge whenever you're ready.

mboersma commented 6 years ago

@aviddiviner fuzz testing sounds especially appropriate for this kind of library, that would be awesome. Also I'm not too surprised it turned up bugs, and we should fix them, but maybe hold off that effort until after this refactoring?

aviddiviner commented 6 years ago

Sure.

mboersma commented 6 years ago

@aviddiviner this looks great to me, didn't have any code comments after reviewing. Your focus on testability is perfect, that was always our complaint using docopt-go and why I added the hack in #9.

There's an 0.6.2 tag in this repo that can be used to reference the previous stable version, and since you left Parse() intact this should be backward-compatible regardless. I ran the tests locally and everything's green. Let's do it.

kovetskiy commented 6 years ago

🚀 hooray!