Skip to content

Conversation

@lumirlumir
Copy link
Member

@lumirlumir lumirlumir commented Oct 8, 2025

Prerequisites checklist

What is the purpose of this pull request?

Which language are you using?

CommonMark and GFM.

What did you do?

I expect the no-missing-atx-heading-space rule to correctly recognize CRLF and CR line endings, but it currently does not.

What did you expect to happen?

I expect the no-missing-atx-heading-space rule to correctly recognize CRLF and CR line endings.

Link to minimal reproducible Example

Adding the following test cases to tests/rules/no-missing-atx-heading-space.test.js would help identify the problem:

	{
		code: "Text before\r\n#Heading with ``` code markers\r\nText after",
		output: "Text before\r\n# Heading with ``` code markers\r\nText after",
		errors: [
			{
				messageId: "missingSpace",
				data: { position: "after" },
				line: 2,
				column: 1,
				endLine: 2,
				endColumn: 3,
			},
		],
	},
	{
		code: "Text before\r#Heading with ``` code markers\rText after",
		output: "Text before\r# Heading with ``` code markers\rText after",
		errors: [
			{
				messageId: "missingSpace",
				data: { position: "after" },
				line: 2,
				column: 1,
				endLine: 2,
				endColumn: 3,
			},
		],
	},

What changes did you make? (Give an overview)

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

In this PR, I've fixed an issue where the no-missing-atx-heading-space rule did not recognize CRLF and CR line endings.

The previous logic relied on a custom line breaker pattern using const newLinePattern = /\r?\n/u;, but this is no longer necessary now that the getLocFromIndex method has been added. So, I've removed it.

Also, the previous logic required a lot of calculations to convert "offset" into "line, column", but that's no longer necessary, so I refactored the code. (The larger git diff was unavoidable.)

I consistently used the pattern let match; while ((match = pattern.exec(text)) !== null) when visiting Paragraph and Heading nodes. Since this pattern is used in over 10 places throughout the Markdown rule implementations, using it consistently would make the codebase easier to understand.

Notable changes:

  • Use named capture groups in the regex patterns.
  • Add the m flag to leadingAtxHeadingHashPattern to avoid custom line break logic.
  • Move the findMissingSpaceBeforeClosingHash helper function into the Paragraph visitor, since this logic was only needed before there was no direct "offset" to "line, column" conversion.
  • Use the let match; while ((match = pattern.exec(text)) !== null) pattern consistently when visiting Paragraph and Heading nodes.

Related Issues

Ref: #493, eslint/css#280

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

I'll update the other rules and processors to recognize CR in #493. This PR is separate from that one to keep the scope smaller.

@eslint-github-bot eslint-github-bot bot added the bug label Oct 8, 2025
@eslintbot eslintbot added this to Triage Oct 8, 2025
@github-project-automation github-project-automation bot moved this to Needs Triage in Triage Oct 8, 2025
@lumirlumir
Copy link
Member Author

Pending until #554 is merged.

@lumirlumir lumirlumir marked this pull request as ready for review October 10, 2025 06:52
@lumirlumir

This comment was marked as resolved.

@lumirlumir lumirlumir moved this from Needs Triage to Implementing in Triage Oct 11, 2025
@lumirlumir lumirlumir marked this pull request as draft October 14, 2025 10:11
@lumirlumir lumirlumir marked this pull request as ready for review October 14, 2025 10:39
@lumirlumir
Copy link
Member Author

friendly ping @mdjermanovic

@mdjermanovic mdjermanovic changed the title fix: handle CR and CRLF in no-missing-atx-heading-space feat: handle CR and CRLF in no-missing-atx-heading-space Oct 22, 2025
Comment on lines +481 to +494
{
code: "Text before\r#Heading with ``` code markers\rText after",
output: "Text before\r# Heading with ``` code markers\rText after",
errors: [
{
messageId: "missingSpace",
data: { position: "after" },
line: 2,
column: 1,
endLine: 2,
endColumn: 3,
},
],
},
Copy link
Member

Choose a reason for hiding this comment

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

I changed the tag to feat because this change can produce more lint errors. For example, the code from this test case was valid before this change.

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 a869d98 into main Oct 22, 2025
24 checks passed
@mdjermanovic mdjermanovic deleted the fix-handle-cr-in-no-missing-atx-heading-space branch October 22, 2025 10:10
@github-project-automation github-project-automation bot moved this from Implementing to Complete in Triage Oct 22, 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.

3 participants