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

The error message produced when providing an invalid averaging value could be clearer #133

Closed
d3v-null opened this issue Feb 6, 2023 · 4 comments

Comments

@d3v-null
Copy link
Collaborator

d3v-null commented Feb 6, 2023

This is a placeholder so that I don't forget.

An invalid averaging value was provided through asvo, and the error message just said something like "error parsing cli", but not which specific argument was at fault.

@d3v-null
Copy link
Collaborator Author

d3v-null commented Mar 8, 2023

This was because the Error types were being printed with

    match result {
        Err(e) => {
            eprintln!("preprocessing error: {e}")
        }
    }

which only showed the error type, and not the detailed error info and line number. now that it's printed with debug "{e:?}", the original error message has been restored.

error parsing args: CLIError(InvalidCommandLineArgument { option: "--avg-time-res <RES>", expected: "a multiple of the integration time, 0.5 [s]", received: "0.1" })

@cjordan
Copy link
Collaborator

cjordan commented Mar 8, 2023

I think printing the debug representation of the error is not what you want. It might make this particular error look OK, but most other errors will not be human readable (e.g. 35e4cba). This is why it's called a debug rep - it's meant to be for programmers, not humans.

I therefore think that it would be best to change the Display method of the errors you're interested in. Alternatively, you could match on specific errors in your code block above and emit additional details there.

@d3v-null
Copy link
Collaborator Author

d3v-null commented Mar 9, 2023

I agree that this would be a better solution long term, but I think including the error messages with line numbers provides a substantial improvement over just the name of the error for now. How about I open up a new ticket for this.

@d3v-null
Copy link
Collaborator Author

error looks like this now

error parsing args: Invalid Command Line Argument: option --avg-time-res <RES> expected a multiple of the integration time, 2 [s] but received 0.9999

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

2 participants