From 6b3d3bbd8fd5831dc77f925b04749d59b6083633 Mon Sep 17 00:00:00 2001 From: Revar Desmera Date: Fri, 11 Sep 2026 08:40:09 -0700 Subject: [PATCH] String literals: a raw CR is a line ending, not content A string wrapped across two source lines drops the newline and keeps the next line's indentation -- already the case, and already true of a `\` continuation over CRLF. A *bare* continuation in a CRLF file was the one path left keeping the CR, so the same string had different content depending on which machine saved the file: echo(len("ab <- LF file: 4, CRLF file: 5 cd")); That is the reference's behaviour (its lexer's `\n` rule drops only the LF; the CR falls through to `.` and is appended), but it is not a behaviour worth reproducing: a raw CR in source is a line ending in every real file -- CRLF on Windows, a lone CR on a pre-OSX Mac -- and a script that wants a real CR writes `\r`. Upstream came to the same conclusion for the `\`-continuation case in openscad/openscad@dabdc41d6. So `unescapeStringLiteral` now drops CR wherever it already dropped LF, which also collapses the backslash-before-CRLF special case into the ordinary one. Net -4 lines. BelfrySCAD#404. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01JFTVMTH6yEktHDPF5csksb --- pyproject.toml | 2 +- src/value.cpp | 40 ++++++++++++++++++---------------------- tests/test_value.cpp | 21 +++++++++++++-------- 3 files changed, 32 insertions(+), 31 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 4b643d5..6e4dedd 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "scikit_build_core.build" [project] name = "openscad_cpp_evaluator" -version = "1.20.0" +version = "1.20.1" description = "C++ OpenSCAD evaluator with Python bindings" readme = "README.md" requires-python = ">=3.12" diff --git a/src/value.cpp b/src/value.cpp index 4ffecb0..06412fb 100644 --- a/src/value.cpp +++ b/src/value.cpp @@ -410,24 +410,28 @@ std::string encodeEscapedCodePoint(std::uint32_t cp) { std::string unescapeStringLiteral(const std::string& raw) { // Most strings need no work at all, and this runs on every evaluation // of a literal on the tree-walking path -- so don't build a second copy - // unless there is something to change. A bare newline counts as - // something to change (see the loop), so it has to open the scan too. - const size_t first = raw.find_first_of("\\\n"); + // unless there is something to change. A bare line ending counts as + // something to change (see the loop), so CR and LF open the scan too. + const size_t first = raw.find_first_of("\\\n\r"); if (first == std::string::npos) return raw; std::string out; out.reserve(raw.size()); out.append(raw, 0, first); for (size_t i = first; i < raw.size(); ++i) { - // A raw LF inside a string literal contributes NOTHING -- writing a - // string across two source lines joins them, keeping the second - // line's indentation: + // A raw line ending inside a string literal contributes NOTHING -- + // writing a string across two source lines joins them, keeping the + // second line's indentation: // s = "abcd // efgh"; // -> "abcd efgh", 12 characters - // A raw CR is not special and stands for itself, so a CRLF file - // leaves the CR behind (len 3 for "xy"). Both verified on - // 2026.02.01. - if (raw[i] == '\n') continue; + // CR counts as a line ending too, which the reference does not do: + // it drops only the LF, so every string wrapped in a file written + // on Windows keeps a stray CR (len 3 for "xy" there, 2 + // here). A raw CR in source is a line ending in every real file -- + // CRLF on Windows, a lone CR on a pre-OSX Mac -- and a string that + // wants a real CR in it writes \r. Same reasoning as the backslash + // continuation below. + if (raw[i] == '\n' || raw[i] == '\r') continue; if (raw[i] != '\\' || i + 1 >= raw.size()) { out.push_back(raw[i]); // a trailing lone backslash stands for itself continue; @@ -467,18 +471,10 @@ std::string unescapeStringLiteral(const std::string& raw) { ++i; break; } - case '\n': ++i; break; // backslash + LF: both go - case '\r': - // A backslash before CRLF takes the whole line ending. - // Deliberately unlike the reference, which treats the - // backslash as an undefined escape and keeps the CR -- - // leaving a stray control character in any string wrapped - // in a file written on Windows. See this suite's - // BackslashNewlineContributesNothing. - if (i + 2 < raw.size() && raw[i + 2] == '\n') { i += 2; break; } - out.push_back('\r'); - ++i; - break; + // A backslash before a line ending takes the whole thing; any + // LF after a CR is dropped by the loop itself. + case '\n': + case '\r': ++i; break; default: out.push_back(next); ++i; break; // \\ and \" land here too } } diff --git a/tests/test_value.cpp b/tests/test_value.cpp index 0daf78a..749059e 100644 --- a/tests/test_value.cpp +++ b/tests/test_value.cpp @@ -470,8 +470,8 @@ TEST(StringEscapes, BackslashNewlineContributesNothing) { // Deliberately unlike the reference implementation, which drops only // the LF and leaves the CR in the value -- a stray control character // in any string continued in a file written on Windows. - EXPECT_EQ(unescapeStringLiteral("a \\\rb"), "a \rb") - << "a lone CR is an ordinary escaped character, not a line ending"; + EXPECT_EQ(unescapeStringLiteral("a \\\rb"), "a b") + << "a lone CR is a line ending too (pre-OSX Mac)"; } // Both evaluation paths build the Value, and only one of them was reached @@ -583,11 +583,16 @@ TEST(StringEscapes, ARawNewlineInsideALiteralContributesNothing) { EXPECT_EQ(unescapeStringLiteral("x\n \n y"), "x y"); } -TEST(StringEscapes, ARawCarriageReturnStandsForItself) { - // Only the LF is special. A CRLF file therefore leaves the CR in the - // string: "xy" is three characters, not two. - EXPECT_EQ(unescapeStringLiteral("x\ry"), "x\ry"); - EXPECT_EQ(unescapeStringLiteral("x\r\ny"), "x\ry"); +TEST(StringEscapes, ARawCarriageReturnIsALineEndingToo) { + // The reference treats only the LF as special, so a string wrapped in + // a CRLF file keeps a stray CR there ("xy" is three characters + // on 2026.02.01). Deliberately not reproduced: a raw CR in source is a + // line ending in every real file -- CRLF on Windows, a lone CR on a + // pre-OSX Mac -- and a script wanting a real CR writes \r. + EXPECT_EQ(unescapeStringLiteral("x\r\ny"), "xy"); + EXPECT_EQ(unescapeStringLiteral("x\ry"), "xy"); + EXPECT_EQ(unescapeStringLiteral("x\r\n y"), "x y") << "indentation is kept"; + EXPECT_EQ(unescapeStringLiteral("x\\ry"), "x\ry") << "an escaped CR still works"; } TEST(StringEscapes, ABackslashBeforeANewlineTakesTheWholeLineEnding) { @@ -600,5 +605,5 @@ TEST(StringEscapes, ABackslashBeforeANewlineTakesTheWholeLineEnding) { // BackslashNewlineContributesNothing above. EXPECT_EQ(unescapeStringLiteral("x\\\ny"), "xy"); EXPECT_EQ(unescapeStringLiteral("x\\\r\ny"), "xy"); // reference keeps the CR - EXPECT_EQ(unescapeStringLiteral("x\\\ry"), "x\ry"); + EXPECT_EQ(unescapeStringLiteral("x\\\ry"), "xy"); }