refactor: extract identifier normalization into a shared utility - #733
refactor: extract identifier normalization into a shared utility#733KumJungMin wants to merge 5 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe change centralizes reference identifier normalization in ChangesReference normalization
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This change centralizes identifier normalization and adds coverage for casing and whitespace behavior. No current merge-blocking risk is identified. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
lumirlumir
left a comment
There was a problem hiding this comment.
Disclosure: I'm a participant of open source contribution program OSSCA: confirmed.
Thanks for the PR. I’ve left a few stylistic suggestions that we usually follow.
| } | ||
| /** |
There was a problem hiding this comment.
| } | |
| /** | |
| } | |
| /** |
Non-blocking, but adding a single blank line here would be helpful.
| @@ -1,3 +1,5 @@ | |||
| import { normalizeIdentifier as micromarkUtilNormalizeIdentifier } from "micromark-util-normalize-identifier"; | |||
There was a problem hiding this comment.
I think the import statement above needs to be placed below the following fileoverview JSDoc comment, in accordance with our convention:
/**
* @fileoverview Utility Library
* @author Nicholas C. Zakas
*/
//------------------------------------------------------------------------------
// Imports
//------------------------------------------------------------------------------
import { normalizeIdentifier as micromarkUtilNormalizeIdentifier } from "micromark-util-normalize-identifier";
// ...| * @param {string} identifier The identifier to normalize. | ||
| * @returns {string} The normalized lowercase identifier. | ||
| */ | ||
| export function normalizeIdentifier(identifier) { |
There was a problem hiding this comment.
Also, for alphabetical order, it would be helpful to place this function between frontmatterHasTitle and stripHtmlComments.
lumirlumir
left a comment
There was a problem hiding this comment.
LGTM, thanks!
I’m leaving it open for two more days, just in case anyone else would like to review it, in accordance with our policy.
Prerequisites checklist
AI acknowledgment
What is the purpose of this pull request?
What changes did you make? (Give an overview)
Added
normalizeIdentifier()tosrc/util.jsto combine micromark’s identifier normalization and lowercase conversion in one place.Updated the following four rules to use the shared utility:
no-duplicate-definitionsno-empty-definitionsno-unused-definitionsno-reference-like-urlsAdded utility tests covering lowercase conversion, trimming leading and trailing whitespace, normalizing consecutive spaces, tabs, and line breaks, and handling empty strings.
Related Issues
fixes #731
Is there anything you'd like reviewers to focus on?
Summary by CodeRabbit
Bug Fixes
Tests