Skip to content

fix(ignore-generator): dedupe anchored .gitignore patterns against defaults and skip negation patterns in suggestions#447

Open
tirth8205 wants to merge 2 commits into
Egonex-AI:mainfrom
tirth8205:fix/ignore-generator-anchored-negation
Open

fix(ignore-generator): dedupe anchored .gitignore patterns against defaults and skip negation patterns in suggestions#447
tirth8205 wants to merge 2 commits into
Egonex-AI:mainfrom
tirth8205:fix/ignore-generator-anchored-negation

Conversation

@tirth8205

Copy link
Copy Markdown
Contributor

Problem

generateStarterIgnoreFile produces a starter .understandignore whose "From .gitignore (uncomment to exclude)" section is supposed to list only patterns not already handled by the built-in DEFAULT_IGNORE_PATTERNS. Two bugs defeat that:

  1. Anchored patterns are not deduped against defaults. isCoveredByDefaults()'s normalize() only strips trailing slashes, not the leading / anchor that is extremely common in real .gitignore files (Create React App, Vite, many JS templates use /node_modules, /dist, /build, /coverage). So /node_modules normalizes to /node_modules, never equals the default's normalized node_modules, and the directory is re-suggested even though node_modules/, dist/, etc. are already in the defaults — defeating the whole purpose of the section.

  2. Negation (!) lines are emitted under "uncomment to exclude". parseGitignorePatterns() keeps every non-comment, non-blank line, including re-include lines like !build/keep.txt. These get dumped verbatim under # --- From .gitignore (uncomment to exclude) ---, which is semantically inverted: a !-pattern force-includes a file, it does not exclude it, and the path is only meaningful relative to the project's own .gitignore (which understand-anything does not apply). So the suggestion is misleading noise.

Fix

  • isCoveredByDefaults(): strip a leading root anchor as well as trailing slashes in normalize() (p.replace(/^\//, "").replace(/\/+$/, "")), so /node_modules and /dist/ normalize to node_modules/dist and correctly match the defaults node_modules//dist/.
  • parseGitignorePatterns(): drop !-prefixed lines when collecting suggestable patterns, since they cannot meaningfully be presented as "uncomment to exclude".

Both are minimal, single-file changes in understand-anything-plugin/packages/core/src/ignore-generator.ts.

Testing

Added two tests to ignore-generator.test.ts under the .gitignore integration block:

  • treats anchored .gitignore patterns as covered by defaults — feeds /node_modules\n/dist\n.env and asserts the suggestion section omits the anchored defaults while keeping .env.
  • does not suggest .gitignore negation (!) patterns — feeds build/\n!build/keep.txt\n.env and asserts !build/keep.txt is not emitted while .env is.

Both tests fail before the fix (the anchored patterns are re-suggested; the !-line is emitted) and pass after. The full core Vitest suite is green (694 tests, 34 files) with no regressions, and tsc --noEmit and eslint on the changed files both exit 0.

🤖 Generated with Claude Code

…faults and skip negation patterns in suggestions

isCoveredByDefaults only stripped trailing slashes, so anchored .gitignore
lines like /node_modules and /dist (Vite/CRA/many JS templates) never matched
the built-in defaults node_modules/ and dist/ and were wrongly re-suggested in
the generated .understandignore. Strip a leading root anchor too so anchored
forms normalize to the default name.

parseGitignorePatterns also kept negation (!) lines, which were then emitted
under "uncomment to exclude" — semantically inverted (a !-pattern force-includes,
it does not exclude) and meaningless without the project's own .gitignore
context. Drop !-prefixed lines from suggestable patterns.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@thejesh23 thejesh23 left a comment

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.

1. Dropping ! patterns silently discards user signal that belongs under the header's own ! guidance.
The file header explicitly tells users "Use ! prefix to force-include something excluded by defaults," but parseGitignorePatterns now filters !-lines out at parse time, so a project's !build/keep.txt is invisible rather than surfaced (commented) as a candidate re-include. A more faithful fix would route negations into a separate "From .gitignore (force-include)" section instead of dropping them.

2. normalize() still misses common anchor variants beyond a single leading /.
**/node_modules, **/node_modules/, and ./node_modules/ are all routinely written in real .gitignore files (and CRA/Vite/Next templates) and none of them strip down to the defaults' node_modules. The regex only handles ^\/, so the dedup section will still re-suggest these. Consider p.replace(/^\.?\/+/, "").replace(/^\*\*\/+/, "").replace(/\/+$/, "") or document the limitation.

3. First new test is brittle when the section is fully deduped.
headerIdx = lines.findIndex(...includes("From .gitignore")) returns -1 if every pattern is covered; lines.slice(headerIdx + 1, ...) then becomes slice(0, ...) and silently scans the header, which would mask a future regression where the section is emitted with no patterns. Assert headerIdx >= 0 (or expect(content).toContain("From .gitignore")) before slicing.

Nit: isCoveredByDefaults doc still says "Normalizes trailing slashes" in the one-line summary; the expanded sentence updates it but the lead is now stale.

…opped negations

normalize() now strips leading `./`, root `/`, and globstar `**/` anchors so
common .gitignore forms like `**/node_modules`, `./dist/`, and `**/build/` dedupe
against built-in defaults instead of being re-suggested. Documents in
parseGitignorePatterns why `!`-prefixed re-include lines are dropped (their
semantics are inverted under an "uncomment to exclude" section and only meaningful
against git's own ignore stack). Adds a coverage test for the new anchor variants
and a headerIdx guard so the anchored-patterns test fails loudly if the section is
ever fully deduped.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@tirth8205

Copy link
Copy Markdown
Contributor Author

Addressed all points.

2. normalize() anchor variants — Fixed. normalize() now strips ./, a root /, and a leading globstar (p.replace(/^\.?\/+/, "").replace(/^\*\*\/+/, "").replace(/\/+$/, "")), so **/node_modules, **/node_modules/, ./node_modules/, **/build/, and ./dist/ all dedupe against the built-in defaults. Added a test (treats **/ and ./ anchor variants as covered by defaults) feeding **/node_modules, **/build/, ./dist/, and asserting a non-default (.env) survives. Broadened the isCoveredByDefaults JSDoc accordingly.

3. Brittle test when the section is fully deduped — Fixed. Added expect(headerIdx).toBeGreaterThanOrEqual(0) before the slice in the anchored-patterns test (and in the new anchor-variants test), so the section silently disappearing now fails loudly instead of scanning the header.

1. Dropping ! patterns — Kept as a drop, now documented in parseGitignorePatterns. The reason it isn't routed to a "force-include" section: a !pattern re-includes a path relative to git's own ignore stack, which understand-anything does not apply. Presenting it under "uncomment to exclude" inverts its meaning, and surfacing it as an understandignore force-include would itself be misleading since the path it re-includes isn't excluded by our defaults in the first place. The file header still documents the general ! mechanism for users who want to author one by hand. The added comment explains this intent so it's no longer a silent discard.

Nit (JSDoc lead) — The standalone "Normalizes trailing slashes" lead was already replaced on this branch; the expanded version is now broadened to cover the ./ and globstar anchors too. (Note: the globstar is described in prose rather than the literal **/ token, because that sequence closes a JSDoc block comment.)

Core suite: 695 passed. tsc --noEmit clean.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants