Skip to content

Don't potentially mix EOL sequences in output documents - #12

Merged
SimonCropp merged 1 commit into
SimonCropp:masterfrom
atifaziz:eols
Mar 8, 2019
Merged

Don't potentially mix EOL sequences in output documents#12
SimonCropp merged 1 commit into
SimonCropp:masterfrom
atifaziz:eols

Conversation

@atifaziz

@atifaziz atifaziz commented Mar 8, 2019

Copy link
Copy Markdown
Contributor

This PR fixes the trouble with C# verbatim strings that new-line (EOL) endings get hard-coded into the compiler binary based on the platform where it was compiled. The resulting documents, once again depending on the platform, could then potentially end up with mixed cases of EOL sequences. It seems that most instances in the code using verbatim strings to insert text into the resulting Markdown could just do with WriteLine of a TextWriter instance, which is usually initialised with the new-line sequence of the host platform. The outcome is dull, having the least element of surprise, which unfortunately/apparently is a Good Thing™. 😜

@claremacrae

Copy link
Copy Markdown
Contributor

Ah - thank you very much.

Could this explain this commit?

approvals/ApprovalTests.cpp@ff7b6e7

Only a few lines changed in the .source.md - whereas the whole .md is shown as changed.

(running on Windows)

@atifaziz

atifaziz commented Mar 8, 2019

Copy link
Copy Markdown
Contributor Author

Only a few lines changed in the .source.md - whereas the whole .md is shown as changed.

Possibly or I'd expect at least partially! It's getting late in my time zone so haven't completely thought this through but I think your .gitattributes and/or global config might have something to do with it too. At the end of the day, it's down to the platform on which the MarkdownSnippets.Tool was compiled, the user's/your documentation platform and Git settings. If your Git's configured right, it might normalise the line endings irrespective of what MarkdownSnippets does.

@SimonCropp SimonCropp added this to the 9.0.4 milestone Mar 8, 2019
@SimonCropp
SimonCropp merged commit 366dd5c into SimonCropp:master Mar 8, 2019
@SimonCropp

Copy link
Copy Markdown
Owner

Thanks for this. Although that is annoying. I prob have this bug in many projects.

I might write a fody weaver that automates it

@SimonCropp

Copy link
Copy Markdown
Owner

This is now deployed. NuGet may take some time to make it available for download.

@atifaziz
atifaziz deleted the eols branch March 8, 2019 23:02
@atifaziz

atifaziz commented Mar 8, 2019

Copy link
Copy Markdown
Contributor Author

This is now deployed.

Kudos for maintaining the ”release it often“ cadence on this project. 💯

SimonCropp added a commit that referenced this pull request Jul 7, 2026
Three behavior-preserving allocation reductions on always-on paths. Each was
measured with GC.GetAllocatedBytesForCurrentThread and produces byte-identical
output; the full snapshot suite stays green.

- GetLanguageFromPath: span-based Path.GetExtension + a single lowercasing
  string.Create, replacing GetExtension + TrimStart('.') + ToLowerInvariant
  (3 string allocations -> 1). 71.2 -> 28.8 B/call. Called for every file
  during discovery and again per snippet source file.

- MarkdownProcessor toc checks: line.Current.TrimStart() == "toc" ->
  span SequenceEqual, dropping a trimmed-string allocation on every indented
  line.

- Line.LeadingWhitespace: computed lazily. Only snippet / web-snippet lines
  ever read it, but the ctor allocated a leading-whitespace substring for every
  indented line in the document.

Wins 2 and 3 together cut ~13.5% of the allocations when applying an
indentation-heavy document. Adds OptimizationBenchmarks3 (#11
GetLanguageFromPath, #12 indented Apply) to the existing benchmark suite.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants