Add more flexible ignore matching for full ignore rule#385
Conversation
WalkthroughA new mechanism was implemented in the ignore rules logic to support "full wildcard" patterns ( Changes
Poem
✨ Finishing Touches
🧪 Generate Unit Tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
internal/ignore/rules_test.go (1)
41-48: Consider checking error returns from rules.Add()While the existing tests don't check errors either, it would be more robust to verify that rules are added successfully.
func TestNegateRules(t *testing.T) { rules := Empty() rules.AddDefaults() - rules.Add("!**/foo.py") + err := rules.Add("!**/foo.py") + assert.NoError(t, err) assert.False(t, rules.Ignore("/Users/foobar/example/src/foo.py", nil)) assert.False(t, rules.Ignore("foo.py", nil)) assert.True(t, rules.Ignore("bar.py", nil)) }🧰 Tools
🪛 golangci-lint (1.64.8)
44-44: Error return value of
rules.Addis not checked(errcheck)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
cmd/cloud.go(1 hunks)internal/ignore/rules.go(6 hunks)internal/ignore/rules_test.go(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
internal/ignore/rules_test.go (1)
internal/ignore/rules.go (2)
Empty(51-53)Ignore(40-40)
🪛 golangci-lint (1.64.8)
internal/ignore/rules_test.go
44-44: Error return value of rules.Add is not checked
(errcheck)
53-53: Error return value of rules.Add is not checked
(errcheck)
54-54: Error return value of rules.Add is not checked
(errcheck)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Test CLI Upgrade Path (windows-latest)
- GitHub Check: Analyze (go)
🔇 Additional comments (5)
cmd/cloud.go (1)
151-151: LGTM!The blank line improves readability by separating the rule processing logic from the return statement.
internal/ignore/rules.go (3)
55-61: String() method implementation looks goodThe implementation efficiently concatenates patterns using bytes.Buffer and provides a useful debugging/inspection capability.
160-166: Special handling for full wildcard pattern is well implementedThe logic correctly implements the "ignore everything by default, then selectively include" pattern when
**/*is the first rule. This is a useful feature for projects that want to be explicit about what to include.
216-233: Full wildcard pattern parsing correctly implements the special behaviorThe implementation properly handles the
**/*pattern by placing it first and filtering out non-negated patterns. Note that when**/*is encountered, any non-negated patterns that appear before it in the file are discarded, keeping only negated patterns.internal/ignore/rules_test.go (1)
50-93: Excellent test coverage for the new full wildcard functionalityThe tests comprehensively verify the behavior of
**/*pattern in various scenarios, including different ordering relative to negated patterns. This ensures the feature works correctly regardless of how rules are arranged in ignore files.🧰 Tools
🪛 golangci-lint (1.64.8)
53-53: Error return value of
rules.Addis not checked(errcheck)
54-54: Error return value of
rules.Addis not checked(errcheck)
1 participant
For bundling, we aren't ignore enough files with the rules we have. In a JS bundle case, we can pretty much exclude everything except the
agentuity.yamland the.agentuitydirectory to keep things as slim as possible. This will enable the ability to change the templates to be more aggressive in ignoring files.We need to roll this out (and force upgrade) before we can roll out the template changes.
Summary by CodeRabbit