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

Usage string should not use main when calling a script #7126

Open
jmgilman opened this issue Nov 13, 2022 · 10 comments
Open

Usage string should not use main when calling a script #7126

jmgilman opened this issue Nov 13, 2022 · 10 comments
Labels
polish this problem makes nu feel unpolished

Comments

@jmgilman
Copy link

jmgilman commented Nov 13, 2022

Describe the bug

When executing a script and passing the help flag (nu myscript.nu --help), the usage string utilizes main as the executable.

$ nu build.nu --help

Builds an image

Usage:
  > main <image>    # <-----------------  Uses main

Flags:
  -h, --help - Display the help message for this command

Parameters:
  image <String>: The name of the image to build

How to reproduce

  1. Create a script using a main function
  2. Run nu myscript.nu --help

Expected behavior

For the uninitiated, the usage of main as the executable can be confusing. It would be more natural to include the name of the script:

$ nu build.nu --help

Builds an image

Usage:
  > build.nu <image>    # <-----------------  Use script name by default

Flags:
  -h, --help - Display the help message for this command

Parameters:
  image <String>: The name of the image to build

Screenshots

No response

Configuration

N/A

Additional context

No response

@sholderbach sholderbach added the polish this problem makes nu feel unpolished label Nov 13, 2022
@rgwood
Copy link
Contributor

rgwood commented Nov 17, 2022

Hmm, I'm a little confused by your expected behaviour.

I can see how the current output is confusing, but given that you're running it like nu build.nu, suggesting that the user run it like build <image> is not right either, no?

What about nu build.nu?

$ nu build.nu --help

Builds an image

Usage:
  > nu build.nu <image>
...

Or even nu <script path>?

$ nu build.nu --help

Builds an image

Usage:
  > nu <script path> <image>
...

@jmgilman
Copy link
Author

jmgilman commented Nov 17, 2022

Sorry, that was an oversight on my part. Yes, it should be build.nu in my example case. Whether or not the script path is included is a good question -- I would posit in most cases that autogenerated help text does not include the full path.

@rgwood
Copy link
Contributor

rgwood commented Nov 17, 2022

Whether or not the script path is included is a good question

Oh, I was thinking of literally outputting > nu <script path> <image> - not replacing <script path> with the path to the script.

I see that you've updated your expected behaviour to:

  > build.nu <image>

I still think that's a little off, because the user can't actually run the script like that. I'm thinking it would make more sense to suggest nu build.nu instead of just build.nu, what do you think?

@jmgilman
Copy link
Author

Looking at some cursory SO answers, I'm fairly certain the UNIX standard is to use just the base name of the script in the usage section.

I still think that's a little off, because the user can't actually run the script like that. I'm thinking it would make more sense to suggest nu build.nu instead of just build.nu, what do you think?

It depends on the contents of the script. I would expect in most cases that a shebang would be provided, in which case it's more than possible to execute the script without invoking nu directly.

@rgwood
Copy link
Contributor

rgwood commented Nov 17, 2022

Shebangs are only available on *nix OSs and we don't have a guarantee that the script contains one (although I guess we could check).

@jmgilman
Copy link
Author

jmgilman commented Nov 17, 2022

True. I'll leave it up to the core team to decide on what looks better. My experience has been the usage line contains only the base name of the script and/or binary.

I suppose a (potentially more difficult) alternative is to let the script author control what gets put there. They already have some level of control using comments.

@rgwood
Copy link
Contributor

rgwood commented Nov 17, 2022

On a related note, we probably want to clean up the error message when giving a script the wrong parameters.
image

@rgwood
Copy link
Contributor

rgwood commented Nov 17, 2022

Spent a bit of time looking at this and it's sadly not quite trivial to fix. At the point where the help content is generated, the engine is not aware of the script file - it's just handling a call to a regular command named main, as far as it's concerned.

Might come back to this another time, but if someone else wants to pick it up here are the relevant bits:

if working_set.find_decl(b"main", &Type::Any).is_some() {
let args = format!("main {}", args.join(" "));
if !eval_source(
engine_state,
stack,
&file,
&path,
PipelineData::new(Span::new(0, 0)),
) {
std::process::exit(1);
}
if !eval_source(engine_state, stack, args.as_bytes(), "<commandline>", input) {
std::process::exit(1);
}

if !decl.is_known_external() && call.named_iter().any(|(flag, _, _)| flag.item == "help") {
let mut signature = decl.signature();
signature.usage = decl.usage().to_string();
signature.extra_usage = decl.extra_usage().to_string();
let full_help = get_full_help(&signature, &decl.examples(), engine_state, caller_stack);

@jmgilman
Copy link
Author

Yes, that was the same conclusion I had drawn earlier. Piping the context of the script name all the way to where the help is generated is not a straightforward task, unfortunately.

@jmgilman
Copy link
Author

On a related note, we probably want to clean up the error message when giving a script the wrong parameters. image

Yes, I agree with this, I'm not usually a fan of spilling out the guts of the script because of a missing parameter. What parameter is missing and maybe displaying the usage line again should be sufficient.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
polish this problem makes nu feel unpolished
Projects
None yet
Development

No branches or pull requests

3 participants