Skip to content

fix(windows): improve yt-dlp filename safety without changing defaults#100

Merged
vanloctech merged 2 commits into
vanloctech:mainfrom
blueyi:fix/windows-safe-filenames
Jul 6, 2026
Merged

fix(windows): improve yt-dlp filename safety without changing defaults#100
vanloctech merged 2 commits into
vanloctech:mainfrom
blueyi:fix/windows-safe-filenames

Conversation

@blueyi

@blueyi blueyi commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add --windows-filenames on Windows so yt-dlp replaces invalid characters before writing files
  • Compute --trim-filenames from the output directory length instead of a fixed 180 bytes
  • Keep the default output template %(title)s.%(ext)s and metadata sidecar naming unchanged

Narrow follow-up to #97 per maintainer feedback: only harden existing safe filename args, without changing default templates, ASCII restrictions, or settings UI.

Test plan

  • cargo test filename --lib
  • cargo check
  • bun run tsc -b
  • Facebook Reel with long caption on Windows (previously Errno 22)
  • YouTube with non-ASCII title keeps Unicode filename
  • Metadata export (description / info.json / comments) still works

Add --windows-filenames on Windows and compute --trim-filenames from the
output directory length so long Facebook/Instagram titles stay within path
limits. Keeps the existing %(title)s output template unchanged.
@vanloctech

Copy link
Copy Markdown
Owner

Thanks, this PR looks much better scoped than the previous filename change. I’m okay with the overall direction: keeping the existing output template, avoiding new settings UI, and only hardening the backend filename safety path is the right approach.

Two small changes before merge:

  1. Since the current default trim limit is 180, could we keep the effective cap at 180 instead of increasing short output paths to 200? That keeps the fix more behavior-preserving while still solving the Windows long-path case with path-aware trimming.

  2. --trim-filenames is byte-oriented, but calc_trim_filenames_bytes() currently uses output_path.chars().count(). Could we use byte length instead, e.g. output_path.as_bytes().len(), so paths with non-ASCII folder names are handled more conservatively?

After those two small adjustments, this should be good to merge.

Address PR review: keep the effective --trim-filenames cap at 180 for
short output paths, and use output_path.as_bytes().len() so non-ASCII
folder names reserve path space correctly.
@blueyi

blueyi commented Jul 5, 2026

Copy link
Copy Markdown
Contributor Author

Thanks, this PR looks much better scoped than the previous filename change. I’m okay with the overall direction: keeping the existing output template, avoiding new settings UI, and only hardening the backend filename safety path is the right approach.

Two small changes before merge:

  1. Since the current default trim limit is 180, could we keep the effective cap at 180 instead of increasing short output paths to 200? That keeps the fix more behavior-preserving while still solving the Windows long-path case with path-aware trimming.
  2. --trim-filenames is byte-oriented, but calc_trim_filenames_bytes() currently uses output_path.chars().count(). Could we use byte length instead, e.g. output_path.as_bytes().len(), so paths with non-ASCII folder names are handled more conservatively?

After those two small adjustments, this should be good to merge.

Done — capped trim at 180 and switched path length to byte count. Thanks for the review!

@vanloctech vanloctech merged commit 0d24937 into vanloctech:main Jul 6, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants