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

Prototype v2 #197

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open

Prototype v2 #197

wants to merge 19 commits into from

Conversation

alexflint
Copy link
Owner

This PR will introduce a v2 of go-arg, to be imported with

import "github.com/alexflint/go-arg/v2

It should be backwards with v1 for about 95% of users. If you only used arg.MustParse then you can use v2 without modifications to your code. The main changes are:

  • add Parser.ProcessCommandLine, Parser.ProcessEnvironment, Parser.ProcessDefaults so that you can control the order in which things are loaded
  • add Parser.OverwriteWithCommandLine, Parser.OverwriteWithEnvironment, Parser.OverwriteWithDefaults for further control over which arguments get overwritten when by which other arguments
  • Drop the Config struct in favor of a list of ParserOptions to NewParser

Features still to be implemented on this branch:

  • Bash autocomplete support
  • Custom templates for help and usage text

For users and contributors to go-arg: please leave your feedback and suggestions on this PR.

//
// ./example --iter=1 --debug // debug is a boolean flag so its value is set to true
// ./example -iter 1 // debug defaults to its zero value (false)
// ./example --debug=true // iter defaults to its zero value (zero)
Copy link
Contributor

Choose a reason for hiding this comment

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

You've got the comments after the example swapped around. Also --iter on the line above?

@codecov-commenter
Copy link

codecov-commenter commented Oct 29, 2022

Codecov Report

Base: 100.00% // Head: 100.00% // No change to project coverage 👍

Coverage data is based on head (c046f49) compared to base (11f9b62).
Patch has no changes to coverable lines.

Additional details and impacted files
@@            Coverage Diff            @@
##            master      #197   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            5         5           
  Lines          688       690    +2     
=========================================
+ Hits           688       690    +2     
Impacted Files Coverage Δ
parse.go 100.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

go.work Outdated
@@ -0,0 +1,2 @@
use .
use ./v2
Copy link
Contributor

Choose a reason for hiding this comment

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

go.work is meant to locally modify your workspace. It shouldn't be committed and shouldn't be part of a release. It's generally recommended to add it to .gitignore.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Thank you!

@GibMeMyPacket
Copy link

Hope you make #146 as soon as possible.
I will migrate to this prototype immediately right after that.

@suntong
Copy link

suntong commented Jun 7, 2023

Last commit was 8 months ago, and it is now 19 commits ahead, 20 commits behind master.

IE, I see delaying would be more harmful than helpful.

Maybe we can have v2 now and have the remaining features added later?

@purpleidea
Copy link
Contributor

I took a look, and one design question baffled me:

We have cli values, env values, default values, and config-file values. I would expect that the precedence order be:

default value
config-file value
env value
cli value

IOW, default is what's used when nothing else is given.
Which can be overridden by a config-file value.
Which can be overridden by an ENV value.
Which can be overridden by the cli flag.

I am probably just naive here, but I can't think of a scenario when this is not the desired precedence order. As a result, I believe that allowing the user to set this order makes our API more complicated. Just my personal opinion, but I am happy to hear alternate viewpoints.

@suntong
Copy link

suntong commented Mar 6, 2024

Agree 🤦‍♂️, totally agree with your opinion @purpleidea 👍

@jweyrich
Copy link

jweyrich commented Jul 12, 2024

I'm also on the waitlist for the multiple "providers".
And agree with @purpleidea rationale above.
In the future it would be cool to see something like the chained providers in the AWS SDK for Java, which allows you to pick your provider (env, file, etc), or compose a chain of them in a specific order. More in https://docs.aws.amazon.com/sdk-for-java/latest/developer-guide/credentials-specify.html and https://github.com/aws/aws-sdk-java/blob/master/aws-java-sdk-core/src/main/java/com/amazonaws/auth/DefaultAWSCredentialsProviderChain.java

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

Successfully merging this pull request may close these issues.

7 participants