the-sett / elm-syntax-dsl

A DSL for creating Elm syntax trees and pretty printing Elm source code.
BSD 3-Clause "New" or "Revised" License
20 stars 4 forks source link

escape sequence in `Elm.CodeGen.string` sometimes isn't escaped in pretty printed expression #50

Closed lue-bird closed 4 months ago

lue-bird commented 5 months ago

While all the usual escape sequences like Elm.CodeGen.string "\n" or with "\\", "\t" or "\"" get printed as "\n", "\\" and "\t", "\"", the same is not true for carriage return and backslash u{...} sequences:

Test.describe "escape sequence in `Elm.CodeGen.string` sometimes isn't escaped in pretty printed expression"
    [ Test.test "string with escaped carriage return is pretty printed correctly"
        (\() ->
            Elm.CodeGen.string "\u{000D}"
                |> Elm.Pretty.prettyExpression
                |> Pretty.pretty 100
                |> Expect.equal "\"\\r\""
        )
    , Test.test "string with escaped combo emoji is pretty printed correctly"
        (\() ->
            Elm.CodeGen.string "👩\u{200D}🚒"
                |> Elm.Pretty.prettyExpression
                |> Pretty.pretty 100
                |> String.toList
                |> Expect.equalLists ("\"👩\\u{200D}🚒\"" |> String.toList)
        )
    ]

the elm-test output is

✗ string with escaped carriage return is pretty printed correctly

    "\"\r\""
    ╷
    │ Expect.equal
    ╵
    "\"\\r\""

✗ string with escaped combo emoji is pretty printed correctly

    '"', '👩', '‍', '🚒', '"'
    ╷
    │ Expect.equalLists
    ╵
    '"', '👩', '\\', 'u', '{', '2', '0', '0', 'D', '}', '🚒', '"'

    The first diff is at index 2: it was `'‍'`, but `'\\'` was expected.

This was noticed by Ed Kelly in https://github.com/lue-bird/elm-review-documentation-code-snippet/issues/5

The fix would be recognizing and handling those in https://github.com/the-sett/elm-syntax-dsl/blob/master/src/Elm/Pretty.elm#L1784 which needs to check both

rupertlssmith commented 5 months ago

Been a while since I looked at this, but I definitely remember problems with escaping unicode that were never fixed.

Will you be submitting a PR with a fix?

lue-bird commented 5 months ago

Sure, let me make a PR for this.

Edit: done in #51

rupertlssmith commented 4 months ago

Published as 6.0.3