scripting: add mp.input

[guidocella]

This lets scripts get textual input from the user using console.lua.

[Dudemanguy]

I'm a big dummy. What does this actually do? Prompt users for textual input?

[guidocella]

Yes, scripts can let the user type textual input and process it. I wrote 4 scripts with it if you want some use cases:

• Enter a file path to be passed to a loadfile, with case-insensitive TAB completion for files in the filesystem.
• As you type, print the playlist filenames that contain what you typed to the console's log, and press enter once the first match is the desired one (like dmenu) to switch it.
• A Lua REPL with TAB completion that can be used to test mpv's API without writing a script. Values returned by the Lua code are printed to the console's log, calling utils.to_string on tables.
• My mpv-lrc package can download lyrics from NetEase's API, but NetEase doesn't return lyrics directly, it returns a list of songs that it considers a match. With mp.input I can print 10 matches in the console's log and the user enters a number from 1 to 10 to select which lyrics to download.

Your own mpv-manga-reader could make use of this to let the user enter a page to navigate to.

The original motivation for this was letting users search for keybindings they type in the stats keybinding page.

[Dudemanguy]

Thanks, yeah that makes sense. I just wanted to make sure I had the correct idea in my mind.

[CogentRedTester]

Forgive me if I'm wrong, but it seems like you would not be able to create more than a single input in each script since the response script-message is hard-coded. Might it not be more powerful (and more expected) if each call to input.get registered a unique script-message to receive responses on? Also I'm not clear how this would handle multiple scripts trying to make input requests at the same time, could you explain that?

I've created a similar but less powerful script that does something similar (mpv-user-input), so I'm curious how you're handling some of these problems.

[guidocella]

input.get just errors if the console is already open. You can get inputs from multiple points in the same script, just not at the same time. I never understood the purpose of queueing calls in your script.

[CogentRedTester]

The purpose wasn't really for a single script to queue calls, it was to prevent multiple different scripts from interfering with each other. The idea being if a script makes an input request the user will be given the chance to answer, no matter what. I figured it would simplify things for script writers to have that guarantee. The queue code is very messy though, that whole script of mine is in need of a rewrite.

[christoph-heinrich]

The fact that any script can at any point in time just close the console or write to the log buffer while a different script is currently getting user input makes me wonder if there should be some sort of verification. But in practice that may not actually be a problem,

A queue like @CogentRedTester suggested seems like a good idea. Something simple like "if console currently open, add a table with parameters to queue." and then whenever the console gets closed have a "if something in queue, take first entry and run the get-input function with it" should work just fine.

Otherwise this looks good.

That whole "script gives suggestions to user and then processes input" thing sounds like it would pair well with the completion suggestions from my PR #10316.
It would require a few modifications to be well suited for that use case (option to keep displaying the suggestions until the user hits enter and option to choose between displaying as list or table), but that would prevent the log buffer from being spammed with completions (even if they are specific to that id) and they would be presented in a more readable manner.

[guidocella]

The fact that any script can at any point in time just close the console or write to the log buffer while a different script is currently getting user input makes me wonder if there should be some sort of verification. But in practice that may not actually be a problem,

It is intentional to allow everything to close the console to allow keybindings like g script-message-to console type 'set playlist-pos-1 ; script-message-to console disable' 20 (you can also use keypress ESC, but a script-message is cleaner and will keep working if we ever make the keybindings customizable). I don't think it's worth validate which scripts writes to the log, and even if we did a script could still send a script message directly lying about the calling script name. Scripts can alread set shared script properties to wrong values anyway.

A queue like @CogentRedTester suggested seems like a good idea. Something simple like "if console currently open, add a table with parameters to queue." and then whenever the console gets closed have a "if something in queue, take first entry and run the get-input function with it" should work just fine.

I think that you will normally call input.get from a keybinding, so it's unlikely for the console to be already open. This would complicate the already large patch further so I'll wait to see if maintainers think this should be added. Also note that event handlers can keep operating after the console closes (this is why you call input.terminate() instead of e.g. closing depending on a return value). In my mpv-lrc script you enter a number from 1 to 10 to select which Netease lyrics to download, then in the submit handler the console is closed with input.terminate(), then curl is called to download the lyrics. In this case closing the console immediately feels more responsive and allows you to press keybindings while the lyrics download.

That whole "script gives suggestions to user and then processes input" thing sounds like it would pair well with the completion suggestions from my PR #10316.
It would require a few modifications to be well suited for that use case (option to keep displaying the suggestions until the user hits enter and option to choose between displaying as list or table), but that would prevent the log buffer from being spammed with completions (even if they are specific to that id) and they would be presented in a more readable manner.

