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

Turn off optparse backtracking to show help for the current command #1278

Merged
merged 4 commits into from
Aug 31, 2024

Conversation

fsoikin
Copy link
Collaborator

@fsoikin fsoikin commented Aug 30, 2024

Description of the change

Fixes #1146

This is kind of by design for optparse, even though it ends up being nonsensical.

This happens for commands that don't have any non-optional options. When the parser parses such command and encounters a mistyped option, the command parsing has already succeeded. It doesn't need any options, so it succeeds as soon as its name is parsed. And then the parser tries to parse remaining options as "global". They still fail, but by that time we're already back in "global" context. The command parsing is over. So optparse shows global help.

This behavior - trying to parse remaining options as "global" - is called "backtracking", and it's governed by an option. Turning backtracking off has a subtle, but not insignificant effect. It manifests for programs that have truly "global" options as well as commands.

Suppose you have a parser like this:

rootParser = fromRecord 
  { command: commandParser
  , anotherOption: strOption (long "foo")
  }

So you can run your program as prog --foo bar command, which will result in { command: ..., anotherOption: "bar" }.
With backtracking enabled, you can also run prog command --foo bar. The --foo option will be parsed after the command.
But with backtracking disabled you can't. Supplying --foo after command will be an error, and you will be shown help for the command.

Fortunately, Spago's parser is setup in a way that doesn't have any global options. What we call GlobalArgs are actually options of every individual command from optparse's point of view. And this means that we don't care for backtracking, so it can be safely turned off. Which is what this PR does.

Checklist:

  • Added the change to the "Unreleased" section of the changelog
  • [ ] Added some example of the new feature to the README
  • Added a test for the contribution (if applicable)

@f-f
Copy link
Member

f-f commented Aug 31, 2024

Fortunately, Spago's parser is setup in a way that doesn't have any global options. What we call GlobalArgs are actually options of every individual command from optparse's point of view. And this means that we don't care for backtracking, so it can be safely turned off. Which is what this PR does.

I always had an issue with all of this - the context is:

  • in the Haskell implementation, we had "real global options" - they were printed out nicely and separately from the command options, but needed to be parsed before the command, which was very confusing as it meant you couldn't e.g. call spago build -v
  • so in this PS implementation I structured the parser so that there are no global options, so you can put flags wherever. But the formatting of the help message is very confusing

So I'd like to end up in a place where:

  • we can use global options in any position of the command line parser
  • and we also get a help message which separates the command options from the global options

The concrete scenarios would be:

  1. we disable the backtracking but find a way to separate the global options in the help output
  2. or we keep the backtracking, go back to global options, and find a way to enable their parsing anywhere

I have the suspicion that none of these scenarios is possible 🥲 But thought I'd bring this up with you in case you have any idea how to go at it

@fsoikin
Copy link
Collaborator Author

fsoikin commented Aug 31, 2024

So in this PS implementation I structured the parser so that there are no global options, so you can put flags wherever

That's not how it works currently. With the current parser structure you can't put global args wherever - only after the command name. So you can do spago build -q, but you can't do spago -q build.

we keep the backtracking, go back to global options, and find a way to enable their parsing anywhere

We don't have to find a way. If we go back to global options and keep backtracking, the global options will be parseable anywhere. That's what backtracking is for. It enables parsing of global options after the command name.

But then issue #1146 will remain. The only way to fix #1146 is to disable backtracking. Backtracking is what makes the parser "forget" the current command after it's been parsed satisfactorily.

But thought I'd bring this up with you in case you have any idea how to go at it

I think this might be possible by hijacking how the help is printed. Not sure though.

Also, these limitations ultimately stem from the particular way optparse is implemented. I think it is needlessly waaaaay too complicated. I haven't actually tried yet, but I'm pretty sure option parsing can be done way simpler and straightforward. From what I can see, optparse is trying to do "regular" parsing, similar to how you'd parse a program source code, but CLI options are not sequential by their very nature, so the parser would be better off with random access model.

@f-f
Copy link
Member

f-f commented Aug 31, 2024

Good thread, thanks for the details. You're right that we need this to fix #1146, so I'll merge this now and we'll eventually figure out the rest

bin/src/Main.purs Show resolved Hide resolved
@f-f f-f merged commit 1f9ecda into purescript:master Aug 31, 2024
5 checks passed
@fsoikin fsoikin deleted the command-help branch August 31, 2024 21:00
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.

Invalid option for a command should give the help for that command instead of the general help
2 participants