Fix various text escaping issues #86
Conversation
This commit normalizes the meaning of "property.Value". It always contains an unserialized property value. Previously, property params were deserialized when parsing but not serialized again.
Previously, ";" was incorrectly escaped. First, ";" was replaced by "\;". Then, the backslash was escpaed to "\\", resulting in "\\;". Now, we first escape all backslashes and then escape other characters.
|
@frereit Do you have an example / test of where this fails in the current implementation? |
|
I have added a test that is fixed by 0f8a325. 3ffa099 is not directly fixing a bug but simply normalizing when escaping happens. This is so that these two statements have the same effect: event.SetProperty(ics.ComponentPropertyDescription, "D:escription")
event.SetDescription("D:escription")which they currently do not have, because |
|
By the way, this closes #68 |
|
Thanks. Running tests now. |
|
Seems there is a white space issue in the test on Mac OS X |
frereit
force-pushed
the
fix/text_escaping
branch
from
59a2d54 to
647cf9e
Compare
|
I think the issue was that the order of property parameters was non-deterministic. I've removed the unneccessary property parameter, the test should now be fixed. It's a bit difficult to debug because the test passes on my local machine. |
I can believe that.. Although I thought I had written this with very limited use of maps as I wanted it to be bidirectional and deterministic.. It didn't show up in tests and I can't see how this would create more nondeterministic behavior, plus only showing up on mac is a bit odd. One earlier PR did add some fuzzing. I will have to make some time to do more advanced testing it looks like. |
property.Valueto always contain unescaped values. They are escaped when serializing, and unescaped when parsing.