Skip to content

Conversation

@gnodet
Copy link
Member

@gnodet gnodet commented May 27, 2025

Problem

The SyntaxHighlighter.addFiles() method had several critical issues when processing glob patterns in nanorc include/theme directives:

  1. Invalid path operations: Called nanorc.resolveSibling(parameter).getParent() with glob patterns containing wildcards (*, ?), which fails on Windows and non-default file systems
  2. Limited pattern support: Did not properly support recursive patterns like foo/bar/**/*.nanorc
  3. File system compatibility: Used Paths.get() which assumes the default file system, breaking compatibility with JAR file systems and other custom file systems

Root 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 addFiles method with a clean separation of concerns:

  1. Extract path parts: New extractPathParts() method splits glob patterns into static prefix and glob pattern components
  2. Consistent path resolution: Use nanorc.resolveSibling() for all path operations to support non-default file systems
  3. Avoid normalization: Let the file system handle path separators instead of manual string manipulation
  4. Proper pattern construction: Build glob patterns correctly for PathMatcher

Supported 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:

  • Default file system
  • JAR file systems
  • Other custom file systems
  • Windows and Unix path conventions

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

@gnodet gnodet added the bug label May 27, 2025
@gnodet gnodet changed the title Fix SyntaxHighlighter glob pattern handling for non-default file systems fix: Fix SyntaxHighlighter glob pattern handling for non-default file systems May 27, 2025
…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
@gnodet gnodet force-pushed the fix-syntax-highlighter-glob-patterns branch from 4bbe8b6 to 6d557cf Compare May 27, 2025 12:13
@gnodet gnodet added this to the 3.30.4 milestone May 27, 2025
@gnodet gnodet merged commit 96c50f3 into master May 27, 2025
9 of 17 checks passed
@gnodet gnodet deleted the fix-syntax-highlighter-glob-patterns branch May 27, 2025 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SyntaxHighlighter fails in Windows

2 participants