Skip to content

Conversation

lumirlumir
Copy link
Member

@lumirlumir lumirlumir commented Oct 1, 2025

Prerequisites checklist

What is the purpose of this pull request?

This PR originated from #536. While working on that, I found the scope had become too large, so I split this refactoring into a separate PR.

In this PR, I've used the native getLocFromIndex method added in #376 instead of the custom findOffsets util.

Notable changes:

  • Replace the findOffsets util with the native getLocFromIndex.
  • Added an additional test for robustness.

What changes did you make? (Give an overview)

In this PR, I've used the native getLocFromIndex method added in #376 instead of the custom findOffsets util.

Related Issues

Ref: #376, #536

Is there anything you'd like reviewers to focus on?

N/A

@eslintbot eslintbot added this to Triage Oct 1, 2025
@github-project-automation github-project-automation bot moved this to Needs Triage in Triage Oct 1, 2025
* @import { Position } from "unist";
* @import { Text } from "mdast";
* @import { MarkdownRuleDefinition } from "../types.js";
* @import { MarkdownSourceCode } from "../index.js";
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change was motivated by #366 (comment).

The Parameters<MarkdownRuleDefinition['create']>[0]['sourceCode'] expression was needed due to a collision between rollup and tools/dedupe-types at the time, but since @eslint/markdown no longer uses rollup, it's now safe to use ../index.js.

"![foo][]\n\n[foo]: http://bar.com/image.jpg",
"[ foo ][]\n\n[foo]: http://bar.com/image.jpg",
"[eslint][\n\n]",
"[*eslint*][]\n\n[*eslint*]: http://bar.com",
Copy link
Member Author

@lumirlumir lumirlumir Oct 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using strong or emphasis inside a label is an easy-to-forget edge case and often leads to unintended side effects when the check is missed, so I've added one more test.

This is a valid label according to the AST:

image

@lumirlumir lumirlumir marked this pull request as ready for review October 1, 2025 09:52
@lumirlumir lumirlumir requested review from a team and mdjermanovic October 1, 2025 09:52
@lumirlumir
Copy link
Member Author

It seems there is a problem on the macOS runner:

https://www.githubstatus.com/

image

@lumirlumir lumirlumir moved this from Needs Triage to Implementing in Triage Oct 1, 2025
Copy link
Member

@mdjermanovic mdjermanovic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@mdjermanovic mdjermanovic merged commit 17c7744 into main Oct 2, 2025
35 of 37 checks passed
@mdjermanovic mdjermanovic deleted the refactor-use-getlocfromindex-in-no-invalid-label-refs branch October 2, 2025 09:11
@github-project-automation github-project-automation bot moved this from Implementing to Complete in Triage Oct 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Complete

Development

Successfully merging this pull request may close these issues.

2 participants