Open jpnjfk opened 5 years ago
Hmm, I get it but this would be a breaking change since this behavior has been in the library since 2014. If you need this then this should be configurable.
Thank you for your comment. Does configurable means using an environment variable? Or callers should be able to control turning on/off the function? Of course, the default value is off. :) Is a global variable okay? I don't want to mess the existing code.
I would expect a flag in the Properties
struct which enables this, e.g. KeepBackslash
, RetainBackslash
or something similar which defaults to false
(i.e. off).
Global variable is not ok, unfortunately. This needs to be properly integrated and there need to be tests for this.
Thank you for your suggestion. I made a new commit whose comment is "To avoid a breaking change, the keepBackslash is false by default". That works as expected though I messed up the source codes including the tab alignments the Visual Source Code automatically did. Could you review my changes again?
In order to propagate the flag Properties.KeepBackslash to the lexer, I had to make changes in the other structure and the argument of lex().
I gets back the two test cases I commented out in load_test.go.
I think that my changes are ready to go. Could you review them?
I've thought about this a bit more. The Java properties implementation silently drops single backslash characters to avoid tripping over invalid escape sequences. The Go version emulates that behavior.
The method does not treat a backslash character, \, before a non-valid escape character as an error; the backslash is silently dropped. For example, in a Java string the sequence "\z" would cause a compile time error. In contrast, this method silently drops the backslash. Therefore, this method treats the two character sequence "\b" as equivalent to the single character 'b'.
(https://docs.oracle.com/javase/7/docs/api/java/util/Properties.html#load(java.io.Reader))
However, if you escape the backslash it works as expected. What am I missing?
func TestBackslash(t *testing.T) {
input := "k\\\\ey=va\\\\lue"
p := mustParse(t, input)
assert.Equal(t, p.MustGet("k\\ey"), "va\\lue")
}
Thank you for explaining the behavior. Yes, the emulation point of view, the behavior is correct by default. As I wrote before, we don't know how each property value will be processed by the callers. So, it would be nice that the package 'properties' doesn't make silent modifications as possible because the backslash may be put on purpose. Since this is an optional behavior, the change would give the developers simplification of escaping characters in text. How do you think?
Hi, Is it possible to add the option? What I want is an option that doesn't make silent modifications as possible. I agree that the workaround you wrote works. However, it would be error prone for translators who aren't programmers. It would be nice to have the package as simple as possible.
This change leaves the escape character backslash '\' when the backslash isn't precede one of " :=fnrt" that need escaping. Currently, any backslash that isn't precede one of the escaped characters is simply dropped. We don't know how each property value will be processed by the callers. So, it would be better that the package 'properties' doesn't make modifications as possible because the backslash may be put on purpose.