diff options
| author | Craig Jennings <c@cjennings.net> | 2026-08-03 15:27:17 -0500 |
|---|---|---|
| committer | Craig Jennings <c@cjennings.net> | 2026-08-03 15:27:17 -0500 |
| commit | 6f90ec5b660c6a2552ac7284a3d9d963771fbbc2 (patch) | |
| tree | 90bcdaafd27fabdd7dfcee7484b8646e2bc6923b /modules | |
| parent | a457ade9207be48f6bfaa519046d7acea390ff1b (diff) | |
| download | dotemacs-6f90ec5b660c6a2552ac7284a3d9d963771fbbc2.tar.gz dotemacs-6f90ec5b660c6a2552ac7284a3d9d963771fbbc2.zip | |
fix(music): size a playlist by its content, not by its symlink
cj/music--append-track-to-m3u-file decided whether to prepend a newline by
seeking to a byte offset from file-attributes, which doesn't follow
symlinks. On a stow-deployed playlist that measures the link string instead
of the file.
Two failure modes, and I only went looking for the second. All 100 symlinked
playlists read the wrong byte, so a file already ending in a newline looked
unterminated and gained a blank line on every append. On the 31 whose link
string is longer than their content the range fell outside the file
entirely. Nothing was inserted, and char-after handed nil to a numeric
comparison, so the append died with a wrong-type error.
The probe now reads the file and checks its last character. That takes
file-attributes out of the path, so this class can't come back. The largest
real playlist is 11 KB and the read costs 0.11 ms, so being clever about
offsets bought nothing.
A test per mode, because one fixture can't show both. Seed the blank-line
case with content shorter than the link string and it lands on the error
path instead, going red for the wrong reason and proving nothing.
The cheap patch was guarding char-after against nil. That would have
silenced the crash on 31 playlists and left the blank line live on all 100.
Diffstat (limited to 'modules')
| -rw-r--r-- | modules/music-config.el | 22 |
1 files changed, 14 insertions, 8 deletions
diff --git a/modules/music-config.el b/modules/music-config.el index 0834aae8..47863e41 100644 --- a/modules/music-config.el +++ b/modules/music-config.el @@ -696,14 +696,20 @@ M3U-FILE should be an existing, writable M3U file path." track-path (let ((rel (file-relative-name track-path dir))) (if (string-prefix-p "../" rel) track-path rel))))) - ;; Determine if we need a leading newline - (let ((needs-prefix-newline nil) - (file-size (file-attribute-size (file-attributes m3u-file)))) - (when (> file-size 0) - ;; Read the last character of the file to check if it ends with newline - (with-temp-buffer - (insert-file-contents m3u-file nil (max 0 (1- file-size)) file-size) - (setq needs-prefix-newline (not (= (char-after (point-min)) ?\n))))) + ;; Does the file need a separating newline first? Read the content and look + ;; at its last character, rather than seeking to a byte offset derived from + ;; `file-attributes'. That call does not follow symlinks, so on a + ;; stow-deployed playlist it measures the link string instead of the file: + ;; every symlinked playlist read the wrong byte and gained a blank line per + ;; append, and where the link string was the longer of the two the range fell + ;; outside the file entirely and the append died on a nil `char-after'. + ;; Playlists are small text files, so reading one is cheaper than being + ;; clever about offsets. + (let ((needs-prefix-newline + (with-temp-buffer + (insert-file-contents m3u-file) + (and (> (buffer-size) 0) + (/= (char-before (point-max)) ?\n))))) ;; Append the track with proper newline handling (with-temp-buffer |
