Repository navigation
Fix slugifyWithCounter() returning the same slug twice - #82
Merged
Merged
Conversation
hyperbola-bear
added a commit
to hyperbola-bear/claude-mods
that referenced
this pull request
Oct 7, 2026
- a review is asked for by "review" with a PR link, a link alone, "review pr", "review pr 418" or "pr review"; the PR is checked out into a review worktree (~/.cache/pr-review-ui/review/) whose HEAD sits at the merge base, so editors, git diff and the pane show the PR's changes; the person's checkout is never touched - point 1's code shows with nothing to click: the editor (VS Code and forks, JetBrains, Zed, Neovim), terminal-browser, or the code pane, which docks from 144 columns and otherwise draws the code under the open point until the terminal is widened - one review standard: verdict line, severity tags, area, Problem / Impact / Fix, a suggested diff, and a merge plan; a review with no points still draws its verdict and summary - the pane draws the change like git diff: old and new line numbers, changed words, more lines on demand, lines fit to the width - the band and the pane step points with a and d - the first session after install asks before installing the GitHub CLI, Ghostty or terminal-browser; /review-setup asks again - cache clones are full: Claude Code runs git with lazy fetching off Checked live in Ghostty against Claude Code 2.1.292, including a real review of sindresorhus/slugify#82 through the worktree. Co-authored-by: clement <clement@clements-MacBook-Pro.local> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
slugifyWithCounter()can return the same slug twice. Its whole purpose is to guarantee unique slugs, so this defeats the feature.The counter is keyed on the incoming slug only (
occurrences), so it has no idea thatfoo-2was already handed out as the counter form of a different input. It happens in either order —foo,foo 2,fooalso yieldsfoo-2twice — and is easy to hit in practice, since heading-to-anchor generation is the main use case and documents that repeat a heading and separately have aFoo 2heading are ordinary.Fix
Track the slugs actually returned in a
Setand keep bumping the counter while the candidate is already taken. The comparison is lowercased to match howoccurrencesis keyed, so a{lowercase: false}call cannot collide with a lowercase one either.reset()clears it alongsideoccurrences.The common path is unchanged: the loop body only runs on an actual collision, so there is no extra work for the normal case beyond one
Setlookup and insert.Results after the fix:
Validation
npm test(xo + ava): 25/25 pass, no lint errors.mainwith'foo-2'where'foo-2-2'is expected.mainproduces duplicates, the patched version never does.