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

"global" section surprising and indistinguishable from real section named 'global' ? #8

Open
Dieterbe opened this issue Jun 16, 2021 · 1 comment · May be fixed by #10
Open

"global" section surprising and indistinguishable from real section named 'global' ? #8

Dieterbe opened this issue Jun 16, 2021 · 1 comment · May be fixed by #10

Comments

@Dieterbe
Copy link

Hello,
seems that the library always adds a "global" section, even for empty files.
This is surprising because it's not mentioned in the readme, nor in the api docs
The way it is implemented seems particularly worrisome. While I can see how we would want a place to keep globally/un-sectioned parameters, the current approach means you cannot distinguish global parameters from a real global section.
for example the below program will print the section as foo=42, whether that parameter was declared globally or in a section literally called [global]
(full output after source code)

package main

import (
	"fmt"
	"io/ioutil"
	"os"

	"github.com/alyu/configparser"
)

func handleErr(err error) {
	if err != nil {
		panic(err)
	}
}

func main() {
	fd, err := ioutil.TempFile("", "")
	handleErr(err)

	defer os.Remove(fd.Name())

	_, err = fd.Write([]byte(`foo=42`))
	handleErr(err)

	err = fd.Close()
	handleErr(err)

	dump("empty file", fd.Name())

	fd2, err := ioutil.TempFile("", "")
	handleErr(err)

	defer os.Remove(fd2.Name())

	_, err = fd2.Write([]byte(`[global]
foo=42`))
	handleErr(err)

	err = fd2.Close()
	handleErr(err)

	dump("file with global section", fd2.Name())
}

func dump(info, fname string) {
	config, err := configparser.Read(fname)
	handleErr(err)
	fmt.Printf("### start dump of %q ###\n", info)
	fmt.Println("config.String():", config.String())

	sections, err := config.AllSections()
	handleErr(err)
	for _, s := range sections {
		fmt.Println("section: ", s.String())
		fmt.Println("# options", s.Options())
		fmt.Println("# optionNames", s.OptionNames())
	}
	fmt.Printf("### end dump of %q ###\n", info)
	fmt.Println()
	fmt.Println()
}
### start dump of "empty file" ###
config.String(): foo=42

section:  foo=42

# options map[foo:42]
# optionNames [foo]
### end dump of "empty file" ###


### start dump of "file with global section" ###
config.String(): foo=42

section:  
# options map[]
# optionNames []
section:  foo=42

# options map[foo:42]
# optionNames [foo]
### end dump of "file with global section" ###
Dieterbe added a commit to grafana/configparser that referenced this issue Jun 16, 2021
fix alyu#8

Note: this does not update a bunch of functions to query for sections
etc, modify them, etc. Those currently don't have access to the new
global section.
@Dieterbe
Copy link
Author

what i have in mind is something like this:
grafana@122fe6a

seems to work, with the caveat that some of the other functionality still needs to be updated

Dieterbe added a commit to grafana/configparser that referenced this issue Jun 17, 2021
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 a pull request may close this issue.

1 participant