Implement better parsing of leftover arguments for run commands #271
Conversation
soenkehahn
force-pushed
the
sh/command-line-flags
branch
from
c9ac6ec to
0b6f73a
Compare
soenkehahn
force-pushed
the
sh/command-line-flags
branch
from
0b6f73a to
bf7f64b
Compare
soenkehahn
changed the title
Executables
| -- arguments. | ||
| handleParseResult $ execParserPure parserPrefs parserInfo allArgs | ||
| Left err -> | ||
| handleParseResult $ Failure $ parserFailure parserPrefs parserInfo err contexts |
There was a problem hiding this comment.
This has gotten much more complicated than I thought it would be. But I haven't found an easier way to do this with optparse-applicative.
| @@ -74,6 +74,32 @@ spec = around_ (hSilence [stderr]) $ do | |||
| command <- testWithGarnTs ["run", "project", "more", "args"] ("project" ~> targetConfig) | |||
| command `shouldBe` Run (CommandOptions "project" targetConfig) ["more", "args"] | |||
|
|
|||
| it "parses run commands with additional flags (starting with `-`)" $ do | |||
There was a problem hiding this comment.
So you need — only for —help to be parsed by the inner command? That feels unintuitive to me. Should we maybe require it for all arguments? I think not needing it made sense when we didn’t have the past command (eg “foo” in “garn build foo”) be optional, but doesn’t anymore.
There was a problem hiding this comment.
Passing flags (rather than string arguments) to the subprocess is already possible with --; as in garn run foo bar -- --baz. It's not a great system, but I think requiring -- only when run happens to have a conflicting option isn't great either (both because it's not clear from the command alone who is getting the option, and because of backwards compatibility).
I can't seem to manage to get any arguments that start with More importantly: I think this isn't super high priority. Maybe we should have a very simple PR that changes this to always require |
|
For whatever reason, I need to pass Parsing |
No description provided.