Skip to content
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

problem combining partially overlapping parsers with Alternative #333

Open
joeyh opened this issue Mar 6, 2019 · 4 comments
Open

problem combining partially overlapping parsers with Alternative #333

joeyh opened this issue Mar 6, 2019 · 4 comments

Comments

@joeyh
Copy link

joeyh commented Mar 6, 2019

The test program below prints this --help

Usage: foo ([--bar ARG] [string] | [string])

And "./foo --bar a b" works. However:

joey@darkstar:~>./foo a
Missing: --bar ARG

Seems that in bar <|> foo, the bar parser can fail in a way that makes it never try the
foo parser, which would succeed if it were tried.

If it's changed to foo <|> bar, the example above works, as does "./foo --bar a b" , so that version of the program works ok. Except, "./foo b --bar a" does not work; normally the bar
parser would accept that, but now the foo parser fails in a way that makes the bar parser not work.

import Options.Applicative
import Options.Applicative.Internal

data Foo = Foo [String] | Bar String String
        deriving (Show)

parser :: Parser Foo
parser = bar <|> foo
  where
        foo = Foo <$> many (argument str ( metavar ""))
        bar = Bar <$> strOption (long "bar") <*> argument str (metavar "dir")

main = print =<< execParser opts
  where
         opts = info (parser <**> helper)
                ( fullDesc
                <> progDesc "testcase"
                <> header "testcase"
                )
@HuwCampbell
Copy link
Collaborator

optparse has semantics more equivalent to Parsec compared to Attoparsec, in that branch selection doesn't backtrack on branch failure (Parsec has a primitive try to support extra backtracking).

What's happened with "./foo a" is:

  • the first branch has been searched, and an appropriate positional has been found.
  • that branch is selected, and the argument is embedded into the parse tree
  • the remaining parser, which is morally equivalent to Bar <$> strOption (long "bar") <*> pure "a" is now the active parser, waiting for more input.
  • none is found, hence the Missing: --bar ARG error message.

There are a few good reasons for this, but I think the clearest is to consider something like this:

foo =   (,) <$> flag' "FLAG" (short 'this') <*> strArgument (mempty) <* flag' () (short 'that')
   <|>  (,) <$> strOption (short 'this') <*> pure "what?"

If we supported the backtracking you're alluding to:
--this hello --that would give ("FLAG", "hello")
while
--this hello would give ("hello", "what?")

Even worse would be if we reversed the order of these choices

foo =   (,) <$> strOption (short 'this') <*> pure "what?"
   <|>  (,) <$> flag' "This" (short 'this') <*> strArgument (mempty) <* flag' () (short 'that')

Here,
--this --that would give ("--that", "what?")
while
--this x --that would have to give ("FLAG", "x")

In essence, permitting full backtracking or parallel branch evaluation means some terms can be absorbed both as positionals and option arguments, or flags and options. It would be downright confusing.

Hopefully that's been persuasive. I'm happy to help with any interface design if you can point me to a code example.

PS. loved your posts on your fridge! That's a great little project.

@HuwCampbell
Copy link
Collaborator

Is that answer satisfactory @joeyh?

If you have other ideas around how to deal with these issues I'm open, but I think it's important to not accidentally permit challenging parsers like those listed above.

@rihardsk
Copy link

rihardsk commented Sep 29, 2023

I just ran into this with

data ModelOutputOpts
  = OutputSamples FilePath
  | OutputPlot FilePath
  | OutputBoth FilePath FilePath

modelOutputParser :: Parser ModelOutputOpts
modelOutputParser
  = OutputBoth <$> sampleOptionHelp <*> plotOptionHelp
  <|> OutputSamples <$> sampleOption
  <|> OutputPlot <$> plotOption
  where
    sampleSpec = long "output-samples" <> short 'o' <> metavar "FILEPATH"
    sampleOption = strOption sampleSpec
    sampleOptionHelp = strOption (sampleSpec <> help "Save samples from the distribution to FILEPATH")
    plotSpec = long "output-plot" <> short 'p' <> metavar "FILEPATH"
    plotOption = strOption plotSpec
    plotOptionHelp = strOption (plotSpec <> help "Plot the resulting distribution to FILEPATH")

@HuwCampbell I get that we can't have backtracking on by default, but you mention that Parsec has try for explicit backtracking, why can't we have the same? I noticed that there's a workaround in #404 but both options there seem kind of unsatisfactory.

@HuwCampbell
Copy link
Collaborator

Despite being a bit awkward the Data.These solution in #404 does work quite well (just don't use the multi suffix option as well).

PRs welcome. I'll have a look as well, but to be honest, I'm not convinced it's a great idea.

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

No branches or pull requests

3 participants