commonmark / cmark

CommonMark parsing and rendering library and program in C
Other
1.61k stars 535 forks source link

Source position seems off #296

Open chriseidhof opened 5 years ago

chriseidhof commented 5 years ago

Give the following input:

X
    **p**

The cmark library reports (through cmark_node_get_start_column) that the strong node starts at line 2, column 1, and ends at column 5. I would have expected it to start at column 5. Is this a bug, or is my interpretation of start column wrong?

honghaoz commented 5 years ago

Looks like this is related to the soft breaks:

image
jgm commented 5 years ago

I'll refer this to @kivikakk who added inline source positions in #228.

chriseidhof commented 5 years ago

Thank you. I added a passing test case in #297, hopefully that makes it easier to get started (or if this is expected behavior, we can merge it as-is).

honghaoz commented 5 years ago

For more information, I found that for md like

A
  B

For the 1st pass when generating the blocks. The container block's string content already skips the spaces. So the container block node has text A\nB\n not A\n B\n, that's probably why the source position is off for the lazy continuation lines.

honghaoz commented 5 years ago

Related issue: https://github.com/commonmark/cmark/issues/204

kivikakk commented 5 years ago

This issue becomes more pronouncedly odd when you try the inverse:

  A
B

produces

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE document SYSTEM "CommonMark.dtd">
<document sourcepos="1:1-2:1" xmlns="http://commonmark.org/xml/1.0">
  <paragraph sourcepos="1:3-2:1">
    <text sourcepos="1:3-1:3" xml:space="preserve">A</text>
    <softbreak />
    <text sourcepos="2:3-2:3" xml:space="preserve">B</text>
  </paragraph>
</document>

The lazy continuation means we end up treating everything as though it started at column 3.

honghaoz commented 5 years ago

@kivikakk I think that's because the first line has two leading spaces. If you try to change it to 3 or 1, you will see the start column of following lines changed.

I briefly went through the parsing logic: https://github.com/commonmark/cmark/blob/master/src/blocks.c#L706 this line advances the parser->offset and parser->column for the 2nd line.

https://github.com/commonmark/cmark/blob/master/src/blocks.c#L186-L187 this line only adds the skipped text to the container node.

For example:

  A
  B

The text of the paragraph node is changed from A\n B\n to A\nB\n

jgm commented 5 years ago

Seems to me that the inline parser should receive a list of starting columns for each input line, which it can use to adjust source positions. This would avoid the need to strip whitespace (as in #298) which imposes a performance penalty. This adds a bit of complexity, which is why I didn't implement inline source positions originally.

bibo38 commented 1 year ago

I would also like to see this feature, as it would allow me to easily extract sections from a bigger markdown file. E.g. API descriptions, where I wanna parse the general format of the documentation (API call definitions, parameter tables), but leave the description in markdown to be processed later on (if even necessary).