Improved Multi-Line Quoting, you can now separate two quotes. #1974

WilliamABradley · 2018-04-14T04:24:10Z

-Updated InitialContent with some examples.

Testing with:

>Quote1

>Quote2.1
>>Quote2.Nest1.1
>>
>>Quote2.Nest1.2
>
>Quote2.3

>Quote3.1
>Quote3.2

>Quote4.1
>
>Quote4.2

>Quote5.1
Quote5.2

>Quote6

PR Type

What kind of change does this PR introduce?

Bugfix

What is the current behavior?

What is the new behavior?

PR Checklist

Please check if your PR fulfills the following requirements:

Tested code with current supported SDKs
Docs have been added/updated which fit documentation template. (for bug fixes / features)
Sample in sample app has been added / updated (for bug fixes / features)
- ~~Icon has been created (if new sample) following the Thumbnail Style Guide and templates~~
Tests for the changes have been added (for bug fixes / features) (if applicable)
Header has been added to all new source files (run build/UpdateHeaders.bat)
Contains NO breaking changes

Other information

The current implementation is a bit of a selection of different styles, as all Renderers differ in their results, this discussion can be continued to determine a better conformance if need be:

StackEdit
GitHub
CommonMark.js
Reddit
Dillinger.io
VS Code

-Updated InitialContent with some examples.

WilliamABradley · 2018-04-14T04:26:59Z

It is one line away from CommonMark conformance, but it means that a Quote can't go back to indentation 1 after going up to indentation 2.

Vijay-Nirmal · 2018-04-14T04:38:59Z

I think CommonMark is the expected output.

@WilliamABradley You can come back to outer Quote. Try this

> Quote2.1
>>Quote2.Nest1.1
 >Quote2.Nest1.2
>
> Quote2.2

Output

And multiline for nested Quote would be

> Quote2.1
>>Quote2.Nest1.1
 >Quote2.Nest1.2
>>
>>Quote2.Nest1.3
>
>Quote2.2

Output

michael-hawker · 2018-04-14T04:45:52Z

Microsoft.Toolkit.Parsers/Markdown/MarkdownDocument.cs

