Jens-G commented on PR #3786: URL: https://github.com/apache/thrift/pull/3786#issuecomment-5567300078
> \\ already does this today, with the same result. On the merge base, const string ISO8601U = "Y-m-d\\TH:i:s.uP" compiles, and gen-php/Example/Constant.php comes out byte-identical to what this PR produces for the ticket's unescaped spelling — return "Y-m-d\\TH:i:s.uP";, which PHP evaluates to Y-m-d\TH:i:s.uP, exactly the format string the reporter wanted. The same holds for the other 23 generators. So the capability is not missing; the PR removes the need to double the backslash. That is worth having, but it is an ergonomics change rather than a bug fix, and it has costs \\ does not: > > It burns the escape namespace. After this, "\x41", "\0" and "\u00e9" have a defined meaning — the literal characters. Adding \xNN, \0 or \uXXXX later becomes a silent behaviour change for anyone who wrote them expecting pass-through. > > It makes the escape set silently context-dependent. "C:\dir\file.txt" now yields C:\dir\file.txt, but "C:\new\table.txt" yields C:<LF>ew<TAB>able.txt — same shape, opposite meaning, no diagnostic either way. Before, the first was rejected, and that rejection was the signal that the string needed escaping. That was exactly my first thought when I read the ticket, so I hade Claude to verify that hunch and back it up. To sum it up: I am not really happy with that change. Maybe we can improve the error message to lead the user to the real solution. What do you say? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
