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

adding new arg parser #6

Merged
merged 6 commits into from
Dec 13, 2015

Conversation

jvarley
Copy link
Member

@jvarley jvarley commented Dec 10, 2015

adding new arg parser, useful help message, and makes all arguments handled in a uniform way.
Functionality for loading hand, obstacle, object is the same, just more clear, changes way in which plugins are loaded in order to have uniform argument format. Also --help now provides useful message for how to pass in args.

@mateiciocarlie
Copy link
Member

I am reluctant to add more third-party code into Graspit. Especially with such a vague license clause: "License: your favourite BSD-style license"

An alternative could be boost parsing.

@jvarley
Copy link
Member Author

jvarley commented Dec 10, 2015

What if I got the author of that library to specify the license for the code? If not, I agree, boost program options would be a reasonable alternative, although that would mean that boost would be a requirement for graspit to run. If you are ok with that, then I can switch it over.

http://www.boost.org/doc/libs/1_59_0/doc/html/program_options/tutorial.html

@mateiciocarlie
Copy link
Member

You are right, not sure this is a good enough reason to bring in such a big dependency as Boost.

If the authors is willing to put in a proper BSD header in all files that should be good enough...

Also, please remove "using namespace optparse", especially from header files.

@jvarley
Copy link
Member Author

jvarley commented Dec 11, 2015

Switched to a different library that has a better defined license.

@mateiciocarlie
Copy link
Member

See note below in new license:

"Redistributions in binary form must reproduce the above copyright notice, this list of conditions and the following disclaimer in the documentation and/or other materials provided with the distribution."

Please add the license of cmdline in our hyperlatex manual, in the section where we mention 3rd party software...

@jvarley
Copy link
Member Author

jvarley commented Dec 12, 2015

Done, I think. I added it to License.txt where ply and tinyxml etc are referenced, I wasn't sure where it should be referenced in the manual.

mateiciocarlie added a commit that referenced this pull request Dec 13, 2015
@mateiciocarlie mateiciocarlie merged commit ddce8b8 into graspit-simulator:master Dec 13, 2015
@jvarley jvarley deleted the adding_cpp_argparser branch January 12, 2016 21:52
jvarley pushed a commit to jvarley/graspit that referenced this pull request May 9, 2016
@disdaining disdaining mentioned this pull request Aug 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants