-
Notifications
You must be signed in to change notification settings - Fork 3.2k
player/lua/stats.lua: ellipsize filename #17034
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
base: master
Are you sure you want to change the base?
Conversation
Takes a string and an optional length and returns string of that length with '...' in the middle. If no length is provided it defaults to the good old 80 chars. If the string is shorter than target length the input string is returned unchanged. This, in conjunction with mpv-player#17021, is intended to eventually fix mpv-player#10975.
If set the string gets ellipsized to the desired length before being appended.
As a POC for a possible fix of mpv-player#10975 the "File" field now uses the filename property ellipsized to 32 chars. For now the length is hard-coded and rather small to make its effect more prominent in testing.
| return text end | ||
|
|
||
| local middle = max_len/2 + max_len%2 | ||
| local ellipsized = ("%s...%s"):format(text:sub(1, middle-1), text:sub(-middle+2)) |
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.
This will split multi-byte characters and grapheme clusters and so won't work as expected or produce invalid UTF-8 with non-ASCII values.
Best you can do in Lua here is to at least operate on codepoints and avoid the invalid UTF-8 part (it may still split things like emoji sequences or modifier characters and produce weird results).
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.
Thanks, I was hoping for Schrödinger's UTF-8 support cat to be alive in Lua. So there doesn't seem to be any way to get this on the cheap. I'll explore bstr support for UTF-8 then.
|
stats already had this for terminal output and we removed it in a187110. Clipping is already handled by |
But I don't like that the clipping is done at the end. I want something that inserts an ellipsis in the middle if at all possible. I feel, I may be getting somewhere with Lua, though. I want to try some things before I push. With some inspiration from |
|
|
|
That is not feasible with ASS. You can't know the text length before rendering short of using It is also completely arbitrary do this for the stats filename and not any other OSD message. If anything this should be implemented in libass. This also doesn't fix the linked issue which is about the output when printing tracks to the terminal, and has no relation to stats. |
|
Those 32 chars are just for testing, because my filename examples are rather short. It is meant to be configurable at least, eventually. I held off while this is still POC. Re: text width, the underlying assumption is that the worst case is a monospace font. So if the text fits using that it sure does with proportional. Doesn't that make it easier to determine what Anyway, if this cannot be done because of limitations with ASS rendering, I'd still like to have the second best option, some script-opt to set, say Or just tell me if this is a dead end so I don't waste any more time. |
This is so wrong. There is no correspondence between a character's size in bytes and the width of the glyphs it generates. For example fullwidth Chinese characters usually occupy two ASCII characters worth of space but consist of more than two bytes. When dealing with codepoints which is what you should be doing at the very least, both of those occupy only one. In the terminal this is handled by agreeing on a function for character width in cells that programs and terminal emulators use (see: In the GUI world I like to believe that we hold ourselves to high standards and do precise width calculations after shaping text with the appropriate fonts. This gets complicated when doing things like line-breaking (or inserting ellipses) though due to technicalities of text layout. This is why for it to be perfect this has to have work done by libass. |
Or just oversimplified?
I know as much. But knowing that a certain cluster of bytes will result in the equivalent of two chars width in a terminal (or monospace font) and knowing the font size should be enough for calculating the space required, no? And if one does use a monospace font, the result should fit perfectly in said space. If the font is proportional, all that's lost is some vacant space.
But you don't, do you? I've just played a video file with a >200 char filename and
I think perfection can never be achieved anyway, so why not take something that will work in the vicinity of good enough and doesn't explode in case it misses the mark by some exotic unicode symbol or two. And if one is fine with status quo, don't opt-in once the option exists and you'll be none the wiser. |
But you don't know the width of ASS output without
It is worse because it cuts arbitrarily leaving empty space, while clipping used all available space at any window and font width, with no code required on our side.
It is not good enough to cut a fixed number of bytes. And it is still stupid to do for a single string in all of mpv. |
So the script-opt variant then.
But clipping has downsides too, someone else has pointed out in the linked issue, IIRC. Having start, end and '...' in the middle leaves more context, especially the file extension.
Then you can just leave said option alone (unset) and be golden. As I've said before, this is not necessarily exclusive to the file name. Any |
|
@guidocella Also, where is clipping even happening right now? Because, I couldn't find anything that seems to be doing it and the |
|
I guess it doesn't actively clip ASS output, it just doesn't wrap if there are no spaces in the filename and it keeps going beyond the window. It does clip terminal output with |
|
Now I get it, the terminal thing. |
|
ASS clipping is done with |
For the time being this is a POC to eventually fix/mitigate the display of long URLs in OSD stats, see #10975 (comment).
This introduces a simple ellipsize function and an additional append attribute
max_lento automagically filter thestrargument. The hard-coded length of 32 chars was chosen for testing purposes. Ideally it can be dynamically changed depending on the available space but I haven't figured out yet if and how that can be determined. If it can't be done, providing a script option is easy enough.In conjunction with #17021 this could be a fix for #10975.
P.S.: I chose to do this in stats.lua because it fixes the immediate issue and Lua comes some batteries included to make this easier and readable. But I am pondering, if extending the filename attribute getter is the better place to do this, i.e. a new subproperty
/short? Anyway, for now this is an easy (partial) win.https://github.com/mpv-player/mpv/blob/master/DOCS/contribute.md