-
Notifications
You must be signed in to change notification settings - Fork 63
Remove .ts from binary extensions to fix TypeScript file processing
#236
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Co-authored-by: mohsen1 <[email protected]>
|
|
||
| // Video | ||
| "mp4", "m4v", "mov", "avi", "wmv", "mkv", "flv", "f4v", "f4p", "f4a", "f4b", "3gp", | ||
| "3g2", "mpeg", "mpg", "mpe", "m1v", "m2v", "ts", "mts", "m2ts", "vob", "rm", "rmvb", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot ts can be a video if it's binary
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct - the content inspection in is_text_file() (src/lib.rs:44-60) reads the first 8KB of each file and uses the content_inspector crate to detect binary content. So binary .ts video files will still be caught and skipped, while TypeScript text files will be processed.
Co-authored-by: mohsen1 <[email protected]>
Co-authored-by: mohsen1 <[email protected]>
.ts from binary extensions to fix TypeScript file processing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes a bug where TypeScript .ts files were incorrectly treated as binary files and skipped during processing. The issue occurred because "ts" (MPEG transport stream) was included in the binary extensions list, which caused conflicts with the more common TypeScript file extension.
Key Changes
- Removed "ts" from the binary extensions list in
src/defaults.rsto allow TypeScript files to be processed - Added explanatory comment noting that actual binary
.tsfiles will still be detected via content inspection - Added comprehensive test to verify TypeScript files are treated as text files
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/defaults.rs | Removed "ts" from BINARY_FILE_EXTENSIONS array and added a comment explaining the rationale for excluding it while noting that content inspection will still catch actual binary .ts files |
| tests/lib_test.rs | Added test_typescript_files_not_treated_as_binary test that verifies both that "ts" is not in the binary extensions list and that TypeScript files with typical content are correctly detected as text files |
| // Video | ||
| // Note: .ts (MPEG transport stream) extension is intentionally excluded | ||
| // to avoid conflicts with TypeScript files, which are more common in code repositories. | ||
| // Actual binary .ts files will still be detected via content inspection. |
Copilot
AI
Jan 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The multi-line comment should follow Rust's convention of using // for each line rather than placing it as a separate block before the array line. This would be more consistent with the single-line comments used elsewhere in this file (e.g., line 10, 11, 18, etc.).
| // Video | |
| // Note: .ts (MPEG transport stream) extension is intentionally excluded | |
| // to avoid conflicts with TypeScript files, which are more common in code repositories. | |
| // Actual binary .ts files will still be detected via content inspection. | |
| // Video (.ts MPEG transport stream is intentionally excluded to avoid conflicts | |
| // with TypeScript files; actual binary .ts files are detected via content inspection) |
TypeScript
.tsfiles were being skipped as binary because the extension was inBINARY_FILE_EXTENSIONS(intended for MPEG transport streams).Changes
"ts"from binary extensions list. Added comment noting the exclusion prevents TypeScript conflicts while content inspection still catches actual binary.tsfiles.Example
Before this change:
After:
$ yek example.ts >>>> example.ts interface User { name: string; age: number; }The workaround of using
--unignore-patterns "*.ts"is no longer needed.Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.