Skip to content

refactor: simplify isCombiningCharacter helper#20524

Merged
fasttime merged 2 commits intoeslint:mainfrom
JLHwung:patch-1
Mar 6, 2026
Merged

refactor: simplify isCombiningCharacter helper#20524
fasttime merged 2 commits intoeslint:mainfrom
JLHwung:patch-1

Conversation

@JLHwung
Copy link
Copy Markdown
Contributor

@JLHwung JLHwung commented Feb 19, 2026

Prerequisites checklist

AI acknowledgment

  • I did not use AI to generate this PR.
  • (If the above is not checked) I have reviewed the AI-generated content before submitting.

What is the purpose of this pull request? (put an "X" next to an item)

[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[ ] Add something to the core
[x] Other, please explain: Refactor

What changes did you make? (Give an overview)

Per D52 Combing Character in the chapter 3 of the Unicode core, and PropertyValueAlias.txt on UCD

The General Category M consists of exactly three sub categories: Mc, Me, and Mn, therefore the regex here can be simplified.

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

@JLHwung JLHwung requested a review from a team as a code owner February 19, 2026 21:31
@github-project-automation github-project-automation bot moved this to Needs Triage in Triage Feb 19, 2026
@eslint-github-bot eslint-github-bot bot added the chore This change is not user-facing label Feb 19, 2026
@netlify
Copy link
Copy Markdown

netlify bot commented Feb 19, 2026

Deploy Preview for docs-eslint canceled.

Name Link
🔨 Latest commit f705182
🔍 Latest deploy log https://app.netlify.com/projects/docs-eslint/deploys/699782f4d25ae40008a0e98e

@github-actions github-actions bot added the rule Relates to ESLint's core rules label Feb 19, 2026
@lumirlumir lumirlumir moved this from Needs Triage to Triaging in Triage Feb 20, 2026
Copy link
Copy Markdown
Member

@lumirlumir lumirlumir 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!

Would like one more check from the team before merging.

@lumirlumir lumirlumir added the accepted There is consensus among the team that this change meets the criteria for inclusion label Feb 20, 2026
@lumirlumir lumirlumir moved this from Triaging to Second Review Needed in Triage Feb 20, 2026
module.exports = function isCombiningCharacter(codePoint) {
return /^[\p{Mc}\p{Me}\p{Mn}]$/u.test(String.fromCodePoint(codePoint));
return /^\p{M}$/u.test(String.fromCodePoint(codePoint));
};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I've also updated this helper in #20396, does that change make sense or will this regexp be better?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This regexp still contains property escape, so it won't work with the icu-less Node.js build. If the goal is compatibility with such build, then of course #20396 so be pursued.

Copy link
Copy Markdown
Member

@fasttime fasttime 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!

@fasttime fasttime merged commit 5cd1604 into eslint:main Mar 6, 2026
38 checks passed
@github-project-automation github-project-automation bot moved this from Second Review Needed to Complete in Triage Mar 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

accepted There is consensus among the team that this change meets the criteria for inclusion chore This change is not user-facing contributor pool rule Relates to ESLint's core rules

Projects

Status: Complete

Development

Successfully merging this pull request may close these issues.

4 participants