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

Desired behavior when creating a new ini file #21

Closed
jmcfarlane opened this Issue Nov 24, 2015 · 5 comments

Comments

Projects
None yet
2 participants
@jmcfarlane

jmcfarlane commented Nov 24, 2015

When using ini.load() to read a file that doesn't exist, you can proceed to use the returned object to make changes and ultimately persist the object to disk via SaveTo, however it looses the top level section. I fixed the issue in my program by making sure the ini file was initialized as an empty file (if missing) prior to loading, but I wondered what the intended behavior is in this case.

Here's an example program to illustrate the question:

package main

import (
    "fmt"
    "gopkg.in/ini.v1"
)

func main() {
    p := "/tmp/missing-file.ini"
    cfg, err := ini.Load(p)
    fmt.Println(cfg, err)
    cfg.Section("section").NewKey("key", "value")
    cfg.SaveTo(p)
}

Running the program results in this:

$ go run test.go
&{true {{0 0} 0 0 0 0} [{/tmp/missing-file.ini}] map[] [] <nil>} open /tmp/missing-file.ini: no such file or directory
$ cat /tmp/missing-file.ini
key = value

Because the write ultimately succeeded, running the same program again
has this result:

$ go run test.go
&{true {{0 0} 0 0 0 0} [{/tmp/missing-file.ini}] map[DEFAULT:0xc820018190] [DEFAULT] <nil>} <nil>
$ cat /tmp/missing-file.ini
key = value

[section]
key = value

Any thoughts on if this is the intended behavior? My instinct is that this is a defect, and the library should either write the correct textual representation of the object, or it should fail to write because the file doesn't exist (consistent with err returned by ini.load()).

Unknwon added a commit that referenced this issue Nov 24, 2015

@Unknwon

This comment has been minimized.

Member

Unknwon commented Nov 24, 2015

Hi, you're right!

I've pushed code to return nil for cfg when error is presented, please go get -u update can test again.

@jmcfarlane

This comment has been minimized.

jmcfarlane commented Nov 27, 2015

After updating my local copy of go-ini via go get -u I see the code has changed, but the behavior (as seen by the test snippet I pasted originally) has not changed. ini.Load() is still returning an object, where I think the intent of the change was to return nil in the "file is missing" case.

@Unknwon

This comment has been minimized.

Member

Unknwon commented Nov 27, 2015

Based on https://github.com/go-ini/ini/blob/v1/ini_test.go#L122-L124 , I think your test processing is not correct. And I get panic by running your test.

@jmcfarlane

This comment has been minimized.

jmcfarlane commented Nov 28, 2015

Indeed! I had my import specified via "gopkg.in/ini.v1" so I wasn't actually seeing your commit. Testing against "github.com/go-ini/ini" and I now see a panic. Thanks :)

@jmcfarlane jmcfarlane closed this Nov 28, 2015

@Unknwon

This comment has been minimized.

Member

Unknwon commented Nov 30, 2015

🍻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment