Skip to content

More informative log level error message #516

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.

Sign up for GitHub

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

Jump to bottom
Merged
Schniz merged 2 commits into from
Aug 30, 2021
Merged

More informative log level error message #516

Schniz merged 2 commits into from
Aug 30, 2021

Conversation

@waldyrious
Copy link
Contributor

@waldyrious waldyrious commented Aug 17, 2021

Ideally, the docs should mention these as well, and I'd like to include that change in this PR, but I'm not sure how to insert the list into the current documentation format:

    --log-level <log-level>
        The log level of fnm commands [env: FNM_LOGLEVEL]  [default: info]

Any ideas, @Schniz? Maybe something like this?

    --log-level <log-level>
        The log level of fnm commands [env: FNM_LOGLEVEL]  [supported: quiet, info, error; default: info]

Or this, to use the docopt syntax?

    --log-level (quiet|info|error)
        The log level of fnm commands [env: FNM_LOGLEVEL]  [default: info]

@vercel
Copy link

vercel bot commented Aug 17, 2021 •
edited
Loading

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/schniz/fnm/7Kf4AmvMxUqPYPUWjATkHDvQnXEi
✅ Preview: https://fnm-git-fork-waldyrious-patch-2-schniz.vercel.app

Schniz
Schniz approved these changes Aug 30, 2021
View reviewed changes
src/log_level.rs
"error" => Ok(Self::Error),
loglevel => Err(format!("I don't know the log level of {:?}", loglevel)),
loglevel => Err(format!(
"Unrecognized log level {:?}. Supported levels are 'quiet', 'info' and 'error'.",
Copy link
Owner

@Schniz Schniz Aug 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if there's a way to automatically extract it. But anyway, I'm merging because it is important fix! thank you!

@Schniz Schniz added the PR: Documentation Some docs were added label Aug 30, 2021
@Schniz Schniz merged commit ed2be2c into Schniz:master Aug 30, 2021
@waldyrious waldyrious deleted the patch-2 branch August 30, 2021 11:54
@waldyrious
Copy link
Contributor Author

waldyrious commented Aug 30, 2021

@Schniz any thoughts on how the docs could also include this information, as per my suggestions in the opening comment?

@Schniz
Copy link
Owner

Schniz commented Aug 30, 2021

@waldyrious looks like we can implement LogLevel::possible_values and add https://docs.rs/clap/2.33.3/clap/struct.Arg.html#method.possible_values to reference to it. Then it should show in the docs 😃 I at least hope so

@waldyrious
Copy link
Contributor Author

waldyrious commented Aug 30, 2021

Ok, nice. Is that something you're planning to work on, or are you suggesting that I do it? 'Cause if it's the latter case, I will probably need a bit more guidance :)

@Schniz
Copy link
Owner

Schniz commented Aug 30, 2021

I will file a new issue with explanations what code should go where 😃

@Schniz Schniz mentioned this pull request Aug 30, 2021
Closed
@Schniz
Copy link
Owner

Schniz commented Aug 30, 2021

@waldyrious opened #523

@lucasweng lucasweng mentioned this pull request Sep 17, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Schniz Schniz Schniz approved these changes

Assignees

No one assigned

Labels

PR: Documentation Some docs were added

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@waldyrious @Schniz