Conversation
…rrectly The original implementation had several issues when processing glob patterns in nanorc include/theme directives: 1. Called nanorc.resolveSibling(parameter).getParent() with glob patterns containing wildcards, which fails on Windows and non-default file systems 2. Did not properly support recursive patterns like 'foo/bar/**/*.nanorc' 3. Used Paths.get() which assumes the default file system, breaking compatibility with JAR file systems Changes: - Refactored addFiles method to extract static path prefix and glob pattern separately using new PathParts helper class - Use nanorc.resolveSibling() consistently for path resolution to support non-default file systems like JAR FS - Avoid path normalization and let the file system handle path separators - Added comprehensive tests with JimFS to verify cross-platform behavior on both Unix and Windows file systems The fix ensures that nanorc configuration files can properly include syntax files using glob patterns like: - '*.nanorc' (simple patterns) - 'subdir/*.nanorc' (subdirectory patterns) - 'foo/bar/**/*.nanorc' (recursive patterns) - '/usr/share/nano/*.nanorc' (absolute patterns) Fixes #1290
4bbe8b6 to
6d557cf
Compare
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
1 participant
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.
Problem
The
SyntaxHighlighter.addFiles()method had several critical issues when processing glob patterns in nanorc include/theme directives:nanorc.resolveSibling(parameter).getParent()with glob patterns containing wildcards (*,?), which fails on Windows and non-default file systemsfoo/bar/**/*.nanorcPaths.get()which assumes the default file system, breaking compatibility with JAR file systems and other custom file systemsRoot Cause
The original code tried to extract directory information from glob patterns by treating them as regular paths, which is fundamentally incorrect since glob patterns contain special characters that are not valid in path operations.
Solution
Refactored the
addFilesmethod with a clean separation of concerns:extractPathParts()method splits glob patterns into static prefix and glob pattern componentsnanorc.resolveSibling()for all path operations to support non-default file systemsPathMatcherSupported Patterns
The fix now correctly handles all these glob patterns:
*.nanorc(simple patterns)subdir/*.nanorc(subdirectory patterns)foo/bar/**/*.nanorc(recursive patterns)/usr/share/nano/*.nanorc(absolute patterns)Testing
Added comprehensive cross-platform tests using JimFS to verify behavior on both Unix and Windows file systems, ensuring the fix works correctly with:
Backward Compatibility
All existing functionality is preserved - the fix only corrects the broken glob pattern handling without changing the API or behavior for non-glob patterns.
Fixes #1290, supersedes #1291
Pull Request opened by Augment Code with guidance from the PR author