[Repo Assist] fix: use ctx.Newline in ToMd table rows and ToFsx output comments#1195
Draft
github-actions[bot] wants to merge 2 commits intomainfrom
Draft
Conversation
Previously, the TableBlock serialiser in MarkdownUtils.fs joined all data rows into a single string using String.concat "\\n" (hardcoded Unix newline), and then emitted a literal "\n" string as the trailing blank line sentinel. On Windows (ctx.Newline = "\r\n") this produced mixed line endings in the ToMd output. This commit refactors the TableBlock case to yield each data row as a separate string (consistent with every other paragraph type), so the outer String.concat newline in formatAsMarkdown applies the correct separator. The trailing blank line now uses the standard "" empty-line sentinel. FsxFormatting.fs had the same kind of issue: the (\* output: ...*) comment wrapper used a literal "\n" to open the comment block. It now uses ctx.Newline so both the opening and the content use consistent line endings. Two new unit tests cover the table row count and the Windows regression. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
16 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤖 This PR was created by Repo Assist, an automated AI assistant.
Summary
Two targeted bug fixes for hardcoded
"\n"newline usage in the Markdown serialisers.1.
Markdown.ToMd—TableBlockrows used hardcoded"\n"(Task 3 — bug fix)Root cause
formatParagraphforTableBlockcollected all data rows into a single string withString.concat "\n", then yielded that one big string. The trailing blank-line was emitted asyield "\n"(a literal newline character string) rather than the standardyield ""(empty-line sentinel) used by all other paragraph types.Effect
On Windows (
ctx.Newline = "\r\n"),Markdown.ToMdproduced mixed line endings in the output: headers and alignments used\r\n(from the outerString.concat newline), while data rows used\n(from the hardcodedString.concat "\n"inside). The trailing blank was also a bare\n.Fix (
src/FSharp.Formatting.Markdown/MarkdownUtils.fs)[for r in rows ...] |> String.concat "\n"with afor r in rows do yield ...loop, so each data row is emitted as a separate string item.yield "\n"→yield "", consistent with all other paragraph types.The outer
String.concat newlineinformatAsMarkdownnow correctly joins data rows with the configured newline.2.
Markdown.ToFsx— output-comment used hardcoded"\n"(Task 5 — coding improvement)Root cause
FsxFormatting.fsopened the(* output: ...*)comment with a literal"\n":The
outputstring was already built withctx.Newline, but the opening comment-marker used a Unix newline regardless.Fix (
src/FSharp.Formatting.Markdown/FsxFormatting.fs)"(* output: \n"→"(* output: " + ctx.Newlineso the entire comment uses consistent line endings.Tests added (
tests/FSharp.Markdown.Tests/Markdown.fs)ToMd table rows each appear on their own line— parses a two-row table, serialises withnewline = "\n", and asserts the non-blank lines equal exactly 4 (header + separator + 2 data rows).ToMd table row count is correct when Windows newline is used— regression test: serialises withnewline = "\r\n"and asserts there are no stray\nembedded in the output (no\r\n\nor\n\r\nsequences).Test Status
dotnet build FSharp.Formatting.sln --configuration Release— succeeded (0 warnings, 0 errors)dotnet test tests/FSharp.Markdown.Tests— 348/348 passed (includes 2 new tests)dotnet fantomas ... --check— formatting verified (no changes needed)dotnet test FSharp.Formatting.sln --configuration Release --no-build— full suite passes (Literate: 143, ApiDocs: 88+4 skipped, CodeFormat: 30, fsdocs-tool: 12, Markdown: 348)