Skip to content
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.

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

Wrong SourceSpan with lazy continuation lines #337

Closed
sylvainheraud opened this issue Sep 10, 2024 · 5 comments · Fixed by #339
Closed

Wrong SourceSpan with lazy continuation lines #337

sylvainheraud opened this issue Sep 10, 2024 · 5 comments · Fixed by #339
Labels

Comments

@sylvainheraud
Copy link

sylvainheraud commented Sep 10, 2024

Hi !
I'm using commonmark-java to highlight delimiters in a RichTextFX editor, and using SourceSpan to compute positions.
I have an issue with 2 examples given by commonmark specification.

Example 292

> 1. > Blockquote
continued here.

Actual behavior (printing SourceSpan in visitChildren)

  • Text{literal=Blockquote} [SourceSpan{line=0, column=7, length=10}]
  • SoftLineBreak{} []
  • Text{literal=continued here.} [SourceSpan{line=1, column=0, length=15}]
  • Paragraph{} [SourceSpan{line=0, column=7, length=10}, SourceSpan{line=1, column=0, length=15}]
  • BlockQuote{} [SourceSpan{line=0, column=5, length=12}, SourceSpan{line=1, column=5, length=10}]
  • ListItem{} [SourceSpan{line=0, column=2, length=15}, SourceSpan{line=1, column=2, length=13}]
  • OrderedList{} [SourceSpan{line=0, column=2, length=15}, SourceSpan{line=1, column=2, length=13}]
  • BlockQuote{} [SourceSpan{line=0, column=0, length=17}, SourceSpan{line=1, column=0, length=15}]
  • Document{} []

I don't understand sourceSpans or the second line. It seems to me that BlockQuote/ListItem/OrderedList should start at column 0 for line 1.

Example 250

> > > foo
bar
  • Text{literal=foo} [SourceSpan{line=0, column=6, length=3}]
  • SoftLineBreak{} []
  • Text{literal=bar} [SourceSpan{line=1, column=0, length=3}]
  • Paragraph{} [SourceSpan{line=0, column=6, length=3}, SourceSpan{line=1, column=0, length=3}]
  • BlockQuote{} [SourceSpan{line=0, column=4, length=5}, SourceSpan{line=1, column=4, length=-1}]
  • BlockQuote{} [SourceSpan{line=0, column=2, length=7}, SourceSpan{line=1, column=2, length=1}]
  • BlockQuote{} [SourceSpan{line=0, column=0, length=9}, SourceSpan{line=1, column=0, length=3}]
  • Document{} []

I don't understand sourceSpan for the 3 BlockQuotes, some of them have negative length. It seems to me that all BlockQuotes should start at column 0 for line 1.

And If I add a character after bar

> > > foo
bars
  • Text{literal=foo} [SourceSpan{line=0, column=6, length=3}]
  • SoftLineBreak{} []
  • Text{literal=bars} [SourceSpan{line=1, column=0, length=4}]
  • Paragraph{} [SourceSpan{line=0, column=6, length=3}, SourceSpan{line=1, column=0, length=4}]
  • BlockQuote{} [SourceSpan{line=0, column=4, length=5}] MISSING SOURCE SPACE?
  • BlockQuote{} [SourceSpan{line=0, column=2, length=7}, SourceSpan{line=1, column=2, length=2}]
  • BlockQuote{} [SourceSpan{line=0, column=0, length=9}, SourceSpan{line=1, column=0, length=4}]
  • Document{} []

In this case, I lose the deepest BlockQuote sourceSpan for line 1

Thank you for your help.

@robinst
Copy link
Collaborator

robinst commented Sep 13, 2024

Yeah, this is a bug with lazy continuation lines, thanks for reporting. I'm planning to look into it before the next release, but if you want to help speed things up you could write a few test cases here:

@sylvainheraud
Copy link
Author

Hi Robin, thank you for your answer. Please found the test cases.

            // From https://spec.commonmark.org/0.31.2/#example-250
            // Wrong source span for the inner block quote for the second line.
            Node document = PARSER.parse("> > > foo\nbar\n");

            BlockQuote blockQuote1 = (BlockQuote) document.getLastChild();
            assertEquals(List.of(SourceSpan.of(0, 0, 9), SourceSpan.of(1, 0, 3)), blockQuote1.getSourceSpans());
            BlockQuote blockQuote2 = (BlockQuote) blockQuote1.getLastChild();
            assertEquals(List.of(SourceSpan.of(0, 2, 7), SourceSpan.of(1, 0, 3)), blockQuote2.getSourceSpans());
            BlockQuote blockQuote3 = (BlockQuote) blockQuote2.getLastChild();
            assertEquals(List.of(SourceSpan.of(0, 4, 5), SourceSpan.of(1, 0, 3)), blockQuote3.getSourceSpans());
            Paragraph paragraph = (Paragraph) blockQuote3.getLastChild();
            assertEquals(List.of(SourceSpan.of(0, 6, 3), SourceSpan.of(1, 0, 3)), paragraph.getSourceSpans());
            // Adding one character to the last line remove blockQuote3 source for the second line
            Node document = PARSER.parse("> > > foo\nbars\n");

            BlockQuote blockQuote1 = (BlockQuote) document.getLastChild();
            assertEquals(List.of(SourceSpan.of(0, 0, 9), SourceSpan.of(1, 0, 4)), blockQuote1.getSourceSpans());
            BlockQuote blockQuote2 = (BlockQuote) blockQuote1.getLastChild();
            assertEquals(List.of(SourceSpan.of(0, 2, 7), SourceSpan.of(1, 0, 4)), blockQuote2.getSourceSpans());
            BlockQuote blockQuote3 = (BlockQuote) blockQuote2.getLastChild();
            assertEquals(List.of(SourceSpan.of(0, 4, 5), SourceSpan.of(1, 0, 4)), blockQuote3.getSourceSpans());
            Paragraph paragraph = (Paragraph) blockQuote3.getLastChild();
            assertEquals(List.of(SourceSpan.of(0, 6, 3), SourceSpan.of(1, 0, 4)), paragraph.getSourceSpans());
            // From https://spec.commonmark.org/0.31.2/#example-292
            Node document = PARSER.parse("> 1. > Blockquote\ncontinued here.");

            BlockQuote blockQuote1 = (BlockQuote) document.getLastChild();
            assertEquals(List.of(SourceSpan.of(0, 0, 17), SourceSpan.of(1, 0, 15)), blockQuote1.getSourceSpans());
            OrderedList orderedList = (OrderedList) blockQuote1.getLastChild();
            assertEquals(List.of(SourceSpan.of(0, 2, 15), SourceSpan.of(1, 0, 15)), orderedList.getSourceSpans());
            ListItem listItem = (ListItem) orderedList.getLastChild();
            assertEquals(List.of(SourceSpan.of(0, 2, 15), SourceSpan.of(1, 0, 15)), listItem.getSourceSpans());
            BlockQuote blockQuote2 = (BlockQuote) listItem.getLastChild();
            assertEquals(List.of(SourceSpan.of(0, 5, 12), SourceSpan.of(1, 0, 15)), blockQuote2.getSourceSpans());
            Paragraph paragraph = (Paragraph) blockQuote2.getLastChild();
            assertEquals(List.of(SourceSpan.of(0, 7, 10), SourceSpan.of(1, 0, 15)), paragraph.getSourceSpans());

@robinst
Copy link
Collaborator

robinst commented Sep 14, 2024

Thanks for these! I've raised a fix for it here: #339

@sylvainheraud
Copy link
Author

Thank you, @robinst !

@robinst
Copy link
Collaborator

robinst commented Sep 19, 2024

This has been released in 0.23.0, see full changelog here: https://github.com/commonmark/commonmark-java/blob/main/CHANGELOG.md#0230---2024-09-16

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants