Hi,

there is a bug in std.getopt. The documentation says:
"To set timeout to 5, use either of the following: --timeout=5,
--timeout 5, --t=5, --t 5, or -t5. Forms such as -t 5 and -timeout=5
will be not accepted."
(http://www.digitalmars.com/d/2.0/phobos/std_getopt.html)

I.e. the following should work
unittest {
	auto args = ["program.name", "--timeout 5"];
	getopt(args, "timeout|t", &timeout);
	assert(timeout == 5);
	timeout = 0;

	args = ["program.name", "--t 5"];
	getopt(args, "timeout|t", &timeout);
	assert(timeout == 5);
	timeout = 0;
}

But it does not.
optMatch is missing the handling in case of a space character. Should
one adjust the documentation or handle the space character as
documented?
The attached patch fixes getopt to handle the space character properly.
Please review.

Jens
-------------- next part --------------
A non-text attachment was scrubbed...
Name: getopt.d-dmd.2.051.patch
Type: text/x-diff
Size: 2036 bytes
Desc: not available
URL: <http://lists.puremagic.com/pipermail/phobos/attachments/20110110/134b44ce/attachment.patch>

Ups. Please ignore my last message. My test case was wrong. Sorry. It works as expected.

Jens

Jens Mueller wrote:
> Hi,
> 
> there is a bug in std.getopt. The documentation says:
> "To set timeout to 5, use either of the following: --timeout=5,
> --timeout 5, --t=5, --t 5, or -t5. Forms such as -t 5 and -timeout=5
> will be not accepted."
> (http://www.digitalmars.com/d/2.0/phobos/std_getopt.html)
> 
> I.e. the following should work
> unittest {
> 	auto args = ["program.name", "--timeout 5"];
> 	getopt(args, "timeout|t", &timeout);
> 	assert(timeout == 5);
> 	timeout = 0;
> 
> 	args = ["program.name", "--t 5"];
> 	getopt(args, "timeout|t", &timeout);
> 	assert(timeout == 5);
> 	timeout = 0;
> }
> 
> But it does not.
> optMatch is missing the handling in case of a space character. Should
> one adjust the documentation or handle the space character as
> documented?
> The attached patch fixes getopt to handle the space character properly.
> Please review.
> 
> Jens

> --- /home/jkm/local/build/dmd2/src/phobos/std/getopt.d	2010-12-20 21:02:36.000000000 +0100
> +++ getopt.d	2011-01-10 12:54:46.000000000 +0100
> @@ -526,6 +526,8 @@
>   */
>  dchar assignChar = '=';
> 
> +dchar spaceChar = ' ';
> +
>  enum autoIncrementChar = '+';
> 
>  private struct configuration
> @@ -538,6 +540,14 @@
>                  ubyte, "", 4));
>  }
> 
> +private sizediff_t separatingPosition(in string arg) {
> +    immutable eqPos = std.string.indexOf(arg, assignChar);
> +    immutable spacePos = std.string.indexOf(arg, spaceChar);
> +	if (eqPos >= 0) return eqPos;
> +	if (spacePos >= 0) return spacePos;
> +	return -1;
> +}
> +
>  private bool optMatch(string arg, string optPattern, ref string value,
>      configuration cfg)
>  {
> @@ -550,12 +560,12 @@
>      //writeln("isLong: ", isLong);
>      // yank the second '-' if present
>      if (isLong) arg = arg[1 .. $];
> -    immutable eqPos = std.string.indexOf(arg, assignChar);
> -    if (eqPos >= 0)
> +    immutable sepPos = separatingPosition(arg);
> +    if (sepPos >= 0)
>      {
>          // argument looks like --opt=value
> -        value = arg[eqPos + 1 .. $];
> -        arg = arg[0 .. eqPos];
> +        value = arg[sepPos + 1 .. $];
> +        arg = arg[0 .. sepPos];
>      }
>      else
>      {
> @@ -712,6 +722,34 @@
>      assert(foo && !bar && args[1] == "nonoption" && args[2] == "--zab");
>  }
> 
> +unittest {
> +    uint timeout;
> +    auto args = ["program.name", "--timeout=5"];
> +    getopt(args, "timeout|t", &timeout);
> +    assert(timeout == 5);
> +    timeout = 0;
> +
> +    args = ["program.name", "--timeout 5"];
> +    getopt(args, "timeout|t", &timeout);
> +    assert(timeout == 5);
> +    timeout = 0;
> +
> +    args = ["program.name", "--t=5"];
> +    getopt(args, "timeout|t", &timeout);
> +    assert(timeout == 5);
> +    timeout = 0;
> +
> +    args = ["program.name", "--t 5"];
> +    getopt(args, "timeout|t", &timeout);
> +    assert(timeout == 5);
> +    timeout = 0;
> +
> +    args = ["program.name", "-t5"];
> +    getopt(args, "timeout|t", &timeout);
> +    assert(timeout == 5);
> +    timeout = 0;
> +}
> +
>  unittest
>  {
>      // From bugzilla 2142

> _______________________________________________
> phobos mailing list
> phobos at puremagic.com
> http://lists.puremagic.com/mailman/listinfo/phobos