Skip to content
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

feat: optimize track title and update Chinese translation #975

Merged
merged 1 commit into from
Sep 3, 2024

Conversation

dyphire
Copy link
Contributor

@dyphire dyphire commented Sep 3, 2024

ref #964

@po5
Copy link
Contributor

po5 commented Sep 3, 2024

You should reuse the existing Lua pattern escape code:

local escaped_shorter_title = string.gsub(shorter_title --[[@as string]], '[%(%)%.%+%-%*%?%[%]%^%$%%]', '%%%1')

@dyphire
Copy link
Contributor Author

dyphire commented Sep 3, 2024

You should reuse the existing Lua pattern escape code:

local escaped_shorter_title = string.gsub(shorter_title --[[@as string]], '[%(%)%.%+%-%*%?%[%]%^%$%%]', '%%%1')

Missed it.

@tomasklaen
Copy link
Owner

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
Copy link
Contributor Author

dyphire commented Sep 3, 2024

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
Copy link
Owner

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
Copy link
Contributor Author

dyphire commented Sep 3, 2024

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
Copy link
Contributor Author

dyphire commented Sep 3, 2024

There was a small error that has been fixed.

@tomasklaen tomasklaen merged commit 904d23e into tomasklaen:main Sep 3, 2024
@Sneakpeakcss
Copy link
Contributor

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 added a commit that referenced this pull request Sep 3, 2024
@tomasklaen
Copy link
Owner

tomasklaen commented Sep 3, 2024

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

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.

4 participants