tomasklaen / uosc

Feature-rich minimalist proximity-based UI for MPV player.
GNU Lesser General Public License v2.1
1.86k stars 68 forks source link

feat: optimize track title and update Chinese translation #975

Closed dyphire closed 2 months ago

dyphire commented 2 months ago

ref #964

po5 commented 2 months ago

You should reuse the existing Lua pattern escape code: https://github.com/tomasklaen/uosc/blob/3621e0b4c6033f6ca2cc0f845cc9fc8209c574f3/src/uosc/elements/TopBar.lua#L68

dyphire commented 2 months ago

You should reuse the existing Lua pattern escape code:

https://github.com/tomasklaen/uosc/blob/3621e0b4c6033f6ca2cc0f845cc9fc8209c574f3/src/uosc/elements/TopBar.lua#L68

Missed it.

tomasklaen commented 2 months ago

Better do a nil check on track.title.

But I don't know. Is this a good idea? I don't feel good about seeing items like this:

image

I could do a poor man's solution to the width calculation instead: ensure there's always space for at least 3 external buttons outside. That should cover 99% use cases.

dyphire commented 2 months ago

Better do a nil check on track.title.

I did at first but when it was an external track the track title was the filename this could not be empty.

But I don't know. Is this a good idea? I don't feel good about seeing items like this:

I improved it and it looks much better now.

I could do a poor man's solution to the width calculation instead: ensure there's always space for at least 3 external buttons outside. That should cover 99% use cases.

I would say that's fine too, even so this PR is still a nice improvement.

tomasklaen commented 2 months ago
if track.lang and track.title:lower() == track.lang:lower() then
    track.title = nil
end

What is this for? Track title in here will always contain extension, so it will never match lang, no?

dyphire commented 2 months ago
if track.lang and track.title:lower() == track.lang:lower() then
  track.title = nil
end

What is this for? Track title in here will always contain extension, so it will never match lang, no?

No, the track.title = track.title:gsub(escaped_filename .. '%.?', ''):gsub('%.?([^%.]+)$', '') has stripped the extension.

dyphire commented 2 months ago

There was a small error that has been fixed.

Sneakpeakcss commented 2 months ago

There's a crash when subtitle menu is opened when nothing is playing:

[uosc] 
[uosc] stack traceback:
[uosc]  ...mpvtest/scripts/uosc/lib/menus.lua:226: in function 'serializer'
[uosc]  ...mpvtest/scripts/uosc/lib/menus.lua:91: in function 'callback'
[uosc]  ...mpv-xmpvtest/scripts/uosc/main.lua:870: in function 'fn'
[uosc]  mp.defaults:233: in function 'fn'
[uosc]  mp.defaults:66: in function 'handler'
[uosc]  mp.defaults:385: in function 'handler'
[uosc]  mp.defaults:515: in function 'call_event_handlers'
[uosc]  mp.defaults:557: in function 'dispatch_events'
[uosc]  mp.defaults:508: in function <mp.defaults:507>
[uosc]  [C]: at 0x7ff6d29fc020
[uosc]  [C]: at 0x7ff6d29fae80
[uosc] Lua error: ...mpvtest/scripts/uosc/lib/menus.lua:226: bad argument #1 to 'gsub' (string expected, got nil)
tomasklaen commented 2 months ago

Fixed. Thx for catching that. Was about to cut a new minor.