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

SaveTo method, the whitespaces are missing #229

Closed
billychou opened this issue Mar 25, 2020 · 3 comments
Closed

SaveTo method, the whitespaces are missing #229

billychou opened this issue Mar 25, 2020 · 3 comments
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@billychou
Copy link

Describe the bug
Hi ,
As shown below, the reg_gsid value contains a blank space.
my.ini

[access_log]
reg_gsid = ` (\d{39,41}) `
cfg, err := ini.Load("my.ini")
cfg.SaveTo("local.ini")

As shown above, the blank space is gone when i use saveTo method.
Please, How to reverse the blank space when i use the method. Thanks a lot.

To Reproduce

cfg, err := ini.Load(file)
if err != nil {
	log.Fatal(err)
}
if strings.HasPrefix(regGsid, " ") {
	println("regGsid prefix blank")
}
cfg.SaveTo("local.ini")
newcfg, err := ini.Load("local.ini")
if err != nil {
	log.Fatal(err)
}
newRegGsid := newcfg.Section("access_log").Key("reg_gsid").String()
if strings.HasPrefix(newRegGsid, " ") {
	fmt.Println("newRegGsid prefix blank")
}

Expected behavior
A clear and concise description of what you expected to happen.

Screenshots

Additional context
Add any other context about the problem here, or any suggestion to fix the problem.

@billychou billychou changed the title SaveTo method, the spaces is gone SaveTo method, the whitespaces are missing Mar 25, 2020
@unknwon unknwon added the bug Something isn't working label Mar 25, 2020
@unknwon
Copy link
Member

unknwon commented Mar 25, 2020

The problem is we don't check leading and trailing whitespaces in

ini/file.go

Line 383 in 09e0d86

if strings.Contains(kname, "\"") || strings.ContainsAny(kname, f.options.KeyValueDelimiters) {

and

ini/file.go

Line 426 in 09e0d86

case strings.Contains(kname, "\"") || strings.ContainsAny(kname, f.options.KeyValueDelimiters):

@unknwon unknwon added the good first issue Good for newcomers label Mar 25, 2020
@prepodavan
Copy link

prepodavan commented Jun 20, 2020

test file_test.go#TestFile_WriteTo compares full.ini and golden file. They have key lots_of_lines (last one). Parser produce value with trailing space without quoting "1 2 3 4 " because actually last line in full.ini has trailing space - "4 " (space between 4 and ).
As I understand there's only one direct way how file.go#writeToBuffer could recognize should or should not any value be surrounded by any quote char - len(strings.TrimSpace(value)) != len(value). But such way will quote lots_of_lines value and produce "1 2 3 4 " not "1 2 3 4" so test file_test.go#TestFile_WriteTo will fail compairing to golden.
To fix this issue in such way either or last line of full.ini should be changed to 4\ (from 4 \) or golden should quote value of lots_of_lines so it will be "1 2 3 4 " (with trailing space inside quotes).
Otherwise you have to remember somehow in code values which should be surrounded. This way will require a lot of changes as far as parser reading functions returns just string value and error without any context (except context of parser itself). So i guess way with changing old behaviour anyhow is more preferable.
I would like to fix this issue anyway and I came to ask which way is more preferable

@unknwon
Copy link
Member

unknwon commented Aug 16, 2020

Thanks @prepodavan!

Golden file is just a way to "cache" expected results, if the comparison failed in a desired way, we just need to update the golden file (with -update flag) :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants