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

Behavior change between 0.1.0 and 0.1.1 #43

Open
davidnewhall opened this issue May 4, 2018 · 3 comments
Open

Behavior change between 0.1.0 and 0.1.1 #43

davidnewhall opened this issue May 4, 2018 · 3 comments

Comments

@davidnewhall
Copy link

davidnewhall commented May 4, 2018

If this were a major version change, I'd accept that "things change" but since it was a minor revision, I feel compelled to point out my now-broken use-case with this library. Maybe someone can correct my flaws and show me a valid way to do this. Or we can identify this as some sort of regression.

type tomlConfig struct {
	IncludeDir string
	Services []*Config
}

type Config struct {
	Name string
	Paths []string
}

func loadConfigFile(configFile string) (tomlConfig, error) {
	var config tomlConfig
	if buf, err := ioutil.ReadFile(configFile); err != nil {
		return config, err
	} else if err := toml.Unmarshal(buf, &config); err != nil {
		return config, err
	} else if config.IncludeDir == "" {
		return config, nil
	}

	// include_dir is not empty, read in more config files from a conf.d folder.
	includedConfigFiles, err := ioutil.ReadDir(config.IncludeDir)
	if err != nil {
		return config, err
	}

	for _, includeFile := range includedConfigFiles {
		filePath := filepath.Join(config.IncludeDir, includeFile.Name())
		if !strings.HasSuffix(filePath, ".conf") {
			continue // Only read files that end with .conf.
		} else if buf, err := ioutil.ReadFile(filePath); err != nil {
			return config, err
		} else if err := toml.Unmarshal(buf, &config); err != nil {
			return config, err
		}
	}
	return config, nil
}

The above code works great with toml library 0.1.0. The Services slice is appended to for each [[services]] found in the included config files. In version 0.1.1 the slice only contains the data from the last config file Unmarshal()'d. Thoughts?

@davidnewhall
Copy link
Author

I've worked around this by using a new tomlConfig for each loop iteration and appending the things I care about to the originally unmarshal'd dataset. I assume I was using a bug in the original code that was failing to clear out defaults. Providing a toml.UnmarshalAppend() function would be nifty so the above pattern can still be used. No priority from me here. Thanks!

@fjl
Copy link
Collaborator

fjl commented Jul 24, 2018

Just checked and you're right: the behavior did change. I think the new behavior is more correct, but a config option could be added to disable 'clearing' of slices and maps. Are you up for implementing that?

@davidnewhall
Copy link
Author

Sorry, I just wound up using burnt sushi.

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

No branches or pull requests

2 participants