mp.input: allow async completions and refactor complete handling#17305
mp.input: allow async completions and refactor complete handling#17305CogentRedTester wants to merge 5 commits intompv-player:masterfrom
Conversation
2597a4b to
e2dfa14
Compare
e2dfa14 to
6377a73
Compare
|
It's simpler to just make #!/bin/sh
cd /tmp
> failed.log
mkdir mpv-scripts 2>/dev/null
cd mpv-scripts
# skip huge and/or non-script repositories
for url in $(curl https://raw.githubusercontent.com/wiki/mpv-player/mpv/User-Scripts.md | grep -Po 'https://[^)]*(?=\)\*)' | grep -Ev 'CNN|CuNNy|mpv-prescalers|rikai-mpv|mpv-composition-guides|Anime4K|Upscale-Hub|bShaders'); do
[ -e ${url##*/} ] && continue
case $url in
https://github.com/*/blob/*) curl -O $(echo $url | sed s~/blob/~/raw/~) ;;
https://gist.github.com/*/*) curl -o ${url##*/} $(echo $url | sed s~github~githubusercontent~)/raw ;;
https://raw.githubusercontent.com/*) curl -O $url ;;
https://sr.ht/*) git clone --depth=1 $(echo $url | sed s~sr.ht~git.sr.ht~) ;;
https://git.sr.ht/*/tree/*) curl -O $(echo $url | sed s~/tree/~/blob/~) ;;
https://www.npmjs.com/package/*) npm i ${url#*/package/} ;;
https://gist.github.com/igv) ;; # unsupported, it's a list of gists
https://www.npmjs.com/search?q=keywords:mpv-script) ;; # not a script
*)
url=${url%\#*}
# converts e.g. https://github.com/paradox460/mpv-scripts/tree/master/writeedits
# to https://github.com/paradox460/mpv-scripts
case $url in
https://github.com/*/*/*|https://codeberg.org/*/*/*) url=$(echo $url | cut -d / -f 1-5) ;;
esac
# multiple users have a mpv-scripts repository, so include the
# username in the directory
dir=$(echo $url | cut -d / -f 4)_$(echo $url | cut -d / -f 5)
[ -e $dir ] && continue
git clone --depth=1 $url $dir || echo $url >> /tmp/failed.log ;;
esac
doneand then |
I have updated the documentation to only mention the function method. Do you think we should add a deprecation notice? (in the documentation and/or a printed warning when done in the code) |
b380826 to
809e8ef
Compare
|
I guess the check to call I think adding a mention in interface-changes is enough since nobody uses this. |
|
Upon further reflection, I believe that the removal of the counter means that we need to also send |
|
In theory you could change the input line back and forth until completions arrive, but even if you revert to the line of when completions were requested then the completions for that old line will apply so what's the matter? |
|
Maybe I have misunderstood how the completions are supposed to work. My interpretation was the following:
However, in the current code, step four above does not happen (at least not accurately). This is because the code that tests for this scenario: if message.client_name ~= input_caller or message.handler_id ~= input_caller_handler
or line ~= completion_old_line or cursor ~= completion_old_cursor then
return
enddoes a comparison with the Normally, this would be difficult to detect, as the responses occur so quickly that the incorrect completions are never actually drawn on the screen, but if the delay is larger (which may happen if we're doing asynchronous I/O) it will be very much noticeable. You can test this on master right now by adding a busy-loop to local function complete(before_cur)
...
...
...
local t = mp.get_time()
while (mp.get_time() - t < 1.5) do end
return completions or {}, completion_pos, completion_append
endThen, in Assuming I have understood all of this correctly, we probably want to either ensure that a client cannot send old completion events (the counter approach I originally proposed), or we have the clients send the |
274d155 to
5df4c58
Compare
|
I have added a commit that has the clients send the original line to |
|
Huh you're right my old line logic was completely non-functional lol. Clearing the completion buffer on cursor move would indeed be better. I think you can
if line ~= "" then
complete()
elseif open then
-- This is needed to update the prompt if a new request is
-- received while another is still active.
render()
endwith |
|
Actually nevermind that looks terrible if completions are replaced immediately e.g. in commands.lua because they quickly disappear and reappear over and over. |
|
I will experiment with some different options to clear/deactivate the completions and see if there is a way to prevent incorrect completions without visual regressions. |
3ce6546 to
ac5c0c8
Compare
|
In the latest commit, instead of clearing the completion buffer, I am setting a The only time when the completion flickering is more noticeable is when the completion options do not change while typing (for example typing the second half of For terminal output mode I am just not drawing the completions, I did not notice any visible flicker on my terminal. |
|
While the flicker is small I still don't think it's worth adding when commands.lua completions is all 99% of users will ever use. I checked what neovim does and until new completions arrive, it filters through the existing completions based on the new input line. You can try to do that if you want to. EDIT: well that may still look bad in our case because of the grid completions instead of column... |
ac5c0c8 to
ed54c17
Compare
Previously, it was impossible to generate a list of completions for an `input.get()` request asynchronously, as the values had to be returned by the callback function. This commit provides a way to send completions asynchronously by passing a second argument to the complete callback; a function which can be passed the return values to respond to the complete event.
Updates to use the new response function passed to `complete` events to send completions back to `console.lua`. This is to pre-empt the old return value method being deprecated.
Previously, completion responses from the same client would be displayed even if the user had since changed the input, as long as a newer complete event had already been sent. This meant that, if clients took a sufficiently long time to return completions, then console.lua would display completions for previous versions of the line which were no longer valid. This commit has the client send back the version of the line their completions apply to, ensuring that `console.lua` displays the correct completions.
ed54c17 to
14b5a33
Compare
|
I have added a short timeout before the completions are faded. On my device there is no visible flicker when using I have also added a sentence to the documentation clarifying that the response function should be called with an empty table if there are no completions to display. Without doing so the old, dimmed completions would be displayed until the input is closed. |
14b5a33 to
6fc6cbd
Compare
This commit flags when console.lua is waiting for completions, and disables the completion cycle key until up-to-date completions are received. While this is happening, completions are not printed when in terminal output mode. In normal OSD mode, the completions are still printed, but at reduced opacity. This is to avoid distracting and ugly flickering from completions being rapidly erased and redrawn. The completion results are only dimmed after a short (sub-second) delay. This is to avoid distracting flickers from the opacity rapidly changing for clients that return completions almost immediately, e.g., commands.lua.
6fc6cbd to
4906b74
Compare
Previously, it was impossible to generate a list of completions for an
input.get()request asynchronously, as the values had to be returned by the callback function.This PR provides a way to send completions asynchronously by passing a second argument to the complete callback; a function which can be passed the return values to respond to the complete event.
This PR also refactors how
console.luahandles completions so as to avoid presenting the user with outdated completions, or allowing the user to select them. The old code was fundamentally broken, something that went undetected becausecommands.lua, the only script using this API, responded so quickly that no-one ever noticed the inconsistencies. To prevent visual flickering, old completions are dimmed to indicate to the user they are disabled.Motivation
Some scripts may not want or be able to generate the completion results synchronously. For example, they may want to do asynchronous IO for performance reasons, or the design/code of the script may simply not allow certain information to be collected synchronously.
My file-browser script is a good example. I would like to be able to use the file-browser code to read the contents of directories so that I can complete paths automatically in input boxes. However, file-browser reads directories asynchronously for performance reasons, and it is now so embedded into the code architecture that it is not viable to change or disable it.
Examples
Here is a basic example of a script responding asynchronously with an exaggerated delay:
And here is a more complex example that uses
lsto complete file paths to send toloadfile: