Unexpected sourcepos.

[ttakuru88]

Hi. "sourcepos" is exactly what I was looking for.
What do you think of this result?

markdown

[AB
CD](/)

my expected

<p data-sourcepos="1:1-2:6"><a data-sourcepos="1:1-2:6" href="/">AB
CD</a></p>

comrak v0.18.0

<p data-sourcepos="1:1-2:6"><a data-sourcepos="2:1-2:6" href="/">AB
CD</a></p>

---

Similar pattern.

markdown

- A
[![B](/B.png)](/B)

comrak

<ul data-sourcepos="1:1-2:18">
<li data-sourcepos="1:1-2:18">A
<a data-sourcepos="2:3-2:20" href="/B"><img data-sourcepos="2:4-2:15" src="/B.png" alt="B" /></a></li>

cmark

<ul data-sourcepos="1:1-2:18">
<li data-sourcepos="1:1-2:18">A
<a href="/B"><img src="/B.png" alt="B" /></a></li>
</ul>

[kivikakk]

That is a bit surprising! It's actually almost per upstream. Here's cmark-gfm:

$ cat o
[AB
CD](/)

$ build/src/cmark-gfm o -t xml --sourcepos
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE document SYSTEM "CommonMark.dtd">
<document sourcepos="1:1-2:6" xmlns="http://commonmark.org/xml/1.0">
  <paragraph sourcepos="1:1-2:6">
    <link sourcepos="2:1-2:6" destination="/" title="">
      <text sourcepos="1:2-1:3" xml:space="preserve">AB</text>
      <softbreak />
      <text sourcepos="2:1-2:2" xml:space="preserve">CD</text>
    </link>
  </paragraph>
</document>

$ build/src/cmark-gfm o --sourcepos
<p data-sourcepos="1:1-2:6"><a href="/">AB
CD</a></p>

So, in terms of the AST, comrak is doing the same thing as upstream, as shown in <link sourcepos="2:1-2:6"; the difference is that upstream doesn't actually put sourcepos on <a> in output HTML at all.

I'd be happy with fixing this in either direction; either fix how we record sourcepos on the link node, or stop outputting in it HTML. (It's possible that "fixing" the former might have unintended consequences.)

[ttakuru88]

I see. I prefer the latter(stop outputting).
I don't think writing logic that has nothing to do with the upstream is a good idea since it will make things complex.

[kivikakk]

Me too! PRs happily accepted; I don't have the time for this right now myself.

[digitalmoksha]

Wait...the idea is to not have sourcepos on the <a>? Are you wanting to remove from other non-block html? Please don't...

I tried running it against markdown-it.rs, I get what was expected

<p data-sourcepos="1:1-2:6"><a data-sourcepos="1:1-2:6" href="/">AB
CD</a></p>

[kivikakk]

In general, I'd rather "matches the upstream we explicitly follow" than not. Right now we have "doesn't match upstream and is incorrect". I'd accept a solution either way.

[digitalmoksha]

But it does match the upstream, doesn't it? Block-level sourcepos works correctly I think.

Inline sourcepos is a great enhancement. I'd certainly rather see a fix, but if that's not feasible at the moment, then how about an option to enable it? Those that want it can enable it, and work on a fix. comrak is already going beyond what upstream offers (thankfully) while trying to maintain backward compatibility. So I think ripping it out should be last resort.

[kivikakk]

Funny thing I found while getting to work on fixing this. I tried pasting the last example, i.e. this:

- A
[![B](/B.png)](/B)

directly into a GitHub comment. Imagine my surprise:

• A
  

[kivikakk]

#439 fixes this, but only by no longer reporting sourcepos information for inlines in output HTML. (Block-level is unaffected, and the data's still there in the AST or XML output if you want it.)

See #439 (comment) for comments on effort required to get inline sourcepos working consistently.

[digitalmoksha]

@kivikakk ouch. This is a huge breaking change for us. We've built significant functionality on top of inline sourcepos. Its removal is a real problem for us.

I understand it's not 100% accurate, but it's been pretty darn accurate for most things.

Also releasing it along with the other great fixes you just put in means we can't use them.

Not sure what I'm going to do yet. Probably the only thing I can do is build our own HTML outputter. Or if an inline-sourcepos option was added to re-enable it, obviously with the caveat that it's sometimes inaccurate.

[kivikakk]

I'd be happy to add it back in with an option, yeah! But "sometimes inaccurate" to me reads as "garbage in, garbage out"; of course, if your pipeline means those cases aren't hit, it's not such a big deal, but we can't even enumerate them all because there's some fundamental brokenness here. (It's why, I'm guessing, they aren't exposed in cmark.) That brokenness then becomes the sourcepos user's brokenness in unexpected ways.

So, happy to take an option to re-add them in the HTML output, but it needs to be called something like experimental-inline-sourcepos. I think removing it from the default sourcepos output is correct, and I'm sorry that this hurts your workflow!