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

Feature idea: configuration file alternative to CLI | env arg #225

Open
SamuelMarks opened this issue Jul 19, 2023 · 8 comments
Open

Feature idea: configuration file alternative to CLI | env arg #225

SamuelMarks opened this issue Jul 19, 2023 · 8 comments
Labels
v2-planned Planned for v2

Comments

@SamuelMarks
Copy link

SamuelMarks commented Jul 19, 2023

I added config file support to my go-arg using project but due to #94 I could not determine when to read the config.

My project hacks support with the builtin json package. Specifically here: https://github.com/offscale/postgres-version-manager-go/blob/master/pvm/config_utils.go (with go-arg annotated struct here: config.go)

Any chance we can get support for a configuration file in your project directly?

Thanks

EDIT: Got it working in my latest commit 48cba8 but it was a major hack; here's a taste [from that commit]:

func FieldAndValueWhenNonDefaultValue(configStruct ConfigStruct) map[string]interface{} {
	val := reflect.ValueOf(configStruct)
	fieldToValue := make(map[string]interface{})

	for i := 0; i < val.NumField(); i++ {
		field := val.Type().Field(i)
		defaultTag := field.Tag.Get("default")
		fieldValue := fmt.Sprintf("%v", val.Field(i).Interface())

		if defaultTag != fieldValue && defaultTag != "" {
			fieldToValue[field.Name] = fieldValue
		}
	}

	return fieldToValue
}

Issues with this design include:

  • Doesn't handle when replacing existing config file field explicitly with the original default value
  • Doesn't check environment variables and that env tag
@alexflint
Copy link
Owner

The approach I'm hoping to take to enable this is the one from #197. Need to get that PR up to date with all recent changes to this repo, and finish a few things before publishing it as "github.com/alexflint/go-arg/v2"

@alexflint alexflint added the v2-planned Planned for v2 label Oct 10, 2023
@purpleidea
Copy link

Heya Alex!

I've been doing a little exploration of the command-line parsing libraries in golang... I'm currently using urfave/cli for https://github.com/purpleidea/mgmt/ ... I've noticed their new "v3" API is generics ridden and really clunky. I thought there must be a better way... I found this project...

...And I think the "struct populating" approach is excellent and very close to what I was looking for!

My two main concerns were:

  1. I'm also interested in pulling data from config files as a default... Hence why I'm commenting here in this issue that I found...
  2. Activity in this repo is quieter than I'd like.

I'd actually love to be more involved, but I'm fairly busy working on https://github.com/purpleidea/mgmt/ so, I don't expect too many hours.

In any case, I just figured I'd check in, say that I like what you've been doing, and in case you're still hacking at your monastery, please keep it up!

Cheers

@SamuelMarks -- if you're hacking on adding your config work into core or helping with Alex's v2 approach and need a hand, please LMK.

@SamuelMarks
Copy link
Author

@purpleidea Actually after my #RewriteInRust I decided to #RewriteInC

@alexflint
Copy link
Owner

Thanks for the note @purpleidea. Care to jump on a call some time to say hi / chat? Any chance you're free at 3pm US Eastern time one day this week?

@SamuelMarks would be great to meet you face-to-face at some point, too, though I know you're busy!

@SamuelMarks
Copy link
Author

Good idea! Send me an invite: my name at gmail.com

@purpleidea
Copy link

@alexflint

Thanks for the note @purpleidea. Care to jump on a call some time to say hi / chat? Any chance you're free at 3pm US Eastern time one day this week?

Sure! I'm free either today or tomorrow at that time, otherwise TBD. Send me your preferred meeting thing, I've got signal, jitsi, gmeet or anything that works in a web browser. my username at gmail. Cheers!

@purpleidea
Copy link

I've been thinking about how to pull values from config file into go-arg. Here's what I've come up with.

  1. It's silly to directly support json, yaml, toml, etc, etc, since someone will always be missing what they want.

  2. Instead, we should have a new struct tag. Let's pretend it's called file. You specify the "key" that you want to use.

  3. In the parser, we have the ability for the user to pass in a function of the signature:

func(ctx context string key) (*string, error)

When the library is looking for a value to use, it calls that function with the key from the struct. If it gets a value back, it passes it through the usual parsing that we use for the default tag. If it has an issue pulling a value (io error or whatever) we error. If it doesn't find a value then we return nil, nil.

That way everyone can have their own parser. And of course if we really want to, we can add one or two common ones as a util package.

HTH

@purpleidea
Copy link

In the next little while, I'd really like to be able to do this for https://github.com/purpleidea/mgmt/ ... I've landed all my new CLI patches, and they look great! Mostly here: purpleidea/mgmt@589a5f9

But also to show how other code then simplifies, look at:

purpleidea/mgmt@d537c3d
purpleidea/mgmt@601fcf4
purpleidea/mgmt@51d21b8
purpleidea/mgmt@9527d0d

It's really an improvement! Thanks Alex for this great library!

So instead of bolting config file reading onto mgmt, I'd rather bolt it onto this go-arg lib where it belongs. I would send a patch, but if that would conflict with your v2 patch, then maybe that generates more work for you which I don't want to do if you're not up for it. This patch also wouldn't need a v2 API change, so I think it's safe to add in before it.

I also don't know how receptive to this idea you are and how many cycles you have to review and merge this.

So please let me know how you'd like me to proceed.

Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v2-planned Planned for v2
Projects
None yet
Development

No branches or pull requests

3 participants