walle/cfg

Odd handling of quotes

dropwhile opened this issue · 4 comments

If I modify decode_test.go with the following...

const configString = `
# This is a comment

# An integer value
answer = 42

# A float value
pi = 3.14
# A boolean value
is_active = true

# A string value
quotes = Alea iacta est\nEt tu, Brute?

# a string in quotes
quoted_string = "this is a string too!"
`

Then adjust the tests to handle that case, I see this result.

--- FAIL: Test_Unmarshal (0.00s)
    decode_test.go:70: Expected "this is a string too!", got "\"this is a string too!\""
--- FAIL: Test_UnmarshalFromConfig (0.00s)
    decode_test.go:135: Expected "this is a string too!", got "\"this is a string too!\""

Is this expected? Most other "INI-like" formats strip quotes from the front and back of strings in this case.

Adding this to GetString makes it behave as I would expect.

diff --git a/config.go b/config.go
index f7ebdfe..38dfd3f 100644
--- a/config.go
+++ b/config.go
@@ -50,6 +50,9 @@ func (c *Config) GetString(key string) (string, error) {
    }

    uq := strings.Replace(val, "\\n", "\n", -1)
+   if strings.HasPrefix(uq, `"`) && strings.HasSuffix(uq, `"`) {
+       uq = strings.Trim(uq, `"`)
+   }
    return uq, nil
 }

If you are amenable to the change, I can open a pull req with associated tests.

walle commented

Thanks for reporting!

I would say that this is expected behaviour, the only character that is special is the line break. Because it would break the line based format if it isn't escaped. But if you write quotes you get quotes :)

This keeps down the number of special cases. E.g. what to do about single quotes, unicode quotes (the quotes that looks like quotes but isn't) and so on. This way everything you put in the key/value will be there. The one special case is line breaks.

But you are right that it's a bit confusing. I will add a test case that verifies that this is the intended behaviour.

If I'm missing something here please tell me.

Maybe adding some clarifying text to the readme would help too?

walle commented

Great idea! I will do so.