Update to clap 4 and improve subcommand typo error messages.

[bhansconnect]

context: https://roc.zulipchat.com/#narrow/stream/231634-beginners/topic/CLI.20Error.20Message/near/356548857

Essentially, as a prereq for better error messages when a user mistypes a subcommand name, we need to first upgrade to clap 4. That will fix a bug in parsing related to requiring --. Currently when you mistype a subcommand name, parsing of the args fails. This means, we don't get any chance to give a better error message.

So:

1. upgrade to clap 4
2. update to the new parsing without requiring --
3. when someone tries to run a file and it isn't found, check if the name is a typo of a sub command.
4. give a nicer error.

[dlsmith]

I'd like to work on this.

[ayazhafiz]

Assigned to @dlsmith !

[dlsmith]

I've opened a draft PR for the clap 4 upgrade here.

[dlsmith]

I have a draft PR here for a change to roc [FILE] parsing and error messaging. I kept the change pretty minimal so far, but happy to adjust:

• -- is still required for roc <COMMAND> .... This seems fairly standard practice in CLIs and avoids parsing ambiguity. My understanding is that the primary motivation for removing -- from roc [FILE] is to enable better messaging.
• When roc [FILE] errors (e.g., roc foromat *), the expected file not found error is just augmented with a hint about subcommand typos (see below). It's not doing any kind of approximate "Did you mean..." matching against available subcommands.
• When a command is provided, it's required up front to avoid ambiguity (e.g., roc helloWorld.roc test).

One related question I have is whether args should even be allowed for roc [FILE], if it's just meant to support #!. It would be nice to avoid the inconsistency with subcommand --. And if you're invoking roc directly, it seems better to be explicit about subcommands, in terms of readability and reasoning about behavior.

$ roc help
Run the given .roc file, if there are no compilation errors.
You can use one of the SUBCOMMANDS below to do something else!

Usage: roc [OPTIONS] [ROC_FILE] [ARGS_FOR_APP]...
       roc <COMMAND>

$ roc help test
Run all top-level `expect`s in a main module and any modules it imports

Usage: roc test [OPTIONS] [ROC_FILE] [-- [ARGS_FOR_APP]...]

$ roc foromat *

This file was not found: /Users/dlsmith/src/roc/foromat

Did you misspell a subcommand?

You can run `roc help` to see the list of available subcommands and for more information on how to provide a .roc file.

[dlsmith]

@bhansconnect, any thoughts?

[bhansconnect]

Sorry I missed this.
For -- assuming it isn't hard to do so, it may be reasonable to follow something like what cargo does:

• cargo run helloWorld.roc -> roc helloWorld.roc
• cargo run format helloWorld.roc -> roc format helloWorld.roc
• cargo run helloWorld.roc --optimize -> roc helloWorld.roc --optimize
• cargo run --optimize helloWorld.roc -> error, --optimize was not expected in this context maybe try -- --optimize

Not sure how hard that would be to support, but I think it is a pretty intuitive set of functionality if it is possible to match with clap. That said, totally fine to just require the -- as well. I think either is fine if you are passing flags.

For the subcommand. I think long term the name matching would be nice (I know we already do this in the compiler for record field names, I wonder if we could just hook into that logic), but I think just adding that to the error message is also enough to clarify.

100% for requiring the command up front.

[dlsmith]

Thanks for the suggestion to look into record field handling for matching logic. I added it to the subcommand message. It seemed simple enough to not be worth factoring out and sharing.

For --, I'm not totally clear on what you meant by following cargo. There's no analog to roc [FILENAME] (everything is behind a subcommand) and -- is required to distinguish program args.

But I believe the current behavior should match the examples you gave, with the exception of the last. cargo run consumes --optimize, so we don't get a chance to provide our own error messaging. However, cargo provides the error we'd like, suggesting -- --optimize.

There is however a potentially confusing inconsistency, depending on whether roc recognizes a flag (see --optimize vs. --xyz).

• cargo run --optimize helloWorld.roc: cargo run consumes --optimize
• cargo run helloWorld -- --optimize: helloWorld.roc consumes --optimize
• cargo run helloWorld.roc --optimize: roc consumes --optimize
• cargo run helloWorld.roc --xyz: helloWorld.roc consumes --xyz

I don't know of a way to get around this inconsistency with clap.

[bhansconnect]

@rtfeldman, any thoughts here? Specifically on roc vs the app getting flags?

I see it as 3 core options (not 100% sure the feasibility of each):

1. roc gets the flags before the app name or --: roc run --optimize helloWorld.roc, but not the ones after the app name: roc run helloWorld.roc --optimize. In this case, -- should never be needed. The app name is essentially the --. If the name is an implicit main.roc, then the -- would be needed before args.
2. roc gets all flags before --. The app name must be before the --. So both of these are equivalent: roc run --optimize helloWorld.roc, roc run helloWorld.roc --optimize. To send --optimize to the app, you must do: roc run helloWorld.roc -- --optimize
3. roc gets all flags before --, but -- can come before the app name (I am not sure clap can support it). So both of these are equivalent: roc run --optimize helloWorld.roc, roc run helloWorld.roc --optimize. To send --optimize to the app, you can do any of the follow: roc run helloWorld.roc -- --optimize, roc run -- helloWorld.roc --optimize, roc run -- --optimize helloWorld.roc.

To my understanding, cargo is slightly different than any of the suggestions because you never specify the app to run. That is implicit from the cargo.toml file. So for cargo, it takes all args before a file name or a --: cargo run --cargoArg fileName.txt --appArg or cargo run --cargoArg -- --appArg.

EDIT: I don't think 3 works in general. Is roc run -- helloWorld.roc running helloWorld.roc, or is it running main.roc and passing in helloWorld.roc

[rtfeldman]

One related question I have is whether args should even be allowed for roc [FILE], if it's just meant to support #!. It would be nice to avoid the inconsistency with subcommand --. And if you're invoking roc directly, it seems better to be explicit about subcommands, in terms of readability and reasoning about behavior.

Yeah, I think this is what what we should do - that is, it's either roc [FILE] or roc [subcommand] […] but if you're doing roc [FILE], you just get one argument (the file to run) and everything else is ignored.

In fact, I'd go as far as to special-case that to make scripts run even faster, e.g.

match std::env::args().first() {
    Some(arg) if arg.ends_with(".roc") => {
        // don't even invoke clap, just immediately do a script build
    }
    _ => {
        // do clap stuff
    }
}

The reason I think this is what we should do is that:

• Some UNIX env implementations do not support passing through arguments; #!/usr/bin/env roc is literally the only way to make them work, and they will not pass through anything you put after roc there (e.g. we can't require a subcommand like #!/usr/bin/env roc run and we can't pass through args like #!/usr/bin/env roc --optimize)
• Given this external design constraint, the only way to support #!/usr/bin/env roc functionality on these systems is to have the roc CLI receiving exactly 1 argument (of a .roc file) work this way. We can't reuse an existing subcommand.
• Given that the only purpose of roc myrocfile.roc is to support #!/usr/bin/env roc, if we add support for other things, then people will use those and perhaps mistakenly think that will work consistently in #!/usr/bin/env roc (which it won't!) - so it seems better to focus the roc myrocfile.roc use case on /usr/bin/env and then say "if you want any behavior other than that, use roc run and pass it arguments to configure it the way you want it tor un."

[bhansconnect]

I think we should at least give warnings if someone passes other args to roc someFile.roc. Otherwise it would be quite confusing. Frankly even doing that feels like bad UX. Why can't I pass an arg to roc. That is what I think most people type on the command line all the time. It is shorter and more convenient.

On top of that, you can send args to env: #!/usr/bin/env -S roc --time works just fine on all of my systems.

[rtfeldman]

hmm interesting, maybe this restriction doesn't hold anymore? I asked chatGPT and it said:

For example, on FreeBSD older than 9.1 and NetBSD older than 6.0, the env command does not support passing options to the command being invoked. These systems have since been updated to support this feature.

Also apparently POSIX requires supporting args being passed through, so if a particular env doesn't support it, that's a bug.

[rtfeldman]

Okay, I think there is a big enough design question here that it shouldn't block the upgrade to clap 4.

I think we should:

• Do whatever's easiest in terms of making this PR able to land, with the understanding that we'll follow up with a future PR to change the behavior once we figure out a design
• Open an #ideas thread on Zulip to discuss design ideas around this in light of the discovery that all the modern env implementations we care about actually can take parameters

[bhansconnect]

Clap 4 is already landed. This is just the arg update now.

[bhansconnect]

I guess for, we can keep all the args the same and just add the better error message for roc foromat .... Letting the user know they might have meant roc format ...

[rtfeldman]

Clap 4 is already landed. This is just the arg update now.

Totally thought this was the PR and didn't realize it was an issue. 😆

[dlsmith]

Thanks for weighing in, @bhansconnect and @rtfeldman!

@bhansconnect, #5425 is ready for review.