Skip to content

Count only doubled **/ against the zero-segment expansion cap [patch] - #187

Open
matt-edmondson wants to merge 2 commits into
mainfrom
fix/185-globstar-cap-joined-segments
Open

matt-edmondson wants to merge 2 commits into
mainfrom
fix/185-globstar-cap-joined-segments

Conversation

@matt-edmondson

Copy link
Copy Markdown
Contributor

Fixes #185

What was wrong

ExpandGlobStarSegments caps how many **/ get the zero-segment alternative (MaxExpandedGlobStarSegments = 4), and it ran expanded++ for every **/ it found. Since #171, a **/ joined to a literal (foo**/) never gets a zero-segment form, but it still used up a slot. So a**/b**/c**/d**/**/x.cs didn't match a/b/c/d/x.cs: its one whole-segment **/ came after the cap was already spent.

Change

The counter now advances only when the **/ actually added a zero-segment alternative. A whole-segment **/ still counts exactly as before, so the cap and its documented behaviour for **/**/**/**/**/… are unchanged. I left the optional "collapse repeated **/**/" idea out to keep this PR small.

Tests

Note

#175 edits the lines next to this loop (separate masks for / and \). Whichever of the two merges second may need a small rebase. Neither change depends on the other.


Generated by Claude Code

Since #171 a "**/" joined to a preceding literal is never expanded into a
zero-segment form, but it still incremented the expansion counter. Four of
them used up the cap, so "a**/b**/c**/d**/**/x.cs" stopped matching
"a/b/c/d/x.cs". The counter now advances only when a "**/" actually added
the zero-segment alternative.

Fixes #185

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017vfJW6FzBtSrY5LKUuDYce
…patch]

The README API test required every overload's same-named parameter to carry
the documented default. The binary-compatibility overloads from #166 have no
defaults by design, so main has been red since #178. They are marked
[EditorBrowsable(Never)] and are not what the README documents, so the default
check now skips them. A wrong default on a visible overload still fails.

Fixes #183

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017vfJW6FzBtSrY5LKUuDYce
(cherry picked from commit 9328e00)

Copy link
Copy Markdown
Contributor Author

ci / .NET / Test on ubuntu-latest failed on b5792a4 in EveryMethodInTheReadmeApiReferenceExistsWithItsDocumentedDefaults, with 270 passed and 1 failed. That failure doesn't come from this PR: the same test fails on main because of #183. Its fix is in #186, so I cherry-picked that commit (6f196b4) here. With it, the full suite runs 271 passed, 0 failed locally. If #186 merges first, this change is identical and will drop out cleanly.


Generated by Claude Code

@sonarqubecloud

sonarqubecloud Bot commented Nov 4, 2026

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
D Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

Copy link
Copy Markdown
Contributor Author

The SonarCloud Code Analysis gate fails on 6f196b4 with a D maintainability rating on new code. That comes from one CRITICAL issue, csharpsquid:S3776: ExpandGlobClasses (TextFilter.cs:520) has cognitive complexity 16, over the limit of 15.

This PR doesn't cause it. It doesn't touch ExpandGlobClasses, and the complexity came in with #177. Main's last Sonar analysis predates #177: since #178, the main push runs fail at Test (#183), so Analyze & Release, which runs Sonar, is skipped. Any PR that edits TextFilter.cs is therefore compared against a stale baseline. #186 doesn't edit that file, so its gate passes.

No fix exists yet. Once #186 merges, main gets a fresh analysis, the issue moves into the baseline, and this gate should clear on the next run. If main still flags S3776 after that, the fix is a small separate refactor of ExpandGlobClasses (for example, moving the per-class rewrite into a helper), not part of this PR. I can't re-run the Sonar check from here.


Generated by Claude Code

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.

Literal-joined "**/" use up the zero-segment expansion cap, so "a**/b**/c**/d**/**/x.cs" no longer matches "a/b/c/d/x.cs"

2 participants