search

kdl-org / kdl

the kdl document language specifications
https://kdl.dev
Other
1.1k stars 61 forks source link

Do not escape / (Solidus, Forwardslash) #197

Closed danini-the-panini closed 2 years ago

danini-the-panini commented 2 years ago

I don't believe / need to be escaped in strings. This may be considered a breaking change now that we've reached 1.0, so another solution might be to allow it to be escaped during parsing, but not to escape it when stringifying.

Closes #194

zkat commented 2 years ago

This is, unfortunately, a breaking change, so I'm moving it to target the v2 branch.

tabatkins commented 2 years ago

The secondary solution (allow it to be escaped in parsing, but don't require it to be escaped in printing) appears to already be expected by the testsuite; see all_escapes.kdl and its expected output.

(I think superfluous escapes are fine - all it means is we can't give special meaning to an escaped forward slash \/ later - so I don't think we need to change any behavior here at all. But it would certainly be tidier to remove \/, since it does suggest that / would do something special in strings if not escaped.)

tabatkins commented 2 years ago

oh lol I should look at commit histories; that was changed by the OP several days ago. :+1:

danini-the-panini commented 2 years ago

@tabatkins oh, sorry, i made a bunch of alternative fixes, one of which was to do exactly what you've suggested, which got merged 😝

hkolbeck commented 2 years ago

If an alternate has been merged, can we close this?

tabatkins commented 2 years ago

The alternate that was merged was just a change to the expected serialization for the tests; the larger (breaking change) question of whether to remove the \/ escape entirely is still open as a possible 2.0 change.