Skip to content

Suppress false positive markdown violation#22222

Closed
adegeo wants to merge 1 commit intomasterfrom
adegeo-fix-md
Closed

Suppress false positive markdown violation#22222
adegeo wants to merge 1 commit intomasterfrom
adegeo-fix-md

Conversation

@adegeo
Copy link
Copy Markdown
Contributor

@adegeo adegeo commented Jan 4, 2021

Summary

This file is causing some linter errors on various PRs. This error is a false positive though and this PR disables that specific error check.

@adegeo adegeo changed the title Suppress incorrect markdown violation Suppress false positive markdown violation Jan 4, 2021
The regular expression pattern `href\s*=\s*(?:["'](?<1>[^"']*)["']|(?<1>\S+))` is interpreted as shown in the following table.


<!-- markdownlint-disable MD011 -->
Copy link
Copy Markdown
Member

@Youssef1313 Youssef1313 Jan 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The table doesn't render correctly (or at least in some parsers), so this isn't a false positive.

image

One possible fix is to use <code></code> instead of backticks, and use the encoding of the vertical bar. Not sure if there is a better solution.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does render correctly. See the live site:

image

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@IEvangelist Not by GitHub parser at least. I'm not sure whether you care about it being rendered correctly in both docs site and GitHub or only docs site. But I personally often view files in GitHub. So better if works in both.

Copy link
Copy Markdown
Member

@IEvangelist IEvangelist Jan 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Youssef1313 Happy New Year!
I agree that it would be nice to have it render correctly in both, but my primary concern is the docs site. Instead of using <code></code>, are there any obvious or alternative approaches you have in mind that do not break the inline HTML concern/linting rules?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@IEvangelist Happy new year to you too!

I'd probably ask @DavidAnson for help on this. I opened DavidAnson/markdownlint#363 asking for help on this.

Meanwhile, you can merge to fix the docs site, which is more important than GitHub-view.

Copy link
Copy Markdown
Member

@IEvangelist IEvangelist left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me @adegeo, :shipit: when you're ready.

@DavidAnson
Copy link
Copy Markdown
Contributor

I fixed something that looks very much like this issue last month: #21977

Was that reverted for some reason?

@DavidAnson
Copy link
Copy Markdown
Contributor

Doesn't seem reverted and still renders correctly on GitHub (and real site), it seems: https://github.com/dotnet/docs/blob/master/docs/standard/base-types/regular-expression-example-scanning-for-hrefs.md

Where isn't this working?

@Youssef1313
Copy link
Copy Markdown
Member

@DavidAnson Thanks!

@adegeo It looks like you only need to merge master into your branch in the PR where markdownlint is failing

@adegeo
Copy link
Copy Markdown
Contributor Author

adegeo commented Jan 5, 2021

For some reason my local fork wasn't pulling down the commit @DavidAnson added. I've cleaned and rebased and now I see it. odd. I think we can close this.

@adegeo adegeo closed this Jan 5, 2021
@adegeo adegeo deleted the adegeo-fix-md branch January 11, 2021 20:04
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.

4 participants