optparse-applicative icon indicating copy to clipboard operation
optparse-applicative copied to clipboard

hang when providing default for [String] arguments

Open eflister opened this issue 12 years ago • 18 comments

in 0.5.2.1 i could have a ["string with spaces", "another one"] default for an arguments str. now in 0.7.0.2 that doesn't compile, i have to have ""string with spaces", "another one"", but that hangs (on one computer, another OOM's) when i execParser it!

also, it wasn't clear from the docs that i can't use auto for [String] arguments, took me a while to figure out i had to use str...

eflister avatar Oct 24 '13 03:10 eflister

Thanks for the report.

The problem is that value modifier does not really do what you expect. It actually provides a default value for every occurrence of the argument. So for example

arguments str (value "foo")

behaves like

many (pure "foo")

when given an empty command line. The latter parser clearly hangs, because it's trying to return an infinite list of "foo" values (and it's not lazy enough to do it successfully).

Now, I could try to fix the arguments builder, and have it accept modifiers for [a] instead of a, but I think it's better to deprecate arguments altogether (since arguments r = many . argument r).

To get the behaviour you expect, you can use something like:

some (argument str idm) <|> pure ["foo", "bar"]

pcapriotti avatar Oct 24 '13 09:10 pcapriotti

i get the same hang even for non-empty command lines.

apparently the same reasoning applies to arguments1 as well. why is an infinite list the right semantics for value? what was wrong with the 0.5.2.1 behavior? i'd like you to keep arguments(1) with a sensical value modifier (i understand if it has to take a String rather than [String], but 0.5.2.1's [String] was most convenient and intuitive), since i can't (easily?) hand-roll a parser that has all the right effects (for instance, stating the default value in the help).

eflister avatar Oct 24 '13 10:10 eflister

i get the same hang even for non-empty command lines.

Sure. I wasn't trying to say that it only hangs for empty command lines, just that the equivalence above holds when the command line is empty.

why is an infinite list the right semantics for value?

An infinite list is the right semantics of many p where p is a parser that always succeeds, like, for example, a parser with a default value.

what was wrong with the 0.5.2.1 behavior? i'd like you to keep arguments(1) with a sensical value modifier

arguments r m is now exactly the same as many (argument r m). In particular, the modifier applies to the underlying argument parser, not to the global parser. This removes a lot of special cases, so I think it is an improvement to the API.

(i understand if it has to take a String rather than [String], but 0.5.2.1's [String] was most convenient and intuitive), since i can't (easily?) hand-roll a parser that has all the right effects (for instance, stating the default value in the help

I don't understand this complaint. What is wrong with what I suggested above?

some (argument str idm) <|> pure ["foo", "bar"]

pcapriotti avatar Oct 24 '13 10:10 pcapriotti

why is an infinite list the right semantics for value?

An infinite list is the right semantics of many p where p is a parser that always succeeds, like, for example, a parser with a default value.

that's a statement of exactly why arguments /= many . argument. an argument list with a finite default value is clearly a finite list!

what was wrong with the 0.5.2.1 behavior? i'd like you to keep arguments(1) with a sensical value modifier

arguments r m is now exactly the same as many (argument r m). In particular, the modifier applies to the underlying argument parser, not to the global parser. This removes a lot of special cases, so I think it is an improvement to the API.

special cases in the implementation? i didn't see an api change other than value no longer having the same type as the parser it is modifying, which was a beautiful design. the new design creates this unexpected special case in the api.

(i understand if it has to take a String rather than [String], but 0.5.2.1's [String] was most convenient and intuitive), since i can't (easily?) hand-roll a parser that has all the right effects (for instance, stating the default value in the help

I don't understand this complaint. What is wrong with what I suggested above?

some (argument str idm) <|> pure ["foo", "bar"]

that won't have the pretty effect of automatically showing the default value in the help listed with the -h flag.

eflister avatar Oct 24 '13 15:10 eflister

that's a statement of exactly why arguments /= many . argument. an argument list with a finite default value is clearly a finite list!

Yes, and you can express it with many (argument str idm) <|> pure defaultList.

special cases in the implementation? i didn't see an api change other than value no longer having the same type as the parser it is modifying, which was a beautiful design. the new design creates this unexpected special case in the api.

Yes, I was referring to special cases in the implementation. The API is also more consistent now (after the deprecation of arguments), since modifiers can only be applied to "atomic" parsers, and not composite ones.

that won't have the pretty effect of automatically showing the default value in the help listed with the -h flag

Ah, that's a fair point. You can always specify it manually in the help text, though.

I'll reopen the issue for now, but I'm not sure if there is a clean solution.

pcapriotti avatar Oct 24 '13 18:10 pcapriotti

special cases in the implementation? i didn't see an api change other than value no longer having the same type as the parser it is modifying, which was a beautiful design. the new design creates this unexpected special case in the api.

Yes, I was referring to special cases in the implementation. The API is also more consistent now (after the deprecation of arguments), since modifiers can only be applied to "atomic" parsers, and not composite ones.

you just described a special case but called it consistency.

that won't have the pretty effect of automatically showing the default value in the help listed with the -h flag

Ah, that's a fair point. You can always specify it manually in the help text, though.

the whole beauty of optparse-applicative is not doing such things!

I'll reopen the issue for now, but I'm not sure that there is a clean solution.

thanks. i like the old way.

eflister avatar Oct 24 '13 21:10 eflister

I just tripped over this as well. There's no way I would have figured out the workaround idiom you describe. At the very least I'd encourage you to describe it with loud shouting WARNING exclamations in the Haddock documentation, but the idea that the "default" isn't the default for values in an array (which can't happen, because you specified at least one to trigger that code path) really doesn't make any sense; default is for when the user didn't specify the option. I'm just lucky I found this bug here; I was about to give up on optparse-applicative.

AfC

istathar avatar Mar 04 '14 01:03 istathar

I don't quite get the work around presented here. I have written the parser using the alternative instance something like this:

data Foo = A | D String

parser :: Parser [Foo]
parser = many $
  flag' A (short 'a')
  <|> (D <$> strOption (short 'd' <> value "foo"))

How would I apply the workaround here?

funrep avatar May 10 '14 09:05 funrep

Just remove the default value for D.

pcapriotti avatar May 10 '14 09:05 pcapriotti

You mean something like this?

((D <$> strOption (short 'd')) <|> pure (D "foo"))

funrep avatar May 10 '14 10:05 funrep

No, I meant D <$> strOption (short 'd').

I'm not sure why you want a default there, it doesn't really make sense. If the option is missing, then it will just not be included in the resulting list. As you wrote it, it will return an infinite list with a tail of D "foo", which will make the whole thing hang.

pcapriotti avatar May 10 '14 10:05 pcapriotti

Well, I have a flag which have an optional argument. I'm currently rewriting a command line parser written simply by pattern matching [String] from getArgs. In this there is an option which if it have no argument should default to "Main.main", when it have an argument then use that one, if not even specified the flag should not occur in the resulting list.

funrep avatar May 10 '14 10:05 funrep

Optional arguments to regular options are not supported (#67). You can use a different option, though:

many $
  flag' A (short 'a')
  <|> (D <$> strOption (short 'd'))
  <|> (flag' (D "foo") (short 'e'))

Would that be acceptable?

pcapriotti avatar May 10 '14 10:05 pcapriotti

Thanks for clarifying, that will be acceptable. :)

funrep avatar May 10 '14 10:05 funrep

wait wait wait we left this open cuz of the discussion in 2013 -- the 2014 discussion doesn't resolve those issues!

eflister avatar Aug 27 '15 04:08 eflister

Ok

HuwCampbell avatar Aug 27 '15 05:08 HuwCampbell

I've just added a warning regarding value in the docs to say don't use it with many and some as this will cause a hang (arguments has been gone for a while). So the issue as I understand it is that there's no automatic way of showing (default: [0]). Or is there something else you think is not correct as well?

HuwCampbell avatar Aug 27 '15 08:08 HuwCampbell

0.5.2.1 design was good, the change makes the api inconsistent (requires unintuitive workaround) and prevents nice handling (eg, auto display of the value in the help) for default values that happen to be lists. (it's been a long time, so i don't remember the details, but that's the flavor)

eflister avatar Aug 27 '15 17:08 eflister