I think you're confusing TAB completion with dmenu emulation. With dmenu you only care about the first entry to see which one matches what you typed, and the entries completely change on every character you type.

[christoph-heinrich]

I think you're confusing TAB completion with dmenu emulation. With dmenu you only care about the first entry to see which one matches what you typed, and the entries completely change on every character you type.

You're right, I was thinking of TAB completion because the way I understood it was that when the user presses TAB, console sends a complete event, to which the script then responds with a complete event, which performs a completion, just like the normal TAB completion would.

[guidocella]

It does, but with "script gives suggestions to user and then processes input" it sounded like you were talking about the script I mentioned to select a playlist entry like with dmenu/autocompletion, as complete event handlers don't process anything after returning the suggestions.

When there are multiple suggestions they are added to the log, no other PR is necessary for that. To clear the previous completions I just call input.set_log({}) at the start of complete handlers.

[CogentRedTester]

This PR inspired me to do some much needed refactoring of my user-input script. I have come up with a much simpler queue implementation which can be seen in CogentRedTester/mpv-user-input#6. If you do decide to go in that direction you are free to use that approach.

[Dudemanguy]

@guidocella: would you mind dusting this one off with a rebase and bringing it back up to date? I think this is a good feature, and I want to finally actually review it. Just a couple of points:

1. Don't use shared_script_property of course. Use the newly added, user-data which is much better.
2. I believe there should be some sort of queue to handle multiple scripts requesting input. Certainly, it would be a rare event, but in practice every script is going to reasonably assume it will always work. If it doesn't, things might get weird.

[guidocella]

I had already updated this to use user-data actually and now force pushed the update. I will see about implementing a queue. Too bad it will have to be implemented in both Lua and Javascript.

[Dudemanguy]

Oh and just in case you aren't already making this assumption, it's OK to error out if there's no actual mpv window (no idea if console magically handles that already or not).

[guidocella]

The console actually accepts input just fine without any mpv window. I specifically programmed my mpv-lrc script so you can choose which lyrics to download even while playing in the terminal.

[Dudemanguy]

Oh nice, I didn't even know that.

[avih]

I don't think the queue commit is useful. It would become unclear (to the user) what's the destination of the next input. Is it a console command? input for some script which requested it while it was open? A previous script which expected one or more inputs? etc.

There's only so much you can do when a singleton user-facing things is attempted to be used by more than one thing.

I think the previous behavior where it errors (possibly with some indication of which error, possibly including an explicit identifiable case for "console in use") was better.

This way the script can display some message like "Console is currently in use".

[guidocella]

I think that the prompt that each input.get() call specifies would explain the destination of the next input. But I do agree that a queue is not worth implementing it because I can only think of use cases where you call input.get() from a keybinding, which you wouldn't activate if you already have the console open.

[avih]

It's not only a "what's the destination" issue. It's a UX issue. Think about scenarios where this could happen, and then think whether such behavior does best for the user. Really, do try to list such scnarios.

To me, the typical case of a script requesting user input in a prompt is in response to some user trigger, such pressing ctrl+/ to search the playlist, or clicking someplace with the mouse, or whatever.

Now, let's assume the console is already open for some other script.

Here's how we could get here:

The user forgot it's already open, did their trigger and typed something and enter - which now went to the wrong script.

Or they did see it's open, but thought their new trigger will now take over - and it didn't.

They most certainly did not expect that the current new trigger will "kick in" only once the current console closes. It's just not how UI works.

UI needs immediacy. User-trigger -> something happens right away (e.g. console opens and waits for input).

I just can't think of scenarios where a "open console" queue (by two different scripts) would be useful. So it's additional code and complexity and potential bugs for no value that I can identify.

[Dudemanguy]

My thought was that mp.input should block until the console is available again. Calling it a queue I guess is maybe inaccurate? I agree this is not a super likely scenario though.

Edit: maybe even make it another argument? i.e. a blocking/non-blocking request.

Ignore my bad ideas.

[avih]

What?

    log: function (message, style) {
        mp.commandv("script-message-to", "console", "log", message, style || "");
    },
    log_error: function (message) {
        mp.commandv("script-message-to", "console", "log", message, "", "");
    },

what does log_error do, exactly? Looks like it does exactly like log when style is not provided?

EDIT: unless the final empty argument is an indication that this is an error message? really?

[guidocella]

How do you want to indicate that it's an error message if you can't poss booleans with script-message? Make a new script-message? I based it on script-message osc-visibility which accepts an arbitrary final argument in the same way.

[avih]

You can make the whole message argument a single JSON string of whatever sub-values you need. Or just drop the error API and let the user use style however they want.

[guidocella]

I restored the JSON, but users are can already use any style they want by calling the regular input.log().

[guidocella]

2. input-handler.lua (name not important) - a script that contains all of the code (currently in console.lua) relating to drawing the ass overlay, accepting keyboard input/cursor controls/etc, remembering histories, receiving script messages from the modules and sending them the proper responses.
3. console.lua - a script that contains all the command evaluation/autocomplete code, handles the console.lua script-opts, receives log messages, etc. Modified so that it uses the input.lua module to actually draw the console and receive input.

console.lua would have to handle all of drawing, keybindings, histories/logs, script messages, script-opts and the completion system, and a new commands.lua would only call input.get, mp.command(line) or help_command on submit, determine completions of mpv commands, and call input.log from the log-message handler.

As I said in my earlier post I believe it will make the code more maintainable to split these two functions into separate scripts, but I acknowledge that this is subjective.
It simplifies the behaviour of console.lua as there are no longer two separate modes of operation within the same script, reducing potential bugs or edge cases.

Having two separate modes of operation is really not complex, even if we manage to reconcile the differences between console.lua and mp.input all this would buy us is moving some code to a different file and not having to manually restore the default prompt/id/history for the regular console operation. If desired, this can be done later. Complicating this PR that has been stalled for 2 years (if we include the months I worked on it before the opening a PR) further is suicide.

It means that the mp.input module works independently of the --load-osd-console option. That option would only disable the console.lua script, the input-handler would still be able to run.

In pratice there is no reason to disable the console, --load-osd-console exists so you can swap a modified copy of console.lua. Rather I think it would be confusing to have 2 separate --load-osd-console and --load-command-input-script or whatever name option corresponding to the new script, which option does what? Though not a big deal since they are for advanced users.

So the main thing that seems to be missing here is controlling the initial cursor position and setting some text to prefill the box with? Once you had those then whenever that command is sent you could terminate the input and re-open it with the new cursor position and prefilled text.

Also here are some more use cases: renaming files, saving files (with a default name), restoring the contents of the box if the input is terminated, setting a default value for anything. Honestly, I think these two are vital options that I am surprised are not included, I have made use of them myself in my scripts that use mpv-user-input.

Fair enough, I will add them.

Sounds like this can be done, it would just require the brackets to be included in the suggestions that console.lua provides?

The difference is aesthetical, and console.lua also has logic not to reinsert the append if it is already in front of the cursor. We can include appends in the suggestions, but it would be a pity to scrap the nicer system. Or let the complete callback return appends, but that would complicate the API for dubious utility.

What exactly is the issue here? Can't the tab completion part of the mp.input API already handle this?

No? Tab completion appends text, it doesn't replace it.

Remembering the line seems to be easy if there were an option to prefill the input box? Just have console.lua save the text sent to the closed callback and use it as the prefill next time the input is opened. If closed also sent the cursor position this would solve the whole issue.

I lean more towards removing this, as closing the console without running a command and having it there when you re-open it can be annoying. But indeed this is easily replicated by adding the default text and cursor arguments.

Also, I am a little unclear about the nature and purpose of the log. Are these log entries permanent lines displayed above the input box like the log messages that console.lua provides?

Yes, unless you manually clear them. I did list example usages in the third comment of the PR. The log can be used as print for the console.

As opposed to the prompt which is a string shown on the same line as the input box?

Correct.

How does the log relate to autocomplete entries?

It doesn't.

[guidocella]

Added default_text and cursor_position and some minor improvements. I also published some mp.input scripts even though they're not usable yet, so they can serve as examples of the API as you noted in point 4: https://github.com/guidocella/mpv-lua-repl https://github.com/guidocella/mpv-console-select

Edit: I also published guidocella/mpv-lrc@0179fd5

[CogentRedTester]

console.lua would have to handle all of drawing, keybindings, histories/logs, script messages, script-opts and the completion system, and a new commands.lua would only call input.get, mp.command(line) or help_command on submit, determine completions of mpv commands, and call input.log from the log-message handler.

Yes, we are on the same page. You may be right that it's too much for this PR, but I think it's worth keeping in mind for the future.

No? Tab completion appends text, it doesn't replace it.

Oh sorry, seems I misunderstood how the tab completion works.

Yes, unless you manually clear them. I did list example usages in the third comment of the PR. The log can be used as print for the console.

Good, that's what I was hoping. Now from what I understand of the code, the log buffers are unique to each id, however the input.log does not actually send the id, so console.lua just modifies the log buffer of whichever input happens to be open at the time?

If so, I think this is somewhat dangerous, all it takes is one script sending a log while another script is open to pollute the whole buffer. It also means you can't add a log while the input is closed. My immediate though about how to replicate console.lua behaviour with mp.input was to pass all the mpv log messages directly to input.log, but this would have screwed with any other script using mp.input. If the log can only be updated while open it means you need to keep a local log buffer and synchronise it with the console.lua buffer, which isn't too hard, but I don't see a reason to force script writers to do this when it can be easily handled by mp.input.