+                            else if (paragraphText.Length > 0 && ((markdown[startOfLine - 2] == '>' || markdown[startOfLine - 1] == '>') || inQuoteNewLine))
+                            {
+                                // If the start of the line is a QuoteBlock, and the Paragraph has already been started, Create a new line.
+                                paragraphText.Append("\r\n");


Should this be Environment.NewLine?

michael-hawker · 2018-04-14T04:47:42Z

Microsoft.Toolkit.Parsers/Markdown/MarkdownDocument.cs

@@ -96,6 +96,9 @@ internal static List<MarkdownBlock> Parse(string markdown, int start, int end, i
            int previousStartOfLine = start;


The parse method seems really large, do we have a work item to split it up into other methods that we can more easily unit test?

It would likely happen if we go through with #1767

michael-hawker · 2018-04-14T04:48:56Z

Microsoft.Toolkit.Parsers/Markdown/MarkdownDocument.cs

@@ -154,7 +157,16 @@ internal static List<MarkdownBlock> Parse(string markdown, int start, int end, i
                        // But it doesn't matter if this is not the start of a new paragraph.
                        if (!lineStartsNewParagraph || nonSpaceChar == '\0')
                        {
-                            break;
+                            // Continue the Quote if the previous line was a quote.
+                            if (((previousStartOfLine - 2) >= 0 && markdown[previousStartOfLine - 2] == '>') || ((previousStartOfLine - 1) >= 0 && markdown[previousStartOfLine - 1] == '>'))


Are spaces allowed before the >? Or is there a trim somewhere?

michael-hawker · 2018-04-14T04:49:18Z

Microsoft.Toolkit.Parsers/Markdown/MarkdownDocument.cs

@@ -277,6 +289,12 @@ internal static List<MarkdownBlock> Parse(string markdown, int start, int end, i
                                paragraphText[paragraphText.Length - 2] = '\r';
                                paragraphText[paragraphText.Length - 1] = '\n';
                            }
+                            else if (paragraphText.Length > 0 && ((markdown[startOfLine - 2] == '>' || markdown[startOfLine - 1] == '>') || inQuoteNewLine))


Do we need a bounds check here as well?

WilliamABradley · 2018-04-14T08:17:09Z

I updated my test markdown, and I have been attempting to mimic CommonMark, it is nearly done, however, I am struggling to get the Parser to go back out of the nest after a nest, as you can see from the new behavior image.

Quote2.3 should be part of the rest of Quote2.

nmetulev · 2018-04-16T03:27:28Z

Microsoft.Toolkit/Extensions/StringExtensions.cs

+        /// <param name="searchString">String to search through</param>
+        /// <param name="toFind">Substring to find</param>
+        /// <returns>Number of times the substring occurs</returns>
+        public static int Occurrences(this string searchString, string toFind)


Not sure this method is necessary.

Also, I'm not sure it does what it's being documented. Why are you checking every character in toFind with every character in searchString?

Yeah, the current logic only works in the single character case (which is how it's used in the PR). If we want a function like this, we should add unit tests and make sure it works for the other scenarios. Though it'd probably be easier/faster to use the built-in IndexOf or Split methods to do this, no?

WilliamABradley · 2018-05-06T00:20:18Z

@michael-hawker @nmetulev Fixed the CommonMark conformance issue, this is now ready for review.

Vijay-Nirmal

Line Break is not working when we have nested quotes.

>> Quote 1

>> Quote 2

This is displayed as

But It should be displayed as

WilliamABradley · 2018-05-09T12:52:02Z

@Vijay-Nirmal fixed that issue, let me know if you discover any more. It is pretty mind bending logic.

nmetulev · 2018-05-13T21:24:47Z

@Vijay-Nirmal, up to you now.

Improved Multi-Line Quoting, you can now separate two quotes.

25e81da

-Updated InitialContent with some examples.

WilliamABradley added the improvements ✨ label Apr 14, 2018

WilliamABradley added this to the 3.0 milestone Apr 14, 2018

WilliamABradley self-assigned this Apr 14, 2018

WilliamABradley requested a review from michael-hawker April 14, 2018 04:24

Fixed DocFX Quote Issue.

4835341

michael-hawker reviewed Apr 14, 2018

View reviewed changes

Added more CommonMark Quote Block conformance.

0011b33

Removed namespace.

bfb91fb

nmetulev requested changes Apr 16, 2018

View reviewed changes

nmetulev added reviewers wanted parsers 🌱 labels Apr 16, 2018

WilliamABradley added 2 commits May 6, 2018 11:09

Merge branch 'master' into Markdown-QuoteBlock-join-fix

b55a72e

Removed StringExtension, now fully aligned with CommonMark.

59a2592

WilliamABradley added 2 commits May 6, 2018 12:24

Fixed Comment.

99e8ba9

Removed Unnecessary Code.

f2218f9

michael-hawker approved these changes May 6, 2018

View reviewed changes

Vijay-Nirmal suggested changes May 6, 2018

View reviewed changes

Fixed CurrentIndentation = 0, LastIndentation > 1.

6d68829

nmetulev approved these changes May 13, 2018

View reviewed changes

Vijay-Nirmal approved these changes May 14, 2018

View reviewed changes

Vijay-Nirmal and others added 2 commits May 14, 2018 23:03

Merge branch 'master' into Markdown-QuoteBlock-join-fix

117bbcd

Merge branch 'master' into Markdown-QuoteBlock-join-fix

a9858aa

nmetulev merged commit da110ef into CommunityToolkit:master May 15, 2018

WilliamABradley mentioned this pull request Dec 15, 2019

Add Custom Markdown Parser Block and Inline Injection #1767

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improved Multi-Line Quoting, you can now separate two quotes. #1974

Improved Multi-Line Quoting, you can now separate two quotes. #1974

WilliamABradley commented Apr 14, 2018 •

edited

Loading

WilliamABradley commented Apr 14, 2018

Vijay-Nirmal commented Apr 14, 2018

michael-hawker Apr 14, 2018

michael-hawker Apr 14, 2018

WilliamABradley Apr 14, 2018

michael-hawker Apr 14, 2018

michael-hawker Apr 14, 2018

WilliamABradley commented Apr 14, 2018

nmetulev Apr 16, 2018

nmetulev Apr 16, 2018

michael-hawker Apr 26, 2018

WilliamABradley May 6, 2018

WilliamABradley commented May 6, 2018

Vijay-Nirmal left a comment •

edited

Loading

WilliamABradley commented May 9, 2018

nmetulev commented May 13, 2018

		@@ -96,6 +96,9 @@ internal static List<MarkdownBlock> Parse(string markdown, int start, int end, i
		int previousStartOfLine = start;

Improved Multi-Line Quoting, you can now separate two quotes. #1974

Improved Multi-Line Quoting, you can now separate two quotes. #1974

Conversation

WilliamABradley commented Apr 14, 2018 • edited Loading

PR Type

What is the current behavior?

What is the new behavior?

PR Checklist

Other information

WilliamABradley commented Apr 14, 2018

Vijay-Nirmal commented Apr 14, 2018

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

WilliamABradley commented Apr 14, 2018

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

WilliamABradley commented May 6, 2018

Vijay-Nirmal left a comment • edited Loading

Choose a reason for hiding this comment

WilliamABradley commented May 9, 2018

nmetulev commented May 13, 2018

WilliamABradley commented Apr 14, 2018 •

edited

Loading

Vijay-Nirmal left a comment •

edited

Loading