Fix (un)escaping when building message #987
Conversation
|
One alternative, albeit more difficult to do, would require us to introduce an more specialized string class that tracks whether something is escaped or not. |
|
Just tried to remove the replacement of
|
|
Actually, it was introduced as part of #705, so I guess it had to do that stuff didn't get formatted correctly. I also remember plenty of cases where FA crashed when somebody used a |
|
Looks like the build is failing |
|
The two failing tests asserts that the output is escaped. My test above with (un)escaping still escaped |
|
@dennisdoomen I've updated the PR. |
dennisdoomen
left a comment
There was a problem hiding this comment.
Honestly, I don't have a clue whether this will break anything. We'll have to see.
|
Any reason not to merge it? |
Unescapeis the inverse ofEscape, such thatstr == Unescape(Escape(str)).But it is problematic if the escaped pattern already exists in the non-escaped string, as calls to
EscapeandUnescapedoesn't seem to be balanced(?).It's difficult to fix this properly, as
Unescapewould need to know if some substring was escaped or not.The approach here is based on the assumption that no one ever uses the ASCII character
0x1B(arbitrarily chosen).Even though that is not universally true, I believe that character is less used than
\.By surrounding the escaped string with
0x1Bwe can now see when a substring was escaped, and also guard against double escaping.Two tests now fails, as
Unescapeis not called on the escaped string, so it still contains0x1B.When_element_has_attributes_it_should_include_them_in_the_outputWhen_element_has_child_element_it_should_not_include_them_in_the_outputI'm not sure if it's a problem though.
Fixes #986