---

While thinking about this issue I came up with a slightly different interface for the mp.input module. I know you have a vision for how this module should work, but I figure I'd explain it and then you're free to ignore the concept or take any ideas you like.

I was inspired by the mp.create_osd_overlay api which acts as a thin wrapper around the osd-overlay command that handles id.
Let me explain with a code example:

-- This function creates a new input handler table.
-- All this function actually does is generate a new unique id, add it as a field to the table,
-- and add some methods to the table.
local repl = input.create_input_handler({
    prompt = '> ',
    opened = function() end,
    edited = function() end,
    submit = function() end,
    complete = function() end,
    default_text = '',
    cursor_position = 1
})

-- This just calls the current `input.add_input` function using the options passed to `create_input_handler`.
-- If a table is passed to the function then fields from it will override the fields passed to `create_input_handler`,
-- this allows things like the default text or cursor position to be modified while maintaining the same histories/log/etc.
repl:activate({default_text = 'default', cursor_position = 3})

-- sends the log commands to `console.lua` along with the id of the created input.
repl:log(...)
repl:set_log({})

It's pretty simple, just a thin wrapper around the commands you've already got, but I think it makes things a little easier to reason about and removes the need for the script writer to handle IDs. It also opens the door in the future to store other things in the table, such as a repl.activated boolean that gets set by the opened and closed messages.

I would be interested to hear your thoughts.

[guidocella]

You are only supposed to call input functions other than input.get() from within its callbacks while the console is open for your script. The log is not designed for long-time reliable storage of information since it remembers only 100 lines, you can't scrollback and it can be cleared at any time with Ctrl+l. Unless you have use cases for which the log must be appended to while the console is closed, scripts that really wnat to can just queue lines and call input.log() in opened, though every input.log() redraws the console. (input.set_log() is implemented differently precisely not to redraw on each line, but it also deletes existing lines.) Edit: actually the console does limit redraws to every 0.05 seconds, so you should have only 2 redraws.

mp.create_osd_overlay is a totally different beast because it's synchronous and each script has its own copy, so it doesn't need callbacks.

input.get scripts do not have to handle IDs. In each point where input.get is called, it is automatically assigned the script_name .. prompt ID, which is unique unless you reuse the same prompt in different points in the same script, and it is reused on subsequent calls to remember the input history and logs. The only reason you would pass the ID is if you intentionally want to share the history and log between calls with different prompts - think of an input.get call that calls loadfile append and one that calls loadfile replace. Though that is niche and I can't really think of actually useful real world use cases for it. The id argument was more of a way to document how input histories and log buffers are remembered, and can be removed if considered confusing.

-- This just calls the current input.add_input function using the options passed to create_input_handler.
-- If a table is passed to the function then fields from it will override the fields passed to create_input_handler,
-- this allows things like the default text or cursor position to be modified while maintaining the same histories/log/etc.

Changing the default text or cursor position already maintains the history and log. If you want to define default arguments and then change them, you can just store the default arguments in a variable and change some of them on each call without having to separate get into create and activate. Or wrap the input.get call in a function that accepts arguments that override those of input.get.

-- sends the log commands to console.lua along with the id of the created input.
repl:log(...)
repl:set_log({})

These should be called in the opened callback, as is documented. Doing it outside would only be reasonable with a synchronous API. input.get could have failed because the console was opened by a different script, or the console may not be enabled yet.

It also opens the door in the future to store other things in the table, such as a repl.activated boolean that gets set by the opened and closed messages.

There is already nothing stopping us from adding variables to the input table. I don't understand what activated would be for, you are the one passing the opened and closed callbacks and can already do whatever you want in them.

[Dudemanguy]

Did only a little bit of testing but it worked just fine. The code additions overall look fine and it's actually not even that much considering the feature that's being added. We can always iron out some more things later as people start actually using it. Sorry for the huge delay on this one.

[guidocella]

I added back the interface change, I removed that after a while because it continously caused conflicts.

Any bikeshedding on the names before merging maybe as we can't just change them later?

opened/closed can also be activated/deactivated or shown/hidden or enabled/disabled.
submit can also be submitted. The other events are in the past tense but I like having the same name as HTML event.
default_text can also be default_input. Text is generic, but input makes me think of an input element like in HTML instead of the text.
terminate can also be cancel. Terminate sounds a bit weird but I think cancel sounds more like ending abruptly or unintentionally which is not the case on successful submit. Unfortunately end is a reserved keyword in Lua.

[Dudemanguy]

I'm personally fine with the names as they are.

[Dudemanguy]

Let's finally get this in.