commonmark / commonmark/commonmark.js

Setext Heading LRDs take precedence over Paragraph LRDs

Open
#298 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
JavaScript
Stars
1.6k
Forks
231
Avg merge
2d 11h
Merged PRs (30d)
3

Description

The only reference to precedence I could find in the spec is this:

If there are multiple matching reference link definitions, the one that comes first in the document is used. (It is desirable in such cases to emit a warning.)

The issue is that when you define a LRD (link reference definition) in a Setext heading (or a probable Setext heading*), this link will populate the refmap before any paragraph LRDs are parsed, which occurs at the document finalization step.

*A paragraph interrupted by a setext heading underline that only contains links, thus creating an empty <p>, which is immediately unlinked from the AST.

Example inputs:

[x]: fizz

[x]: buzz
===
[x]

has an href=buzz,

Whereas

[x]: fizz

[x]: buzz

[x]

has an href=fizz.

My interpretation of the spec is that both should result in href=fizz as that is the first LRD.

Assuming this is a bug, my idea of a fix would be to use a modified Parser.refmap where it has an Map<number, Array<LRD>> and the key would be the line number of the containing paragraph (sourcepos[0][0]), as the exact lines don't matter as long as the array is populated in-order.

So if this was the input

[line1]: a
[line2]: b
rest of paragraph

[line5]: e
===

would have the map

{
 5: [e],
 1: [a, b]
}

(where a, b, e are objects of typedef LRD {destination: string, title: string, label: string} with map def
Map<number, Array<LRD>>)

Then setext headings' block start and document finalization can occur in the order they do, but could add another step after document finalization. E.g.

/**
 * @param {Map<number, Array<{destination: string, title: string, label: string}>>} unorderedRefmap
 */
function getFinalRefmap(unorderedRefmap) {
  const sortedKeys = [...unorderedRefmap.keys()].sort((a, b) => a - b);
  const finalRefmap = {};
  for (const k of sortedKeys) {
    const values = unorderedRefmap.get(k);
    for (const v of values) {
      if (!finalRefmap[v.label]) {
        finalRefmap[v.label] = {destination: v.destination, title: v.title};
      }
    }
  }
  return finalRefmap ;
}

Something like that should work, its just a matter of adding to the placeholder refmap correctly, which is a bit trickier since inline.js needs a reference to it and mutates it.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Reproduce the two examples and inspect Parser.refmap, inline.js, and the document-finalization step to trace when Setext and paragraph LRDs are recorded. Verify how the proposed ordering can be applied without breaking inline.js mutations. Done means duplicate labels consistently resolve to the first LRD in document order, including Setext-heading cases.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
compilers
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.