Don't splice a link into the middle of a URL

[pranayshirolkar]

Fixes #118.

Problem

Copy a link from a native app or a clipboard tool and paste it into a GitHub comment box, and the Markdown comes out with a [ planted in the middle of the URL:

text/plain: https://github.com/owner/repo/blob/main/a.js#L7
text/html:  <a href="https://github.com/owner/repo/blob/main/a.js#L7">repo/blob/main/a.js#L7</a>

pasted:     https://github.com/owner/[repo/blob/main/a.js#L7](https://github.com/owner/repo/blob/main/a.js#L7)
expected:   [repo/blob/main/a.js#L7](https://github.com/owner/repo/blob/main/a.js#L7)

It renders as two links, one of them junk. The shape that triggers it — text/plain is the URL, text/html is an anchor labelled with a shortened rendering of that URL — is how a lot of tooling writes a link to the pasteboard (macOS automation scripts, clipboard managers, "copy link" affordances that shorten the visible text).

Cause

convertToMarkdown splices [label](href) over the label at the offset where indexOf finds it in the plain text:

https://github.com/github/paste-markdown/blob/main/src/paste-markdown-html.ts#L83-L90

That's right for prose containing a link — example link plus more text — because there the plain text is a flattening of the HTML. When the plain text is the URL and the label is a piece of it, the label is found inside the URL (offset 25 in the example above) and the splice covers only the tail.

Fix

When the label's occurrence sits inside a longer whitespace-delimited token, decide by what that token is:

• it is this link's own href — the label is a shortened rendering of the URL, so the link replaces the whole token. This produces the link the clipboard meant.
• it is some other URL — leave the paste alone. Splicing inside a URL is never right, and there is no sensible Markdown to emit.
• anything else — unchanged: the label's own occurrence is the span, so <a href="…">foo</a>bar pasted alongside foobar still yields [foo](…)bar.

areEqualLinks already handles the trailing-slash and casing differences between an anchor's normalized href and the pasted text, so the comparison reuses it.

Tests

Two added, both failing before this change and passing after (38 passed, 2 failed → 40 passed):

• links the whole url when the label is a shortened rendering of it
• doesn't splice a link inside a url that is not its own href

All existing tests pass unchanged.

[Copilot]

🟡 Changes recommended

Punctuation-wrapped URLs can still be corrupted because URL detection only checks the token’s first character.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

Prevents shortened link labels from being inserted into the middle of pasted URLs.

Changes:

• Detects URL-containing plaintext tokens and selects a safer replacement span.
• Adds regression tests for matching and mismatched URLs.

File summaries

File	Description
src/paste-markdown-html.ts	Adds link-span detection and URL safeguards.
test/test.js	Adds regression coverage for shortened labels.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Balanced

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[pranayshirolkar]

@keithamus , can you take a look at this when you get a chance?

[keithamus]

I don't work at GitHub any more (I work at Mozilla now). @primer will need to look at this instead or maybe @mattcosta7?

[Copilot]

Review details

Suppressed comments (2)

src/paste-markdown-html.ts:153

• The URL check returns before the punctuation-trimmed candidate is examined whenever the raw token starts with a scheme. Consequently, a shortened link followed by normal sentence punctuation (for example, plaintext https://github.com/owner/repo. with an href lacking the period) is declined even though lines 124–125 say this case is recognized. Check every candidate for an href match first, and only then decide that the token is some other URL.

    for (const span of [token, withoutWrappingPunctuation(markdown, token)]) {
      const text = markdown.slice(span.index, span.index + span.length)
      // The label describes this whole URL, so the link replaces the whole URL.
      if (areEqualLinks(href, text)) return span
      // Splicing inside a URL is never right, so a label inside one that is not this link's own
      // href leaves the paste alone rather than corrupting it.
      if (looksLikeURL(text)) return null

src/paste-markdown-html.ts:109

• This recognizes only hierarchical schemes containing ://, despite the function's contract covering text that starts with a scheme. A label inside mailto:user@example.com, urn:..., or another valid non-hierarchical URL still falls through and is spliced into that URL when the anchor has a different href.

function looksLikeURL(text: string): boolean {
  return /^[a-z][a-z\d+.-]*:\/\//i.test(text)

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Balanced

[pranayshirolkar]

Thanks @francinelucca — your test reproduced it. Fixed in e8f8960 (branch also rebased onto main to clear the conflict).

Closing punctuation now comes off one character at a time, longest span first, and each length is compared against the href. So (…/wiki/Ruby_(programming_language)) matches with the URL's own ) kept and only the prose's ) trimmed. Opening punctuation still comes off all at once, since a URL never starts with any.

That also covers one of the suppressed comments from the same review: every candidate is now compared to the href before the token is treated as some other URL, so a link followed by a full stop (https://…/a.js#L7.) links instead of being declined. Added links a url the sentence ends right after for it.

On the other suppressed comment (mailto:/urn: not recognized): I left looksLikeURL requiring // deliberately. A bare scheme: test would also match prose like TODO:foo, and TODO:<a>foo</a> would then be declined instead of linked. I corrected the function's comment, which wrongly promised any scheme, to say so.

44 passed; eslint, tsc and prettier --check are clean.

[francinelucca]

👋🏽 thanks for getting this going @pranayshirolkar, added a failing test for #117 (comment), can you take a look?

[pranayshirolkar]

👋🏽 thanks for getting this going @pranayshirolkar, added a failing test for #117 (comment), can you take a look?

@francinelucca done — fixed in e8f8960 and rebased onto main; details are in the thread. CI hasn't run on the new push yet; could you approve the workflows when you get a chance?