Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update the code fix to respect markdown rules #4131

Closed
Youssef1313 opened this issue Sep 4, 2020 · 8 comments · Fixed by #4825
Closed

Update the code fix to respect markdown rules #4131

Youssef1313 opened this issue Sep 4, 2020 · 8 comments · Fixed by #4825
Assignees
Labels
Area-Microsoft.CodeAnalysis.Analyzers Feature Request help wanted The issue is up-for-grabs, and can be claimed by commenting
Milestone

Comments

@Youssef1313
Copy link
Member

@mavasani FYI, the code that generates this need to be updated to respect markdown rules, i.e. leave blank lines around headings.

See https://github.com/DavidAnson/markdownlint/blob/main/doc/Rules.md#md022---headings-should-be-surrounded-by-blank-lines

Originally posted by @Youssef1313 in #4126 (comment)

@Evangelink
Copy link
Member

@Youssef1313 Do you want to handle the ticket?

@Youssef1313
Copy link
Member Author

@Evangelink I'm not very familiar with this area and how exactly it works. So I think someone else can get it faster than me. But if it became a stale for a while, I'll probably give it a try.

@Evangelink
Copy link
Member

I'll take care of it :)

@Evangelink
Copy link
Member

@mavasani Shall I use Environment.NewLine or force the \n line ending? I am a bit concerned that we might have some errors with non-windows environments. I guess I could probably start with the NewLine and wait to see results of the ubuntu pipeline.

@Youssef1313
Copy link
Member Author

Youssef1313 commented Feb 10, 2021

@Evangelink The fixer currently appends Environment.NewLine (it uses StringBuilder.AppendLine):

I'm a bit surprised that the current Linux builds doesn't fail 😄
The files in repo are using \r\n, and Linux build will produce \n, so the comparison should fail. Why it doesn't? I don't know. 😕 (The analyzer ignores the end of lines - it loops through all lines, not comparing exact text match):

@Evangelink
Copy link
Member

Thanks @Youssef1313! I wrote my comment before looking at the code and then thought it was a stupid question but felt weird to delete my comment afterward.

@Evangelink
Copy link
Member

@Youssef1313 do you recall if there was anything else than the header blank line that wasn't covered?

@Youssef1313
Copy link
Member Author

@Evangelink I don't. But we'll know when markdownlint is enabled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Microsoft.CodeAnalysis.Analyzers Feature Request help wanted The issue is up-for-grabs, and can be claimed by commenting
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants