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

PR #673 broke my code. The nested structures don't get the default values correctly #764

Closed
flw-cn opened this issue Sep 15, 2019 · 4 comments · Fixed by #771
Closed

PR #673 broke my code. The nested structures don't get the default values correctly #764

flw-cn opened this issue Sep 15, 2019 · 4 comments · Fixed by #771

Comments

@flw-cn
Copy link

flw-cn commented Sep 15, 2019

I wrote a short example to illustrate the problem.

package main

import (
	"fmt"

	"github.com/spf13/cobra"
	"github.com/spf13/viper"
)

type Config struct {
	Foo struct {
		Bar string
	}
}

func main() {
	config := &Config{}
	cmdRoot := &cobra.Command{
		Run: func(*cobra.Command, []string) {
			viper.Unmarshal(config)
		},
	}
	flags := cmdRoot.PersistentFlags()
	flags.String("foo.bar", "hello-world", "test default value fo foo.bar")
	viper.BindPFlags(flags)
	cmdRoot.Execute()
	fmt.Printf("foo.bar: [%v]\n", config.Foo.Bar)
}

The above code I expected to get the output is:

foo.bar: [hello-world]

This has worked very well in the past. But since PR #673, it can't work properly, it will output:

foo.bar: []

I checked the contents of the PR #673 , I can't fully understand the changes. But I believe the changes blocked my previous usage.

How should I modify my code to fit the new version of viper?

@sagikazarmark
Copy link
Collaborator

@flw-cn Thanks for opening this PR. It seems that the linked PR indeed introduced a regression.

@inkychris
Copy link
Contributor

It looks like this only affects flags? The new keyComponents function should always return as many components as possible, ie: it will favour [foo bar] over [foo.bar]. For some reason, it is not seeing foo when it is only set by a flag value so it then sets foo.bar as the last resort which then doesn't get unmarshalled correctly.

@sagikazarmark
Copy link
Collaborator

I explained in #673 why that's the case. Defaults and explicit sets are also affected. Basically everything that's not a nested structure (config files, key value stores)

@inkychris
Copy link
Contributor

I'm not seeing the same behaviour for SetDefault or Set...

package viper

import (
	"github.com/spf13/pflag"
	"github.com/stretchr/testify/assert"
	"testing"
)

type Config struct {
	Foo struct {
		Bar string
	}
}

func Test673_PFlags(t *testing.T) {
	v := New()
	flags := pflag.NewFlagSet("test", pflag.ContinueOnError)
	flags.String("foo.bar", "cobra_flag", "")
	assert.NoError(t, v.BindPFlags(flags))

	config := &Config{}
	assert.NoError(t, v.Unmarshal(config))
	assert.Equal(t, "cobra_flag", config.Foo.Bar)
}

func Test673_Default(t *testing.T) {
	v := New()
	config := &Config{}
	v.SetDefault("foo.bar", "default")
	assert.NoError(t, v.Unmarshal(config))
	assert.Equal(t, "default", config.Foo.Bar)
}

func Test673_Set(t *testing.T) {
	v := New()
	config := &Config{}
	v.SetDefault("foo.bar", "default")
	v.Set("foo.bar", "override")
	assert.NoError(t, v.Unmarshal(config))
	assert.Equal(t, "override", config.Foo.Bar)
}

Currently for me, the PFlags test is failing but the other two are okay; am I testing the right thing here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants