magiconair / properties

Java properties scanner for Go
BSD 2-Clause "Simplified" License
323 stars 77 forks source link

enhance error message for circular references #50

Closed sriv closed 3 years ago

sriv commented 3 years ago

Note: This is an improvement over #49 for circular reference error message.

Context

When there are properties with circular references, the error message thrown contains one of the key in the cycle. This can be a bit annoying to fix, since it does not give a trace. Given this library supports loading properties from multiple sources, grep etc can fail.

Input:

foo = depends on ${bar}

bar = which depends on ${baz}

baz = ${foo} completes a cyclic reference

The error message thrown when properties.MustLoadFile is called with the above file:

circular reference in "foo = ${foo}"

This pull request is an attempt to make it look better, my take:

circular reference in:
foo=depends on ${bar}
bar=which depends on ${baz}
baz=${foo} completes a cyclic reference

This is not perfect, it still does not highlight the exact source of these properties, but I felt that the overhead of carrying that context to properties.expand may be a bit too much, since the method is currently stateless.

Thanks for the library!

sriv commented 3 years ago

The build is presently failing in golang <= 1.9, because strings.Builder is not available. I see that travis builds are setup for go 1.3+

Just wanted to check if the project aims to be compatible with go <=1.9, in which case I can use plain string concatenation.

magiconair commented 3 years ago

go1.3 compatibility isn't strictly necessary but I've been enjoying the fact that this library is usable with all the runtimes over the last 6 years that it has been somewhat of a pet peeve to keep it that way. If it is really necessary to break it then I'll do it. However, I think you can just use bytes.Buffer and it'll work.

sriv commented 3 years ago

Fair enough, and I think it makes sense especially when the change is as simple as you suggested. Should have thought of it myself, thanks for the quick review.

magiconair commented 3 years ago

Hmm, so while being fun it isn't very helpful...

image
magiconair commented 3 years ago

10 min for a CI run isn't really acceptable

magiconair commented 3 years ago

Tagged v1.8.4. Thank you!