select.lua: add this script #14087
select.lua: add this script #14087
Conversation
|
Download the artifacts for this pull request: |
|
Is this meant to replace https://github.com/guidocella/mpv-console-select, for example? |
|
Yeah I just ported the code of that script. |
|
I think we could extend it with "command palette" style input. That would list all defined keybinds/commands and allow to select one of them. With common ctrl+p binding. Also we could have ctrl+shift+p to search in all of the properties and on selection send it to console as Additional, I'm personally not a big fan of EDIT: I think this selection should be bigger. Changing all console maybe is not best like #14088, but track selection and things like that should have the same size as F9 for example. It is impossible to see anything if you use it in HTPC setup. |
|
Added listings of bindings and properties and separate font and border sizes.
I prefer key sequences for being easier to press and allowing mnemonic groupings and use around 30 of them. I already find F8 and F9 bindings bad because they're arbitrary and therefore hard to remember, and we already have 4 ctrl+letter keybindings including ctrl+c and ctrl+s. It would be inconsistent to have e.g. playlist selection on ctrl but not chapter selection on ctrl+c and sub-seek on ctrl+s. We also already have 7 of these select keybindings and may add more so only |
guidocella
force-pushed
the
select
branch
2 times, most recently
from
a7ab0f8 to
5e6de1a
Compare
guidocella
force-pushed
the
select
branch
6 times, most recently
from
d71eb63 to
c06650b
Compare
DOCS/man/mpv.rst
Outdated
| Select a chapter. | ||
|
|
||
| g-s | ||
| Select a subtitle line to seek to. This requires ``ffmpeg`` in PATH, or in |
There was a problem hiding this comment.
I'm not sure about this option. All those external subprocesses to just get subtitle list looks like a hack to me.
There was a problem hiding this comment.
I think we should implement it as a property later to remove the ffmpeg dependency. It's been discussed in #12810
There was a problem hiding this comment.
I won't reject this change, because I recognize value of searching subtitle, but I feel unease about subprocess running like that. I know it is common in scripts, but first party integration should be smarter.
There was a problem hiding this comment.
Agreed but this is the best we can do until someone is willing to implement a property.
There was a problem hiding this comment.
We do cache subtitle packets now so the information is there. Of course, it can get dropped because of a sub track switch, etc.
guidocella
force-pushed
the
select
branch
2 times, most recently
from
be00189 to
541392d
Compare
guidocella
force-pushed
the
select
branch
2 times, most recently
from
4212d6d to
f419c40
Compare
guidocella
force-pushed
the
select
branch
4 times, most recently
from
fec0c91 to
34e2d0e
Compare
I misunderstood CogentRedTester's review in mpv-player#10282 (comment) as referring to the cursor_position in mp.input's arguments instead of the one received by the closed callback. The cursor_position passed to input.get doesn't need to be converted to a number because it was already JSON, while the cursor_position received by the closed callback is currently a string, and we need to pass JSON in input-event script messages to keep it as an integer to work around mpv converting integer script message arguments to string. This is more noticeable after implementing mp.input.select(): its submit argument currently receives the selected index as a string, and this makes Lua error if you use it as an index of a numerical table, e.g.: submit = function (id) mp.set_property(property, tracks[tonumber(id)].selected and "no" or tracks[tonumber(id)].id) ... end, This commit avoids having to call tonumber(id).
Avoid messing up the max_log_lines calculation when mp.input.select() is called with very long items. This doesn't work with Japanese characters because they are bigger.
selected_match can become 0 when pressing certain scrolling keybindings without any match, and for i = first_match_to_print, last_match_to_print do in populate_log_with_matches() runs from 0 to 0 and accessing matches[0].text crashes console.lua. Return early when it is 0.
With mp.input.select() these keybindings were both scrolling and moving the cursor because of how the condition was written and handle_pgup()/handle_pgdown() not returning a truthy value.
When you select an item, due to the submit handler being called asynchronously, the default item list is redrawn before the console closes, which is jarring. Fix this by always closing the console as soon as enter is pressed, as keeping it open is unlikely to be useful with a fuzzy selector (unlike with input.get() where it can be used e.g. to implement a Lua REPL). If desired we can later add a close_on_submit flag defaulting to true. Also fix a crash when pressing enter without any match.
This adds script messages to select playlist entries, tracks, chapters, subtitle lines, bindings and properties using the newly introduced mp.input.select(). This fully closes mpv-player#13964.
|
run |
|
This was shot down in #10282 (comment) |
|
Just tested this a bit, works as described. I really like it so far. But this is a feature change, should probably be mentioned somewhat prominently in the changelog. Alone for the fact that it uses a keybind ( The chord-style input ( |
|
why is there no selected style in playlist, even though the current playing file is default item when opened, it's easy to forget which one is currently playing after searching and scrolling. |
|
@verygoodlee What do you mean? The marked entry has been highlighted for me. Which build are you using, exactly? |
|
I mean this selected style In playlist I don't know which one is currently playing |
|
It's a bit noisy to have circles everywhere when the playing entry is already preselected, and it wouldn't be helpful after searching because the original order is not restored if you delete the input string anyway (though this can be easily changed). More opinions are needed on whether we want to add the circles. |
|
another question, is it necessary to observe property changes during menu opening. |
|
I don't think that's necessary. |
Requested in mpv-player#14087 (comment) and mpv-player#15031 (comment) This is mainly useful to keep highlighting the current playlist entry after moving the mouse.
Requested in mpv-player#14087 (comment) and mpv-player#15031 (comment) This is mainly useful to keep highlighting the current playlist entry after moving the mouse.
Requested in mpv-player#14087 (comment) and mpv-player#15031 (comment) This is mainly useful to keep highlighting the current playlist entry after moving the mouse.
Requested in mpv-player#14087 (comment) and mpv-player#15031 (comment) This is mainly useful to keep highlighting the current playlist entry after moving the mouse.
Requested in mpv-player#14087 (comment) and mpv-player#15031 (comment) This is mainly useful to keep highlighting the current playlist entry after moving the mouse.
Requested in mpv-player#14087 (comment) and mpv-player#15031 (comment) This is mainly useful to keep highlighting the current playlist entry after moving the mouse.
Requested in #14087 (comment) and #15031 (comment) This is mainly useful to keep highlighting the current playlist entry after moving the mouse.
This adds script messages to select playlist entries, tracks, chapters and subtitle lines using the newly introduced mp.input.select().
This fully closes #13964.