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

Switch flags package to one that allows "not set" or nil as default value #44

Open
atc0005 opened this issue May 2, 2020 · 5 comments
Assignees
Labels
Milestone

Comments

@atc0005
Copy link
Owner

atc0005 commented May 2, 2020

See GH-10 and GH-43 for background details.

In short, since this project allows for specifying options via config file (TOML) and flags, we need a way to determine when a flag was not specified so that we can fallback to the config file (if applicable) and only then the application default.

If we go with alexflint/go-arg we have the atc0005/elbow project to use as a reference; instead of using that project's handling of config values, I'd like to keep the current Getter behavior of this project. The current logic used in this project allows for per-setting overrides vs the global merge behavior that I implemented for atc0005/elbow.

If possible I'd like to keep all existing flags with the possible exception of the one for ignoring errors (as discussed in GH-43).

@atc0005 atc0005 added enhancement New feature or request dependencies labels May 2, 2020
@atc0005 atc0005 added this to the Future milestone May 2, 2020
@atc0005 atc0005 self-assigned this May 2, 2020
@atc0005
Copy link
Owner Author

atc0005 commented Jul 28, 2020

From looking over the existing documentation and diving into the code, the alexflint/go-arg library doesn't support explicitly specifying both short and long form flags. You can have one or the other, but not both unless you specify the short form and allow the long form to be automatically created based on the struct field name.

Here are some example long form names currently defined:

  • config-file
  • dns-server
  • ignore-dns-errors

Looking over the docs, I don't think that the dash-separated flag names are automatically supported/mapped.

@atc0005
Copy link
Owner Author

atc0005 commented Jul 28, 2020

I've opened an issue in the alexflint/go-arg repo to see if there is another way I've not spotted yet.

For now, it's probably less invasive to workaround the issue(s) noted in GH-10 by implementing the changes for GH-43.

@atc0005
Copy link
Owner Author

atc0005 commented Sep 14, 2020

From looking over the existing documentation and diving into the code, the alexflint/go-arg library doesn't support explicitly specifying both short and long form flags.

I heard back some time ago and after brief testing confirmed that both short and long form flags are supported.

@atc0005 atc0005 removed the waiting label Sep 14, 2020
@atc0005
Copy link
Owner Author

atc0005 commented Sep 16, 2020

In short, since this project allows for specifying options via config file (TOML) and flags, we need a way to determine when a flag was not specified so that we can fallback to the config file (if applicable) and only then the application default.

While working on a new project I reviewed the configuration handling for this one, in particular, the log format and log level handling.

Currently, if the user does not specify a log format, the defaultLogFormat value is applied. In this case, text. When the LogFormat() getter method is called, it first checks the c.cliConfig.LogFormat field to confirm it is non-empty. When it passes that check (as it always will if the user doesn't provide a value), then the default value is used, regardless of any config file setting. If the user specifies a value, then that value is used.

Ultimately, there are workarounds (such as using something like undefined as the default flag value), but I'm coming to the conclusion that if I want to use flags only, then the standard library package will do, otherwise I should continue to use alexflint/go-arg so that I can continue to use pointer-based config struct fields to indicate an absence of a value.

@atc0005 atc0005 pinned this issue Sep 16, 2020
@atc0005
Copy link
Owner Author

atc0005 commented Sep 16, 2020

On a related note, see GH-108. That functionality should be reviewed/updated at the same time that I make required changes to resolve this issue.

@atc0005 atc0005 unpinned this issue Oct 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant