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

doesn't print usage information on --help or -h #2

Closed
jelmer opened this issue Aug 8, 2020 · 3 comments
Closed

doesn't print usage information on --help or -h #2

jelmer opened this issue Aug 8, 2020 · 3 comments

Comments

@jelmer
Copy link
Contributor

jelmer commented Aug 8, 2020

If erbium is run with --help or -h, it appears to ignore the argument (maybe it interprets it as a configuration file path?) and loads as normal rather than printing information about its flags or lack thereof:

 /usr/local/bin/erbium-dhcp  --help
Found new interface IfInfo { name: "lo", addresses: [], lladdr: None, llbroadcast: None, mtu: 65536 }
...
@stappersg
Copy link
Contributor

Upon seening

$ LANG=C sudo target/debug/erbium --help
[2021-01-02T21:31:06Z INFO  erbium] erbium 0.2.6 (e2d2af8)
[2021-01-02T21:31:06Z ERROR erbium] Error: Failed to load config from --help: No such file or directory (os error 2)

makes me think the original reported issue has been improved.

But an usage message isn't yet presented to user.

--- a/src/main.rs
+++ b/src/main.rs
@@ -63,6 +63,11 @@ async fn go() -> Result<(), Error> {
     } else {
         std::path::Path::new(&args[1])
     };
+    if config_file.starts_with("-") {
+        /* User is trying `-h`, `--help` or another option we don't support */
+        error!("Usage: {} <configfile>", args[0].to_string_lossy());
+        return Ok(());
+    }
     let conf = erbium::config::load_config_from_path(config_file)
         .await
         .map_err(|e| Error::ConfigError(config_file.to_path_buf(), e))?;

is something that compiles but never reaches true.

https://doc.rust-lang.org/std/path/struct.Path.html#method.starts_with says

Only considers whole path components to match.

And whole path starts with / ...

In other words: The reported issue got some attention, but still no information printed about flags.

isomer added a commit that referenced this issue Jan 3, 2021
This now outputs usage information if there is more than one argument,
or if the first argument starts with a "-".

This also returns non-exit-0, so that if we add command line arguments
in the future, and somehow someone gets rolled back to an old version of
the binary, then the program will exit unsuccessfully, which hopefully
will make it clearer what happened (esp if someone has some kind of
integration test).

This continues to improve the situation in issue #2, but I don't think
it really resolves it yet.  I've not yet come to a conclusion about what
I want to do with command line arguments.
@isomer
Copy link
Owner

isomer commented Jan 3, 2021

I've not really come to a conclusion about how I want to handle command line arguments just yet, but you're right, this is a pretty nasty quality of life issue, so I've bodged in a solution similar to what you proposed.

I'm going to leave this issue open until I figure out how I want to handle parsing command line arguments properly. Do you think this is sufficient, or have I missed any other obvious cases?

@stappersg
Copy link
Contributor

Do you think this is sufficient

Yes.

And I suggest to close this issue.

@isomer isomer closed this as completed Aug 3, 2022
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