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

ParseWithOptions: add the ability to override defaultTypeParsers #272

Merged

Conversation

Daniel538
Copy link
Contributor

@Daniel538 Daniel538 commented Jul 17, 2023

This small fix/feature adds the ability to override default ParserFunc in Options{FuncMap} for ParseWithOptions

When it can be useful:
When for some reason you can't use default ParseDuration format but want to have a time.Duration
Like today I ran into a small but very annoying problem
I had some executions like INTERVAL=1 ./script.sh and wanted to override time.Duration parser but it was not possible.
We may also want to override the URL parser or some other future defaultTypeParsers

@codecov
Copy link

codecov bot commented Jul 20, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (4f6bb22) 100.00% compared to head (15384ec) 100.00%.

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #272   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            3         3           
  Lines          430       431    +1     
=========================================
+ Hits           430       431    +1     
Impacted Files Coverage Δ
env.go 100.00% <100.00%> (ø)

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

@caarlos0
Copy link
Owner

fwiw you can already do this by creating a type wrapping the intended type, and implementing a custom wrapper for it, e.g.:

type MyDuration time.Duration

func (t *MyDuration) UnmarshalText(text []byte) error {
	sec, err := strconv.ParseInt(string(text), 10, 64)
	if err == nil {
		*t = MyDuration(time.Duration(sec) * time.Second)
		return nil
	}
	tt, err := time.ParseDuration(string(text))
	*t = MyDuration(tt)
	return err
}

func TestCustomDurationParser(t *testing.T) {
	type config struct {
		Dur1 MyDuration `env:"DUR_1"`
		Dur2 MyDuration `env:"DUR_2"`
	}

	t.Setenv("DUR_1", "20m")
	t.Setenv("DUR_2", "10")

	var cfg config
	isNoErr(t, Parse(&cfg))
	isEqual(t, time.Duration(cfg.Dur1), 20*time.Minute)
	isEqual(t, time.Duration(cfg.Dur2), 10*time.Second)
}

@Daniel538
Copy link
Contributor Author

Daniel538 commented Jul 20, 2023

fwiw you can already do this by creating a type wrapping the intended type, and implementing a custom wrapper for it, e.g.:

type MyDuration time.Duration

func (t *MyDuration) UnmarshalText(text []byte) error {
	sec, err := strconv.ParseInt(string(text), 10, 64)
	if err == nil {
		*t = MyDuration(time.Duration(sec) * time.Second)
		return nil
	}
	tt, err := time.ParseDuration(string(text))
	*t = MyDuration(tt)
	return err
}

func TestCustomDurationParser(t *testing.T) {
	type config struct {
		Dur1 MyDuration `env:"DUR_1"`
		Dur2 MyDuration `env:"DUR_2"`
	}

	t.Setenv("DUR_1", "20m")
	t.Setenv("DUR_2", "10")

	var cfg config
	isNoErr(t, Parse(&cfg))
	isEqual(t, time.Duration(cfg.Dur1), 20*time.Minute)
	isEqual(t, time.Duration(cfg.Dur2), 10*time.Second)
}

Yes I can
But if I use some external function with time.Duration signature it requires me to always do something like

type MyDuration time.Duration

func (md MyDuration) ToDuration() time.Duration {
	return time.Duration(md)
}

func doSomething(duration time.Duration) {
	fmt.Println(duration)
}

doSomething(value.ToDuration())

or

doSomething(time.Duration(value))

Or parse my own type separately from the main struct and use ToDuration() and store in the struct, which is a bit awkward in my opinion

And if I want to use it in combination with flag I have to implement String() and Set() methods in MyDuration for flag.Var() instead of using flag.DurationVar() (flag.DurationVar works with numbers without any customization)

And BTW the defaultTypeParsers from the var name hints to me that I should be able to use something custom if I want to :)

@caarlos0
Copy link
Owner

caarlos0 commented Aug 7, 2023

yeah, you right.

all right, let's merge this, but it'll be a breaking change.

@caarlos0 caarlos0 merged commit 62b4ae1 into caarlos0:main Aug 7, 2023
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants