-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Use p-ranav/argparse framework for command line argument parsing #9445
Conversation
5341f30
to
8e78e06
Compare
5212a05
to
f4a1350
Compare
eb2b522
to
dc69a7d
Compare
@@ -381,7 +381,7 @@ double CPLStrtodDelim(const char *nptr, char **endptr, char point) | |||
} | |||
else | |||
{ | |||
errno = answer.ptr == nptr ? EINVAL : ERANGE; | |||
errno = answer.ptr == nptr ? 0 : ERANGE; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this change? This isn't an error anymore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is to make it compliant with the documented semantics of strtod. This change was triggered by https://github.com/p-ranav/argparse which relies on that at https://github.com/p-ranav/argparse/blob/a1c41c5537c919c1a56661ec1cdf5a49b9e99af6/include/argparse/argparse.hpp#L380 to determine if a string is a number or not
Cf email thread at https://lists.osgeo.org/pipermail/gdal-dev/2024-March/thread.html#58647
Up to now,
sozip
,gdal_viewshed
,nearblack
,ogrinfo
,gdaladdo
,gdal_translate
,ogr2ogr
(so a mix of pure command line utilitys and utility-as-a-C-function) have been converted. This should exercise pretty much all what we need: optional and positional arguments, boolean/integer/double data types, repeated arguments, mutually exclusive arguments. So I'm quite confident now we could generalize the use of that framework to all other utilitiesI've had to extend p-ranav/argparse with various changes to improve the usage/synopsis output, and add some helpers to make it easier to use:
All of the above changes have been merged by upstream.
I'll try to submit them to upstream if they are interested into them (I've started with a bugfix to make the library usable with clang's -fsanitize=undefined-integer-overflow that we use in one of our CI config. EDIT: now merged upstream)I've added a GDALArgumentParser class extending the base argparse::ArgumentParse that adds a few GDAL specific helpers, in particuler to define arguments that are common to multiple utilities (like -of, -co, etc.)
A few examples:
sozip
without required argument outputs the short usage. This also demonstrates a separation between basic/regular and advanced options:sozip --long-usage
:gdal_viewshed --long-usage
:ogr2ogr
: Ported, and improve categorization of options