search

bmjames / scala-optparse-applicative

Scala port of Paolo Capriotti's optparse-applicative library. This repository is no longer maintained; newer versions exist in this fork: https://github.com/xuwei-k/optparse-applicative
BSD 3-Clause "New" or "Revised" License
72 stars 9 forks source link

Remove Kiama dependency #13

Closed coltfred closed 7 years ago

coltfred commented 7 years ago

This is a proposed resolution for #7. I ported Wadler's paper http://homepages.inf.ed.ac.uk/wadler/papers/prettier/prettier.pdf in a fairly straight forward way. I needed to add column and nesting which are improvements that have been made to the haskell PPrint lib.

I need to add a few more tests, but the laws hold and manual testing of argument lists looked good.

cc/ @bmjames @tpolecat - Thoughts?

tpolecat commented 7 years ago

I will pull this and test it out with my stuff tomorrow. Looks great!

coltfred commented 7 years ago

@tpolecat I incorporated your comments. Please do let me know if you have more or if you see anything weird. I still need to write more tests, but it's getting late. :sleeping:

bmjames commented 7 years ago

This is looking great, thanks :-)

bmjames commented 7 years ago

I didn't get time to look into why, but I noticed that printing the help doc for SubparserExample (sbt "test:runMain net.bmjames.opts.test.example.SubparserExample --help") seems to be non-terminating.

coltfred commented 7 years ago

@bmjames I'll take a look tonight. Thanks for finding that!

coltfred commented 7 years ago

@bmjames @tpolecat Turns out my port from haskell didn't take into consideration that haskell's laziness is awesome. I was literally exploring the entire rendering space even if I didn't have to. I went from calling findBestCore 53,758,517 times to calling it 390. Needless to say, it's much faster now. :laughing:

Also, just for completeness sake, the test @bmjames was running would eventually terminate, it just takes 60+ seconds.

Do either of you have other complaints?

bmjames commented 7 years ago

It looks good to me.

@tpolecat, did you get a chance to try it out?

tpolecat commented 7 years ago

I haven't. I will do that today.

tpolecat commented 7 years ago

Yep, sorry for the delay. Tried it out and works great. 👍

bmjames commented 7 years ago

@coltfred Would you mind reverting the changes to README and squashing the rest of the commits please? Then I'll merge.

The README changes are fine, but I'd like to apply them when I'm ready to make a release, as the latest release referred to in the README (0.5) still contains the Kiama dependency.

tpolecat commented 7 years ago

I wish there were a better way to manage doc changes that need to be "on hold" until a release. I run into this too.

coltfred commented 7 years ago

@bmjames I've reverted. You can squash my branch into one commit on master using the squash and merge feature that was added earlier this year. https://github.com/blog/2141-squash-your-commits

Would you still like me to squash or will that be acceptable?

bmjames commented 7 years ago

@coltfred That will be entirely acceptable, thanks for pointing that out!

Thanks for the changes, and also thanks @tpolecat for reviewing.