Skip to content

Conversation

ghost
Copy link

@ghost ghost commented Sep 3, 2013

@monarchdodra
Copy link
Collaborator

The problem is actually one of passing an odd number of arguments. This currently works:

    auto args = [""];
    getopt(args);

But this fails:

    auto args = [""];
    int i;
    getopt(args, "i", &i, "i");

I think this requires more of an assertive fix that:

  1. The length of T is even.
  2. The odd arguments are string types.
  3. The even arguments are pointer types, functions or delegates.

It's OK if it doesn't compile, as long as the error from a verbose assert.

@ghost
Copy link
Author

ghost commented Sep 3, 2013

Well in that case I don't think either approach should be made, since we could add a new backward-compatible API where the oddness/even-ness of the arguments doesn't matter, e.g.:

getopt(args, "count", &count, @doc("this is the thread count"), "name", &name);

This will have 5 arguments. Anyway, depending on particular argument layout with complex rules is not going to make a good API, so maybe we should not touch this code for now.

@ghost ghost closed this Sep 3, 2013
@ghost
Copy link
Author

ghost commented Sep 3, 2013

I forgot to mention: this is with regards to recent NG discussions about adding an optional helper argument to each getopt option, which we haven't decided yet how we're going to implement. But we shouldn't give ourselves double the work by adding complex checking now and removing it later.

@monarchdodra
Copy link
Collaborator

But we shouldn't give ourselves double the work by adding complex checking now and removing it later.

True, but I don't think it's too complicated to check each argument exists and is the correct type as you are using/compiling them.

I mean, the error on line 392:

        else
        {
            // it's an option string
            auto option = to!string(opts[0]);
            auto receiver = opts[1];

Could at the very least simply be:

        else
        {
            // it's an option string
            static assert(isSomeString!(T[0]), "unexpected non-string option.");
            auto option = to!string(opts[0]);
            static assert(opts.length > 1, "missing receiver for last option.");
            auto receiver = opts[1];

Sprinkling static asserts never hurt anyone. The current implementation is just assuming everything is perfectly pass, which gives it the impression it is "choking".

@ghost
Copy link
Author

ghost commented Sep 3, 2013

Ah yeah, we could add those. This would also fix http://d.puremagic.com/issues/show_bug.cgi?id=10956.

This pull request was closed.
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