removed async code, refactored code, some fixes #15

Alex-Dobrynin · 2025-02-11T21:15:12Z

everything in the title, but also fixed headings, if there are some headings with the same name, so hyperlinks should increment each time

Closes #14

Toine-db · 2025-02-13T09:32:22Z

src/Plugin.Maui.MarkdownView/Common/StringHelper.cs

    }

-    public static string RemoveSpecialCharactersExcept(this string input, char[] allowedSpecialChars)
+    public static bool TryCreateUri(this string path, out Uri? uri)


I know this does almost the same thing, I even used it myself, but for readabilty I used a simple http checker just because theoreticly you can create Uri without http and we don't want that in this case.

Toine-db · 2025-02-13T09:33:38Z

src/Plugin.Maui.MarkdownView/Common/StringHelper.cs


-        return sb.ToString();
+    public static string RemoveSpecialCharactersExcept(this string input, params char[] allowedSpecialChars)


funny that I had just removed this almost exact same code a few months ago, again for readabilty and it didn't have performance gains in this case

Toine-db · 2025-02-13T09:35:36Z

src/Plugin.Maui.MarkdownView/Common/HyperlinkHelper.cs

@@ -7,10 +8,8 @@ public static class HyperlinkHelper
 {
    public static bool IsHeadingLink(string hyperlink)
    {


for readabilty (and debugging) using a local var has some pros, without having any cons because the compiler will optimize it later on.

Toine-db · 2025-02-13T09:35:58Z

src/Plugin.Maui.MarkdownView/Common/HyperlinkHelper.cs

+                    linkedHeader = markdownView
+                        .GetRootChildren()
+                        .OfType<HeaderLabel>()
+                        .FirstOrDefault(x => hyperlink.Equals(x.HeadingId, StringComparison.OrdinalIgnoreCase));


good point, will use this

Toine-db · 2025-02-13T09:36:55Z

src/Plugin.Maui.MarkdownView/Plugin.Maui.MarkdownView.csproj

-	</ItemGroup>
-
-</Project>
+</Project>


Thanks for showing that somehow strange packages where included

Toine-db · 2025-02-13T09:40:27Z

src/Plugin.Maui.MarkdownView/Common/HyperlinkHelper.cs

        try
        {
            var parent = hyperlinkControl.Parent;
            IView? linkedHeader = null;

            // Search for Header with HeadingId
-            while (linkedHeader == null
-                   && parent != null)
+            while (linkedHeader is null


I prefere using comparison operators when comparing stuff, for readabilty, in my case it provides some extra contrast because of the difference between alphabet characters and operators symbols.

But this is really personal, no cons or pros on this one. Sorry, gona keep it this way for now.

Toine-db · 2025-02-13T09:46:23Z

src/Plugin.Maui.MarkdownView/ViewSuppliers/MauiBasicViewSupplier.cs


-        return absolutePath;
+        // Fallback to path combination with normalized slashes
+        return string.Format("{0}/{1}",


I never try to have a competition to get as much as possible on 1 line, lines are free and are optimized away by the compiler.

Thats why I always try to do 1 action on each line of code (if the action is not to small).
Again....for readabilty, you probably notice that I'm really keen on readabilty ;-)

Good code should read as a book, a child should be able to read an understand it....(slightly exaggerated)

Toine-db · 2025-02-13T09:46:49Z

src/Plugin.Maui.MarkdownView/ViewSuppliers/MauiFormattedTextViewSupplier.cs

@@ -18,10 +20,10 @@ public class MauiFormattedTextViewSupplier : MauiBasicViewSupplier
        var textViewStyle = GetTextBlockStyleFor(textBlock);
        var spanStyle = GetSpanStyleFor(textBlock);

-        var collectedRootViews = new List<View?>();
-        var textSpansCache = new List<Span>();
+        List<View?> collectedRootViews = [];


This indeed improves readabilty +1

Toine-db · 2025-02-13T10:03:21Z

src/Plugin.Maui.MarkdownView/ViewSuppliers/MauiBasicViewSupplier.cs

+            .RemoveSpecialCharactersExcept('-')
+            .ToLower();
+
+        if (Headings.TryGetValue(headingId, out var list))


This is a solution for something thats not really an issue. When someone uses same headers for multiple sections and it links to it afterwards, strange by the way, he should be able to imagine that the system will not know which one was intended. Because the system won't crash or five any other error this won't be a real issue, only bad text / markdown writing.

With including a number you are creating dynamic magical strings, what will never be linked to by the markdown writer as well.

Toine-db · 2025-02-13T10:05:37Z

src/Plugin.Maui.MarkdownView/ViewSuppliers/MauiBasicViewSupplier.cs

@@ -140,15 +140,22 @@ public virtual void OnReferenceDefinitionsPublished(IEnumerable<MarkdownReferenc
                Style = Styles?.ImageSubscriptionViewStyle,
                Text = subscription
            };
+            var stackLayout = new VerticalStackLayout
+            {
+                Style = Styles?.ImageLayoutViewStyle,


I see what you are trying to do here, but the ImageLayoutViewStyle was also intended as an extra layer for the developer/designer to style optionaly because images can be hard to style or position sometimes.
Thats why I desided to always include a ImageLayoutView and give it a style, regardless of whether there is 1 or more children.

Toine-db · 2025-02-13T10:27:20Z

src/Plugin.Maui.MarkdownView/ViewSuppliers/MauiFormattedTextViewSupplier.cs

@@ -255,7 +257,7 @@ protected virtual async void OnHyperlinkTappedAsync(HyperlinkSpan hyperlinkSpan)

    protected virtual View? TryCreateFormattedLabel(List<Span> textSpans, Style? style)
    {
-        if (!textSpans.Any())
+        if (textSpans.Count == 0)


fyi: Any is faster than Count, but almost negligible....so the only reason why I use Any is for Readability preference

Toine-db · 2025-02-13T10:32:20Z

src/Plugin.Maui.MarkdownView/ViewSuppliers/MauiFormattedTextViewSupplier.cs


        switch (validViews.Count)
        {
            case 0:
                return null;
            case 1:
-                return validViews.First();
+                return validViews[0];


this won't be faster when using lists, because lists uses indexers etc with First().
When using [0] approuch you probably are better of with ToArray() on the top, than it will be faster in the end.

but in the end the difference won't be noticeable.....just fyi ;-)

Toine-db · 2025-02-13T10:40:54Z

src/Plugin.Maui.MarkdownView/ViewSuppliers/MauiBasicViewSupplier.cs

        {
            return url;
        }

-        var isAbsolutePath = Uri.TryCreate(url, UriKind.Absolute, out _);
-        if (isAbsolutePath)
+        if (Uri.IsWellFormedUriString(url, UriKind.Absolute))


I have used this previously, but it filtered more than desired....in this case

Toine-db · 2025-02-13T10:45:31Z

src/Plugin.Maui.MarkdownView/MarkdownView.cs

-    private CancellationTokenSource? _loadingCts;
+    protected readonly ILogger<MarkdownView>? Logger;
+    protected readonly VerticalStackLayout Container;
+    protected CancellationTokenSource? LoadingCts;


most likely everything is handled on the UI thread with your approuch, so this token will never be used

Toine-db · 2025-02-13T10:51:44Z

src/Plugin.Maui.MarkdownView/MarkdownView.cs

            foreach (var view in views)
            {
-                loadingToken.ThrowIfCancellationRequested();
-                _layout.Add(view);
+                if (token.IsCancellationRequested) return;


ThrowIfCancellationRequested has some pros and is designed to work with the Task and Cancelation system, I recommend using that. But this is not used in a Task, so it matters less and will probably never be used anyway.

It is good to remember that when a Method is finished without errors, like a return does, you expect it to have gone well. This way you don't have an indication that something has caused a cancellation.

removed async code, refactored code, some fixes

a57b99e

Toine-db reviewed Feb 13, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

removed async code, refactored code, some fixes #15

removed async code, refactored code, some fixes #15

Alex-Dobrynin commented Feb 11, 2025

Toine-db Feb 13, 2025

Toine-db Feb 13, 2025

Toine-db Feb 13, 2025

Toine-db Feb 13, 2025

Toine-db Feb 13, 2025

Toine-db Feb 13, 2025

Toine-db Feb 13, 2025

Toine-db Feb 13, 2025

Toine-db Feb 13, 2025

Toine-db Feb 13, 2025

Toine-db Feb 13, 2025

Toine-db Feb 13, 2025

Toine-db Feb 13, 2025

Toine-db Feb 13, 2025

Toine-db Feb 13, 2025


		return sb.ToString();
		public static string RemoveSpecialCharactersExcept(this string input, params char[] allowedSpecialChars)

removed async code, refactored code, some fixes #15

Are you sure you want to change the base?

removed async code, refactored code, some fixes #15

Conversation

Alex-Dobrynin commented Feb 11, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment