GNOME Bugzilla – Bug 329548
Add G_OPTION_ARG_DOUBLE
Last modified: 2011-02-18 15:49:38 UTC
That would be useful I guess.
Marking for love. Needs docs too.
Hello, I think I can do the hack, but I have a few questions. 1) Should this option expect both locale formatted and C formatted numbers ? 2) Should we use strtod, g_strtod or g_ascii_strtod to convert the string to double ? 3) Does the position of G_OPTION_ARG_DOUBLE in GOptionArg enum matter ? 4) My english isn't very good, but I am willing to change the docs. Is docs/reference/glib/tmpl/option.sgml the only file to change ? Cordially
1) no, imo 2) g_ascii_strtod() 3) yes, because the existing values get compiled into GOption-using apps. you need to append at the end 4) yes, since you are only adding an enum value to the api (function docs live in the c source next to the function)
Created attachment 58675 [details] [review] patch against cvs head with cvs diff up Hello, this patch should do the job. The double argument must be a C formatted number. I have add a test in option-test.c for double argument parsing. It has succeed, but I am ready to add other tests if necessary. Have a nice day.
*** Bug 336083 has been marked as a duplicate of this bug. ***
Matthias - question for you. I was wondering if strtod() would be more appropriate here, since the command-line represents a UI of sorts. Since the shell/command line syntax is the UI, shouldn't the number be formatted according to the user's locale? Thanks.
Don't know, really. I have very little experience with entering localized floats on the commandline. If we use strtod(), we introduce a locale-dependence in goption which wasn't there before, so we will have to add some documentation about calling or not calling setlocale() before using the goption api. I could be convinced otherwise though. Maybe the right approach is to use g_strtod() here, which first tries strtod() and then g_ascii_strtod()
I whole-heartedly endorse using g_strtod() then. Is it ok to commit this patch to HEAD once that's in place?
yes. it would be nice to add a test for parsing localized floats, and maybe the docs should mention it too. if you want to do that, feel free to commit it. otherwise, i'll look into it tomorrow.
Created attachment 62059 [details] [review] Patch that does what Matthias requested I'm a bit hesitant about test5(), since we can't be sure that the tester has a non-ASCII locale to switch to, or what locale that might be. For the moment, it's commented out. But everything else should be implemented as-requested. I'll let you decide what's best. Thanks.
Using the "de" locale in a test is fine with me. We already have some tests which can fail if certain locales/encodings are not available. And the only time when the tests really have to pass is when I am doing a release.
2006-03-27 Matthias Clasen <mclasen@redhat.com> Add support for floating point numbers to goption. (#329548, Behdad Esfahbod, patch by Antoine Dopffer and Dom Lachowicz) * glib/goption.h: * glib/goption.c: Support double arguments. * tests/option-test.c: Test double arguments.`
2006-03-27 Dom Lachowicz <cinamod@hotmail.com> * tests/option-test.c: Copy-and-paste error slipped into test5. Enable test5, as per Matthias' comments in bug 329548#c11.