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

added a setting to make viper case sensitive #592

Closed
wants to merge 1 commit into from

Conversation

e-nikolov
Copy link

@e-nikolov e-nikolov commented Nov 7, 2018

In some cases it is useful to preserve the case sensitivity of keys. This change adds the option to call viper.SetCaseSensitive(true) to enable case sensitivity.

This feature is needed for example when unmarshalling a config into a map[string]interface{}, changing some of the config values and then saving it again to a file. If the keys are always lowercased, the structure of the regenerated config will be changed, which is undesirable.

Addressing #260, #293, #373

@CLAassistant
Copy link

CLAassistant commented Nov 7, 2018

CLA assistant check
All committers have signed the CLA.

Copy link
Collaborator

@sagikazarmark sagikazarmark left a comment

Choose a reason for hiding this comment

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

IMHO this is a major change in the library, so it should be extensively tested to make sure it works as intended.

@@ -277,6 +279,13 @@ func (v *Viper) OnConfigChange(run func(in fsnotify.Event)) {
v.onConfigChange = run
}

func SetCaseSensitive(flag bool) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe add some comments here that will appear in godoc.

func SetCaseSensitive(flag bool) {
v.SetCaseSensitive(flag)
}
func (v *Viper) SetCaseSensitive(flag bool) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe add some comments here that will appear in godoc.

@@ -1827,6 +1850,14 @@ func (v *Viper) findConfigFile() (string, error) {
return "", ConfigFileNotFoundError{v.configName, fmt.Sprintf("%s", v.configPaths)}
}

func (v *Viper) properCase(s string) string {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe call this prepareKey or something less tied to the casing.

@@ -1572,6 +1591,10 @@ func (v *Viper) WatchRemoteConfigOnChannel() error {
}

func (v *Viper) insensitiviseMaps() {
if v.caseSensitive {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This looks unnecessary, at least I think you will never reach this check, since you do it before calling this method:

https://github.com/spf13/viper/pull/592/files#diff-38f1012180f40493dcf780c312a40ec3R1407

I think it would be better to do these checks outside of the insensitive methods.

@@ -219,6 +220,7 @@ func New() *Viper {
v.env = make(map[string]string)
v.aliases = make(map[string]string)
v.typeByDefValue = false
v.caseSensitive = false
Copy link
Collaborator

Choose a reason for hiding this comment

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

It will be false by default, no need to set it here IMHO.

@kansz
Copy link

kansz commented Jun 2, 2020

What time the feature can merged back to master branch? I need it

@lnashier
Copy link

@sagikazarmark @e-nikolov can you comment why it was closed?

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.

5 participants