Implemented interface for text-links

[FaFre]

Just added a small interface to make it more comfortable to work with text links from outside.

[CXuesong]

Thanks for your PR! I got your point, but when thinking about your goal twice, I'm wondering if it makes much contribution. Before going through my justifications, could you please share with me a bit with your actual usage scenario? Perhaps we can find another way to make your days better 😀

The major issue here is that

• ITextLink.Target does not have a self-contained definition.
• ITextLink.Text is trivial as you can always use ToPlainText to retrieve the plain text representation of an arbitary node
  • if you have an actual case where you need to extract the Run representation of a certain link, then please state your usage scenario so I can look into it.

For Target property, suppose you have an arbitrary instance of ITextLink, you cannot really tell what target this Target is. Consider members of some other interfaces like IServiceProvider.GetService or ICollection.Add. They have self-contained definition, i.e. you can declare that certain class has certain trait by implementing some interface. But for ITextLink, I can see no such a trait except that "the implementation of ITextLink has a Target property that returns Run".

Then what you are proposing is basically a "Type Class" (c.f. dotnet/csharplang#110, or interface in TypeScript). Such kind of feature is useful especially because it does not require the classes to explicit implement an interface. As mentioned at the very beginning of Interfaces section of TS handbook

One of TypeScript’s core principles is that type checking focuses on the shape that values have. This is sometimes called “duck typing” or “structural subtyping”. In TypeScript, interfaces fill the role of naming these types, and are a powerful way of defining contracts within your code as well as contracts with code outside of your project. [1]

But please also note that C# originated from the concept of OOP. I suggest that for now we do not add the interface until we can see there is enough common traits that we can abstract based on, since OOP is representing objects with abstrations (classes and interfaces).

[FaFre]

Thank you for the detailed response! You made me overthink some stuff. I'm extracting all links from a page and store them inside a collection. To make it easier I introduced ITextLink after reading over IInlineContainer. Now, I refactored my code to handle links in a wrapper-class, which also generates the full URL's for the wiki-internal links. I reverted the changes from this PR locally, the benefits of this contribution are probably quite low as you said.

[CXuesong]

Sometimes the line is pretty blurred. This is why I thought it could be helpful if you can share your usage scenario.

I have been wondering all the time "how are you consuming this ITextLink.Target without knowing the type of the ITextLink implementation?" I guess you should be aware of whether the link is wikilink or external link...

Now, I refactored my code to handle links in a wrapper-class, which also generates the full URL's for the wiki-internal links.

Okay so with some C# 8 language features (basic pattern matching), you can write something like this

(string text, string target) ParseLink(InlineNode linkNode) {
    switch (someNode) {
        case WikiLink wikilink:
            return (wikilink.Text.ToString(), GetFullUrl(wikilink.Target.ToString());
        case ExternalLink externalLink:
            return (externalLink.Text.ToString(), externalLink.Target.ToString());
        default:
            throw new ArgumentException("Expect link node.", nameof(linkNode));
}

Note that I'm using InlineNode for the input parameter, which is the nearest common ancestor of WikiLink and ExternalLink. We can do nothing more than that and that's the inherent limiation of the current C# language. TypeScript supports type union so we can write something like this

parseLink(linkNode: WikiLink | ExternalLink): [text: string, target: string]

But no, not for C#. We need to wait for dotnet/csharplang#3737, instead of introducing interfaces only in order to represent such type union rather than an abstraction of certain trait. Otherwise things could get very awkward.

As for IInlineContainer, it is an abstract class at the beginning until cc4475a. When writing InlineContainer class, I wanted something (abstract class / interface) to represent "something that contains InlineNodes", because I do need some methods that can prepend/append InlineNode (e.g. some plain text) to an existing ListItem, Heading, Paragraph, Run, etc.

MwParserFromScratch/MwParserFromScratch/Nodes/Wikitext.cs

Lines 78 to 106 in 5f47c6f

	/// <summary>
	/// Append a <see cref="PlainText"/> node to the beginning of the paragraph.
	/// </summary>
	/// <param name="text">The text to be inserted.</param>
	/// <returns>Either the new <see cref="PlainText"/> node inserted, or the existing <see cref="PlainText"/> at the beginning of the paragraph.</returns>
	/// <exception cref="ArgumentNullException"><paramref name="text"/> is <c>null</c>.</exception>
	public static PlainText Prepend(this IInlineContainer container, string text)
	{
	if (text == null) throw new ArgumentNullException(nameof(text));
	var pt = container.Inlines.FirstNode as PlainText;
	if (pt == null) container.Inlines.AddFirst(pt = new PlainText());
	pt.Content = text + pt.Content;
	return pt;
	}
	/// <summary>
	/// Append a <see cref="PlainText"/> node to the end of the paragraph.
	/// </summary>
	/// <param name="text">The text to be inserted.</param>
	/// <returns>Either the new <see cref="PlainText"/> node inserted, or the existing <see cref="PlainText"/> at the end of the paragraph.</returns>
	/// <exception cref="ArgumentNullException"><paramref name="text"/> is <c>null</c>.</exception>
	public static PlainText Append(this IInlineContainer container, string text)
	{
	if (text == null) throw new ArgumentNullException(nameof(text));
	var pt = container.Inlines.LastNode as PlainText;
	if (pt == null) container.Inlines.Add(pt = new PlainText());
	pt.Content += text;
	return pt;
	}

In the implementation of the methods above, I care about what are in the InlineNode collection of the nodes instead of exactly what type the nodes is. It looks very similar to "duck type", but there is a self-contained definition of IInlineContainer.Inlines. Because it's simply "a collection of InlineNodes".

If your usage scenario is similar to this, introducing an interface should help. But then that interface could be IRunContainer, which does not even aware whether the Run is a Target (wikilink / external link) or something else. And it does not make much contribution, either, as there is currently no downstream requirement for such an interface.

[FaFre]

Yes, I also thought about the detection of the actual link-type, since the target is handled completely different. A interface would be not a clear approach here, that's also why I wrote a wrapper class, I'm much more happy with this solution now. Thanks for taking your time to explain all those things!