Skip to content

A few nullable changes #527

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Mar 12, 2021
Merged

A few nullable changes #527

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 0 additions & 14 deletions src/Markdig.Tests/TestReferralLinks.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,20 +37,6 @@ public void TestLinksWithCustomRel(string[] rels, string expected)
Assert.That(html, Contains.Substring($"rel=\"{expected}\""));
}


[Test]
public void TestLinksWithNullRel()
{
var markdown = "[world](http://example.com)";

var pipeline = new MarkdownPipelineBuilder()
.UseReferralLinks(null)
.Build();
var html = Markdown.ToHtml(markdown, pipeline);

Assert.That(html, !Contains.Substring("rel="));
}

Copy link
Collaborator Author

@MihaZupan MihaZupan Mar 11, 2021

Choose a reason for hiding this comment

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

I don't know if I'm missing something, but I don't understand this test.

If you don't want the rel being output... don't add the extension?

[Test]
[TestCase(new[] { "noopener" }, "noopener")]
[TestCase(new[] { "nofollow" }, "nofollow")]
Expand Down
8 changes: 4 additions & 4 deletions src/Markdig/Extensions/ReferralLinks/ReferralLinksExtension.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// This file is licensed under the BSD-Clause 2 license.
// See the license.txt file in the project root for more information.

using System;
using System.Collections.Generic;
using System.Linq;
using Markdig.Renderers;
Expand All @@ -13,19 +14,18 @@ public class ReferralLinksExtension : IMarkdownExtension
{
public ReferralLinksExtension(string[] rels)
{
Rels = rels?.ToList();
Rels = rels?.ToList() ?? throw new ArgumentNullException(nameof(rels));
}

public List<string>? Rels { get; }
public List<string> Rels { get; }

public void Setup(MarkdownPipelineBuilder pipeline)
{
}

public void Setup(MarkdownPipeline pipeline, IMarkdownRenderer renderer)
{
string? relString = Rels is null ? null :
string.Join(" ", Rels.Where(r => !string.IsNullOrEmpty(r)));
string relString = string.Join(" ", Rels.Where(r => !string.IsNullOrEmpty(r)));

var linkRenderer = renderer.ObjectRenderers.Find<LinkInlineRenderer>();
if (linkRenderer != null)
Expand Down
13 changes: 6 additions & 7 deletions src/Markdig/MarkdownExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -465,21 +465,20 @@ public static MarkdownPipelineBuilder UseNoFollowLinks(this MarkdownPipelineBuil

public static MarkdownPipelineBuilder UseReferralLinks(this MarkdownPipelineBuilder pipeline, params string[] rels)
{
if (!pipeline.Extensions.Contains<ReferralLinksExtension>())
if (pipeline.Extensions.TryFind(out ReferralLinksExtension? referralLinksExtension))
{
pipeline.Extensions.Add(new ReferralLinksExtension(rels));
}
else
{
var referralLinksExtension = pipeline.Extensions.Find<ReferralLinksExtension>();
foreach(string rel in rels)
foreach (string rel in rels)
{
if (!referralLinksExtension.Rels.Contains(rel))
{
referralLinksExtension.Rels.Add(rel);
}
}
}
else
{
pipeline.Extensions.Add(new ReferralLinksExtension(rels));
}
return pipeline;
}

Expand Down
18 changes: 13 additions & 5 deletions src/Markdig/Renderers/Html/Inlines/AutolinkInlineRenderer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,22 @@ public bool AutoRelNoFollow
}
set
{
string rel = "nofollow";
if (value && !Rel.Contains(rel))
const string NoFollow = "nofollow";

if (value)
{
Rel = string.IsNullOrEmpty(Rel) ? rel : Rel + $" {rel}";
if (string.IsNullOrEmpty(Rel))
{
Rel = NoFollow;
}
else if (!Rel!.Contains(NoFollow))
Copy link
Collaborator Author

@MihaZupan MihaZupan Mar 11, 2021

Choose a reason for hiding this comment

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

This ! isn't needed on Core 3.1+, sadly older runtimes aren't smart enough to realize string.IsNullOrEmpty is a null check

Copy link
Contributor

@iamcarbon iamcarbon Mar 11, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

This logic needs reworked to correctly handle null.

Copy link
Collaborator Author

@MihaZupan MihaZupan Mar 11, 2021

Choose a reason for hiding this comment

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

It does handle it via string.IsNullOrEmpty

Copy link
Contributor

@iamcarbon iamcarbon Mar 11, 2021

Choose a reason for hiding this comment

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

Indeed. Missed the outer logic changes here. 👍

{
Rel += $" {NoFollow}";
}
}
else if(!value && Rel.Contains(rel))
else
{
Rel = Rel.Replace(rel, string.Empty);
Rel = Rel?.Replace(NoFollow, string.Empty);
}
}
}
Expand Down