loadfile: clip long track titles #17037
Conversation
|
Note that track metadata comes after URLs so that will get clipped too. Not sure if that is worth it or the issue should be a wontfix. |
|
I haven't read the previous PRs closely, but wouldn't it be better if we stripped the parameters from URL filenames? The current behavior for an URL like edit: disregard this, it fails if the url is instead |
guidocella
changed the title
guidocella
force-pushed
the
track-clip
branch
from
378fbc7 to
58d937b
Compare
|
Actually I don't think track lines necessarily have to fit exactly on 1 line. Changed it to just clip track titles longer than 99 characters. |
guidocella
force-pushed
the
track-clip
branch
2 times, most recently
from
8447aae to
449715b
Compare
|
I just checked...I have many tracks that are close to 200 characters or more...I think many OS's do a fine job of limiting it to 255 or so. I just have a tooltip to show entire track name in uosc. Useful when fuzzy searching for particular terms otherwise user wouldn't be able to see certain matches. Does this patch just clip tracks for the built-in mpv playlist..if so no big deal. |
|
No, the track title itself is clipped. Which is also useful to not hide metadata in the track menu. |
|
Are you confusing tracks and playlist items? External track titles are usually just something like "en.srt". |
|
yea it I was thinking audio tracks as in chapters in audio.....sub tracks... ok. Still not quite sure if I can limit them to 99 but will try but think there definitely are cases where they exceeed. Just don't want it to affect autoloading subs. Also I have usoc to display 3 or 4 lines or whatever it takes to show all files in a playlist in this case only OS affects length of filenames, subs. |
|
This has no relation to the playlist. |
guidocella
force-pushed
the
track-clip
branch
from
449715b to
33fba36
Compare
|
Download the artifacts for this pull request: Windows |
guidocella
force-pushed
the
track-clip
branch
from
33fba36 to
09caf5d
Compare
player/loadfile.c
Outdated
| if (title.len) | ||
|
|
||
| const unsigned char *cut_pos = NULL; | ||
| term_disp_width(title, 90, &cut_pos); |
There was a problem hiding this comment.
Why should users only see 90 characters on a 4k window with small font size? #17034 (comment)
There was a problem hiding this comment.
Because external track URLs are not as important as the main filename or media-title. Long track URLs are usually garbage like the example in #10975, and actually hide the metadata information in the right column of the track menu, which is actually worth seeing.
There was a problem hiding this comment.
Because external track URLs are not as important as the main filename or media-title.
External filenames can end with useful metadata like S01E01.(DiRECTOR.CUT.ENGLiSH).srt. And because in this PR the truncating is applied to the title property rather than for display only, a script that searches through track titles containing certain text will no longer match them.
and actually hide the metadata information in the right column of the track menu, which is actually worth seeing.
There is only an issue because the terminal display decides to place title before metadata. The order can be changed.
There was a problem hiding this comment.
S01E01.(DiRECTOR.CUT.ENGLiSH).srt gets shortened to (DiRECTOR.CUT.ENGLiSH).srt which is just 26 characters, nowhere near 90 which is when this would start causing issues. But we can also clip URLs only.
I was talking about the select.lua menu, lines are wrapped in terminal output so it doesn't have this issue.
When a track title is taken from its filename and is very long, clip it to not make track information too verbose. term_disp_width() is convenient even for ASS output because it avoids cutting in the middle of UTF-8 characters. Fixes mpv-player#10975 along with mpv-player#17021.
guidocella
force-pushed
the
track-clip
branch
from
09caf5d to
d57ab2c
Compare
5 participants
When a track information is longer than the terminal width, clip it and add ellipsis. Useful for long URLs.
Fixes #10975 along with #17021.