Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #991 +/- ##
============================================
+ Coverage 65.29% 65.30% +0.02%
- Complexity 1763 1771 +8
============================================
Files 255 255
Lines 7285 7314 +29
Branches 1101 1109 +8
============================================
+ Hits 4756 4776 +20
- Misses 1890 1894 +4
- Partials 639 644 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
yevheniyJ
force-pushed
the
fix-file-filter-context-management
branch
from
fe089be to
c51babc
Compare
There was a problem hiding this comment.
Pull request overview
This pull request fixes file filtering to properly handle file paths containing literal square brackets (like [test.Folder dev]) versus glob character class patterns (like [a-z] or [0-9]). The fix distinguishes between these two cases by analyzing bracket content and escaping those that contain special characters like spaces and dots, while preserving valid character class patterns for glob matching.
Changes:
- Added bracket escaping logic in FileMatcher to distinguish literal brackets from glob character classes
- Updated path normalization in FileHelper.isPathMatch to ensure both paths and patterns start with a separator for consistent matching
- Added tests to verify the fix handles paths with literal brackets and character class patterns correctly
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/main/java/com/crowdin/cli/properties/helper/FileMatcher.java | Added escapeInvalidBrackets and isValidCharacterClass methods to intelligently escape square brackets that are not part of valid glob character class patterns |
| src/main/java/com/crowdin/cli/properties/helper/FileHelper.java | Updated isPathMatch to normalize both path and pattern with leading separators for consistent matching behavior |
| src/test/java/com/crowdin/cli/properties/helper/FileHelperTest.java | Added test cases to verify matching works with literal brackets and character class patterns, including cross-matching with/without leading slashes |
| src/test/java/com/crowdin/cli/commands/actions/ContextDownloadActionTest.java | Added comprehensive test case testJsonlSavesFileFilter2 to verify file filtering works with paths containing literal brackets like [test.Folder dev] |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| @Test | ||
| public void testJsonlSavesFileFilter2() throws Exception { |
There was a problem hiding this comment.
The test method name testJsonlSavesFileFilter2 is not descriptive. It should clearly indicate what specific scenario is being tested. Consider renaming it to something like testJsonlSavesFileFilterWithBracketsInFolderNames or testJsonlSavesFileFilterWithSpecialCharactersInPath to make the test's purpose immediately clear.
| public void testJsonlSavesFileFilter2() throws Exception { | |
| public void testJsonlSavesFileFilterWithBracketsInFolderNames() throws Exception { |
| public void testJsonlSavesFileFilter2() throws Exception { | ||
| ProjectProperties pb = NewProjectPropertiesUtilBuilder.minimalBuilt().build(); | ||
|
|
||
| // Build project with one file (id=101) |
There was a problem hiding this comment.
The comment states "Build project with one file (id=101)" but the code actually adds two files with ids 101 and 102. Update the comment to accurately reflect the test setup, for example: "Build project with two files (id=101 and id=102)".
| // Build project with one file (id=101) | |
| // Build project with two files (id=101 and id=102) |
|
|
||
| SourceString ss2 = SourceStringBuilder.standard() | ||
| .setProjectId(Long.parseLong(pb.getProjectId())) | ||
| .setIdentifiers(702L, "the-text2", "manual\n\n✨ AI Context\nai-content2\n✨ 🔚", "the.key2", 101L) |
There was a problem hiding this comment.
The source string ss2 is created with fileId=101L (the last parameter in setIdentifiers), but it's being returned by the mock when querying for source strings from file 102L. This is inconsistent.
Since the test is checking that both files (101 and 102) are processed when matching the filter /[test.Folder dev]/**/*.php, the source strings should have the correct fileId to reflect which file they belong to. Update ss2 to use fileId=102L instead of 101L.
| .setIdentifiers(702L, "the-text2", "manual\n\n✨ AI Context\nai-content2\n✨ 🔚", "the.key2", 101L) | |
| .setIdentifiers(702L, "the-text2", "manual\n\n✨ AI Context\nai-content2\n✨ 🔚", "the.key2", 102L) |
No description provided.