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

When parsing an argument from a file or env var, the error should mention the source #1167

Closed
3 tasks done
mostynb opened this issue Jul 30, 2020 · 10 comments
Closed
3 tasks done
Labels
area/v2 relates to / is being considered for v2 kind/feature describes a code enhancement / feature request status/triage maintainers still need to look into this
Milestone

Comments

@mostynb
Copy link
Contributor

mostynb commented Jul 30, 2020

Checklist

  • Are you running the latest v2 release? The list of releases is here.
  • Did you check the manual for your release? The v2 manual is here
  • Did you perform a search about this feature? Here's the Github guide about searching.

What problem does this solve?

I have had a few bug reports from users that run a binary that uses urfave/cli in orchestrated/containerised environments which automatically set some environment variables with default values, and those environment variable names match some of those used by cli in my application. The application exits with a "could not parse ..." error, but the user confused because they did not specify that value anywhere themselves.

Solution description

When a parse error fails, we should state where the value was obtained (since multiple environment variables and files are allowed as the source of a single flag).

Describe alternatives you've considered

We could remove the possibility of allowing multiple sources (environment variables/files) for a given flag, but that would break existing users.

@mostynb mostynb added area/v2 relates to / is being considered for v2 status/triage maintainers still need to look into this labels Jul 30, 2020
mostynb added a commit to mostynb/cli that referenced this issue Jul 30, 2020
If you allow a flag to be set from environment variables or files and
a parse error occurs from one of them, it is very useful for the error
message to mention where the value came from.

Without this, it can be difficult to notice an error caused by an
unexpected environment variable being set.

Implements urfave#1167.
@stale
Copy link

stale bot commented Oct 28, 2020

This issue or PR has been automatically marked as stale because it has not had recent activity. Please add a comment bumping this if you're still interested in it's resolution! Thanks for your help, please let us know if you need anything else.

@stale stale bot added the status/stale stale due to the age of it's last update label Oct 28, 2020
@mostynb
Copy link
Contributor Author

mostynb commented Oct 28, 2020

Bump.

@stale
Copy link

stale bot commented Oct 28, 2020

This issue or PR has been bumped and is no longer marked as stale! Feel free to bump it again in the future, if it's still relevant.

@stale stale bot removed the status/stale stale due to the age of it's last update label Oct 28, 2020
mostynb added a commit to mostynb/cli that referenced this issue Nov 3, 2020
If you allow a flag to be set from environment variables or files and
a parse error occurs from one of them, it is very useful for the error
message to mention where the value came from.

Without this, it can be difficult to notice an error caused by an
unexpected environment variable being set.

Implements urfave#1167.
@stale
Copy link

stale bot commented Jan 30, 2021

This issue or PR has been automatically marked as stale because it has not had recent activity. Please add a comment bumping this if you're still interested in it's resolution! Thanks for your help, please let us know if you need anything else.

@stale stale bot added the status/stale stale due to the age of it's last update label Jan 30, 2021
@mostynb
Copy link
Contributor Author

mostynb commented Jan 30, 2021

Bump.

@stale
Copy link

stale bot commented Jan 30, 2021

This issue or PR has been bumped and is no longer marked as stale! Feel free to bump it again in the future, if it's still relevant.

@stale stale bot removed the status/stale stale due to the age of it's last update label Jan 30, 2021
@mostynb
Copy link
Contributor Author

mostynb commented Jan 31, 2021

Bump.

@stale
Copy link

stale bot commented Jun 2, 2021

This issue or PR has been automatically marked as stale because it has not had recent activity. Please add a comment bumping this if you're still interested in it's resolution! Thanks for your help, please let us know if you need anything else.

@stale stale bot added the status/stale stale due to the age of it's last update label Jun 2, 2021
@stale
Copy link

stale bot commented Jul 4, 2021

Closing this as it has become stale.

@stale stale bot closed this as completed Jul 4, 2021
@meatballhat meatballhat reopened this Apr 22, 2022
@meatballhat meatballhat removed the status/stale stale due to the age of it's last update label Apr 22, 2022
@meatballhat meatballhat added this to the Release 2.5.0 milestone Apr 23, 2022
@meatballhat meatballhat added the kind/feature describes a code enhancement / feature request label Apr 23, 2022
@meatballhat meatballhat changed the title v2 feature: when parsing an argument from a file or env var, the error should mention the source When parsing an argument from a file or env var, the error should mention the source Apr 23, 2022
@mostynb
Copy link
Contributor Author

mostynb commented May 22, 2022

#1168 was merged (thanks!) - closing this.

@mostynb mostynb closed this as completed May 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/v2 relates to / is being considered for v2 kind/feature describes a code enhancement / feature request status/triage maintainers still need to look into this
Projects
None yet
Development

No branches or pull requests

2 participants