input: add a way to pipe multiple commands #15921
Conversation
|
Download the artifacts for this pull request: |
Dudemanguy
force-pushed
the
input-conf-pipe
branch
2 times, most recently
from
733893d to
4e82d26
Compare
|
One thing to consider is that Bonus chatter: one thing that is probably out of scope of this PR is basic manipulation of the data, say we get node map and just want to extract one value. This could be done similarly to how |
|
I don't think this a bad idea in principle. But it's just hard to imagine the need ever arises outside of maybe the eventual |
Dudemanguy
force-pushed
the
input-conf-pipe
branch
from
4e82d26 to
9d4dc3b
Compare
|
Quoting should work properly now. Moving the command result around is kind of awkward, but didn't really think of anything better. |
|
I understand the value of this being nicely generic, and also that this is presumably a sort of proof of concept and not necessarily a fully-baked thing (judging by "Very rough right now and needs work. But this is mainly to show that what I mentioned in ... is totally doable."), so I do keep this in mind below. Do you have real-world use cases in mind where this can be useful to do things which are impossible or inconvenient with the current input.conf syntax, other than And specifically with Still in the context of file-dialog only, I made a more detailed comparison of the pipe approach here with a fictional approach of a And while few of the bullets there were addressed by @Dudemanguy, I think most of them were not. So maybe here is a better place to continue that subject? |
|
As a proof of concept could we fix clipboard in this PR? Replace with comething like The main objective would be to fix case like this one #15576 (comment) but not limited to we can also prevent passing
See the above.
The main idea was to replace |
|
Edit: sorry my mistake. The parsing in this case succeeds but there's no data and indeed no error message so it would work. |
This is cute, but the suggestion at the reply to that comment feels to me much simpler - to support something like
That sounds interesting, but I don't see how this improves with the pipe. Wouldn't loadfile execute either way, regardless if it gets an error or something else which indicates "no value" directly from
Huh... so you guys don't see the pipe as a viaable alternative to modifying specific commands for file-dialog support? Is the As I said, I do get that it's nicely generic and might be able kill many stones with one bird(!), but at least with the few cases mentioned here (clipboard sanitation, dialog -> set-property, and tailored-dialog-options-for-commands) this is either already implemented or insufficient or might be replaced with something specific and simple which fits the context (sanitation). |
This works only for clipboard,
That's up to discussion. I don't think pipe chain should continue if any of the command in chain failed. What would be the point? |
Yes, but the part I wrote which you forgot to quote is that currently this is the only case which we know of which needs sanitation. It's obviously trivial to come up with other cases, but not ones which are unavoidable normally. The clipboard on win11 from explorer is the only real use case which needs a solution, which I know of at least.
I see, so you're adding features, and it grows into a mini shell. Off the top of my head, I think it can make sense with a pipe, but I didn't think about it enough. At least with a shell, which I presume is the inspiration for this enhancement of input.conf syntax, it doesn't stop at the first command which fails, or even care about the exit code of any command except the last (of course, we do have pipefail with POSIX 2024, and earlier in some shell, but that is not the default, and it still doesn't stop the other commands at the pipe). In this context, there are other concerns I raised at my post at the other PR regarding the pipe, about who interprets Eventhough I very much like the concept of a pipe, and its generality, even in mpv, and even considering that this may be useful to more things than we know of currently, from what I've seen so far, it feels to me that addressing each concern individually is still a better approach. The clipboard error specifically does pose an issue which I think can indeed be solved well using pipe+abort-on-first-error approach suggested here, but I didn't think about it enough, and there might be other satisfactory solutions besides the pipe. |
Dudemanguy
force-pushed
the
input-conf-pipe
branch
from
8134566 to
493bf81
Compare
I added a
The current implementation is that it does continue. At least as far as shell semantics goes, that would be the expected behavior as well.
I'm not sure I understand what the concerns are. |
Dudemanguy
force-pushed
the
input-conf-pipe
branch
from
493bf81 to
adff8a4
Compare
What happens if there are two unquoted Hmmm.. is this an extension of the input.conf syntax? I believe until now there was zero difference between using Does this difference only exist when the command is part of a pipe? does it apply to the first command in a pipe too? Will my input.conf which I wrote yesterday and has unquoted This specific thing is a very slippery slope IMO in making the input.conf syntax more complex and fragile than it needs to be. This is not completely without precedent though. In (posix) shell, there can indeed be difference in few cases of strings which looks completely normal but behave differently depending on quoting (like the here-document boundary word, which elsewhere is interpreted identically regardless of quotes, but in here-doc it specifies whether the content would be expanded or not, and few other cases). However, these are also the more notorious parts of shell syntax, and IMO this behavior creep from shell is an extremely slippery slope which may be very hard to stop as more people would start using this. We need to think very hard and long about where this could go. The pipe is cute, and so is the abort-on-first-error behavior, but I do not think mpv should implement mini-shell which interprets input.conf syntax. We already support two builtin scripting languages which are exactly intended for things which are hard or impossible to do at input.conf. We don't want to make input.conf a NIH scripting language IMO. |
Dudemanguy
force-pushed
the
input-conf-pipe
branch
from
adff8a4 to
8c3eadd
Compare
Very similar to typical shell semantics. If a command writes output to a result, the next command can now read those results and use it. The current implementation assumes that the only command results will always be strings and does not attempt to deal with any other type. This can always be expanded in the future if a need arises.
Dudemanguy
force-pushed
the
input-conf-pipe
branch
from
8c3eadd to
f9353f7
Compare
Both are substituted. I'm not sure if there's any practical use for this, but I see no reason to disallow it and it would be more work to only apply it to one -.
Of course, there's no other way this could work.
So I just updated the implementation a little bit that I think addresses this in a reasonable way. The only unconditional special character this PR adds is
FWIW, there is no abort-on-first-error behavior currently. I'm undecided on that because |
Of course there is. If the command itself decides how to interpret EDIT: the Admittedly, I have zero experience in language design, and this is completely outside of my comfort zone, but I can still see clearly what it does and compare it to similar platforms and their advantages and weaknesses in historical perspective (shell). The implementation of
Agreed, it's a bit better, and now also well specified. But the other factors remain.
I know and understand. The abort part was suggested just now, and I actually like it, regardless if it ends up implemented or not. I said what's here is cute, even ignoring necessity or alternatives. But overall, in the big picture, you have to agree that this looks like behavior creep from shell, with shortcuts to simplify the implementation (like identifying And we already have two proper languages which are well specified and documented and which can do real procedural behavior, including what the pipe feature supports (i'm not even suggesting to write a script which does pipes, but it's entirely possible). I get that it's fun to write, and maybe with some real immediate value in specific use cases, but I think the cost and the technical debt in the long term is not worth it, especially when we already have lua and js to complement input.conf in a very meaningful way. |
No worries. The
I don't understand why you're calling it a shortcut.
input.conf is great because it allows users to do fairly complex things without having to write a single line of code. Sure I fully understand that there is a limit and we shouldn't be trying to turn it into some kind of 3rd pseudo language. But this is PR is nowhere near that. You can bring up this point if someone tries to implement data structures or something in input.conf. This is a very natural piping idiom that even noobs at the shell can use and understand.
Can you actually explain in concrete terms what "cost" and "technical debt" are being incurred here? |
Sure. Both of them refer to the long term. Technical debt means that shortcuts are taken now because it's easier, but they may make it hard to extend generically or maybe require workarounds in the future which we don't envison now, because they're not the "real" correct way. This referred specifically to interpreting The long term cost is both in terms of supporting this mini language, handling issues, and potentially enhancing it further as deficiencies are identified or new enhancements are identified as useful. This is not a new language which you're desiging now from scratch and you can make great decisions about how the parts fit together. It already exists and it's already complex, and any new addition - like this pipe, adds on top of that. One way to look at it is "it's only a pipe and it's easy and useful and we can always consider each enhancement individually", and I can even agree with most of those. Another way is to take a step back and look at what it already is, what's the potential trajectory here, assess the overall value which it adds vs the altrenatives which already exist or which can be created with some effort, and try to forsee whether this is a good path to walk in. All of these are obviously subjective and no two people would have identical points of view. But for me, it feels like not a good path, especially considering the exting complexity of input.conf syntax, and the existing lua/js support in mpv which can extend it effectively as much as someone wants to put effort into it. |
I don't agree.
I find this entire argument to be incredibly disingenuous. This is not a "mini language". There is no intention to make a "mini language". This does not even have 1% of the features I would expect from any language. Please stop calling it what it is not. You keep making claims about "complexity" but have not actually quantified actually is complex or so hard to support. Again, this feature is merely a simple addition to the already existing multi command execution that we have in
This is just fearmongering. It's a generic, simple solution that can solve usecases previously discussed and possibly more. Vague appeals to how somehow this is going to lead to some dark trajectory doesn't make sense and honestly is just nonsense.
I don't think input.conf syntax is complex. |
|
And just to not be completely negative, here's something which I do see as a good path to walk in: all the recent UI enhancements using scripts, mainly select.lua and the ever-extending use cases where it applies. I think it's great, it adds value to users almost without affecting development of the core of mpv (up to new support functionalities at the core which are not simple or practical to keep in a script). That's obviously off topic, it's not black and white either, and supporting these scripting UI enhancements is also not without cost, but it's contained in a way which I think minimizes the long term cost. This is obviously a subjective view as well. |
I think what @avih is trying to say is not that people should be writing a Lua script to do this, but that a Lua script could easily be created that performs this piping operation without needed to modify the player core. I wanted to test this out so I tried writing a script that pipes the output of one command to another, and it turns out to be pretty simple: local mp = require 'mp'
local msg = require 'mp.msg'
local err_obj = {}
local function main(...)
local args = {...}
if #args == 0 then return end
local commands = {{}}
local tail = 1
for _, arg in ipairs(args) do
if arg == '|' then
tail = tail + 1
commands[tail] = {}
else
table.insert(commands[tail], arg)
end
end
local carry = nil
for i, command in ipairs(commands) do
for j, arg in ipairs(command) do
if arg == '-' then
command[j] = carry
end
end
local res, err = mp.command_native({'osd-auto', unpack(command)}, err_obj)
if res == err_obj then
mp.osd_message('error: '..err)
msg.error(err)
end
carry = res
end
end
mp.register_script_message('pipe', main)With this a simple entry in <key> script-message pipe normalize-path "${filename}" | show-text -Allows return values from commands (the few that have them) to be inserted into the next command. Obviously this is a very naive example that is likely buggy and with little to no error handling or input validation, but it shouldn't be difficult to add, along with other features. The only downside is the |
And I never had issues with this part. We agree this should be interpreted by the parser/"shell". I already said that explicitly before.
Is it really not enough to answer it in one word: shell? The shell doesn't have knowledge of it, and yet pipe has magically worked and still works for about 5 decades now. The connection which the shell identifies is the pipe syntax. Not the So as much as you see Maybe it's harder to implement for mpv without identifying I'm not arguing about the rest because it's subjective. I can only say how I feel about it, and I'm definitely not saying that your point of view is wrong. |
Of course and a lua script could easily be created to execute multi commands sequentially too. Why don't we delete all the core input and command code that handles multiple commands and tell people to use scripts?
That entire explanation was in terms of mpv not the shell. It is not the same thing and I'd appreciate it if you would not conflate it.
What? How do you tell the difference between |
It's not of course. I did not say that and I did not try to say that. I even said explicitly that I'm not suggesting to implement pipe as a script. Don't put words into my mouth, and don't jump blindly on any suggestion that this is what I wanted to say, especially when I explicitly said exactly the oposite. Just so that you don't have to look it up:
What I said and meant is that input.conf can already be extended in infinite ways using scripting, and therefore the enhancement of input.conf syntax is not an insurmountable limitation, and there's no real need to extend it meaningfully, be it pipes or whatever else. This doesn't include fixes or even minor refinements, which of course should be implemented if issues are identified. (please let's not start an argument about the threshold for "minor refinements") There was also no suggestion about convenience. I certainly agree that it would be nicer to use pipes directly at input.conf (if the user can find a real use for it) rather than writing a script or using a 3rd party script which achieves a similar goal. I even called it "cute" more than once. I did mean it. It's cute and convenient. Heads up, just in case you missed the last line, please don't try to convince me that it's more convenient at input.conf. I think it's quite obvious that it is indeed convenient. Also, in case you forgot or missed that, because this seems to be a highly recurring theme, I'm not suggesting to replace pipes or any other core part of mpv with a script. Can I count on you to remember this? Do you need me to put a sticky note someplace so that you can come back to this message and re-check just to be sure whether or not I'm suggesting to replace pipes with a script? The point of this part was to say that it's possible also without input.conf pipes syntax.
This reply was about the general notion that identifying If all your statements which suggested that it must be interpreted by the parser were meant explicitly in the mpv context, then I misunderstood. I'm going to assume that I misunderstood, and that indeed you meant exclusively in the context of mpv, so let's move on.
First, as you can tell from that quote, I never suggested that you or the parser or anyone should be able to tell difference between quoted and unquoted What I also suggested, is that the parser should not even care about But whatever the exact quesiton was, the honest answer is that I don't know because I didn't look at this code recently, and even if I looked, I can't know whether I would be able to understand it enough to give you a straight and fully working alternative to identifying Back to the answer I can give. I hope I'm not doing an injustice, but I'll answer the question "how can it work if the parser doesn't identify which argument should be replaced?". One approach I could imagine is that the context which the command implementation takes as argument has a field which represents "output of the previous command in a pipe". Elsewhere, "stdin" is the standard input place where applications check "input from someplace", so let's call this command context field "stdin" as a temporary name. Do correct me if I'm wrong, but I'm assuming the mpv "shell" (parser, however you want to call it) already has to execute the commands in order, because it should be able to replace Let's say that the input.conf line is So when it identifies the pipe syntax, and after it got the output of Then If it supports taking input from a previous command, then it checks its stdin. If it doesn't support it, then it doesn't check its stdin. If it only supports it in place of a We can even do here things which don't work with normal pipes, like making it possible to identify at the the value of stdin whether there was no previous command in the pipe, or there was a previous command which for some reason didn't have an output - maybe like if there was some error there. Does this answer the question of "how to do pipes without the parser identifying which argument should be replaced"? I hope it does answer, at least in rough outline, the "it's not possible" and "how can it be possible" questions. However, even if it is possible, this IS work. The parser part maybe not so much, probably even less work that identifying Maybe there's some way which could make most of them do that automatically. Maybe there isn't, and indeed it requires tedious modifications of commands. It's entirely possible that it's the latter, which probably makes it hard, but not impossible. And this is what I meant in what you quoted me.
The concrete reason is that it adds special cases to the syntax where the same This is an idiosyncrasy, just one more on top of the existing ones, which makes input.conf syntax just a little bit harder to grok than before, and this is what I call increasing technical debt, because it's another factor which you can't forget about from now on. Mandatory reference: Monty Python's The Meaning of Life, the restaurant scene. |
What? So now you just randomly decided that we shouldn't attempt to identify the string literal "-"? Why? This is just after I made a few changes to ensure that it would still work like before. Unbelievable. You have to be kidding me you literally said earlier:
That statement quite obviously implies that you expected the quoted
We can and already do that with my solution.
No it doesn't answer the question because you ignored it, arbitrarily changed the terms and answered something else completely.
This is not what the term "technical debt" means and it's frankly insulting that you insist on calling an enhancement to input.conf syntax that can be implemented in an almost perfectly backwards-compatible way (literally the only thing that would break would be something like Also it is ridiculous that you call this solution "technical debt" and then turn around and argue that individual commands should all be explicitly modified with some
Sorry I need to address this. I can't let you get away with all these endless normative statements. Anyways, no this not "work". This is codemonkey levels of "work" where you go back and modify each command individually. It is not hard. It's just stupid and why would you do this when there are better solutions. I am very sick of this discussion. Please come back with actual patches, point out something wrong with the code, etc. Dodging points I've made and constantly gaslighting me is not helpful. |
I didn't randomly decide anything. Look. It's right there at the whole progression of this line of discussion. I said something very specific, "Maybe it's harder to implement (pipes) for mpv without identifying - at the parser". You quoted that and said "What? How do you tell the difference between "-" and - in input.conf if the parser isn't identifying it?" And I want to answer it, but I don't know how, and I don't know why you're asking me. I never said someone should be able to tell this difference, and I don't know the answer to that question, or even why it matters to begin with. I said the oposite, and thought, apparently wrongly, that you wanted clarification on how it's even possible. So I then then gave a suggestion of how it can work wihout telling the difference and without the parser even caring. Now, you said so very explicitly, so I have to believe you that it doesn't answer your question. Please believe that I did my best, but apparently failed. Reminder, my only claim and suggestion in the context of I hope we can agree that these are the things I said about the So do you have a question about these things, or other things which maybe I can help clarify in the context of "I don't think identifying '-' belongs in the parser"? |
|
This a complete and utter waste of my time. The general problem of "identifying -" includes, quite obviously, telling the difference between the string literal variant People that have actual productive things to say are always welcome. |
I could say exactly the same about you, but I won't, because this won't help the discussion in any way. So I give you the benefit of doubt that it was in good faith. I can say about myself without any doubt that it was in good faith, and I hope you could accept it as well.
Considering how it went so far, I can't blame you, and I'll do the same. Both because it takes two to tango, but also because I don't have anything else to say which I didn't already say before. But if at some stage you have some question which you think was not answered by what I said above, and you do want to hear my reply, then do ask. |
Very rough right now and needs work. But this is mainly to show that what I mentioned in https://github.com/mpv-player/mpv/pull/15845/files#r1962613721 is totally doable.
This lets you pipe multiple commands together in input.conf style syntax like:
expand-text "${chapters}" | show-text -where|is the pipe of course and-tells the command to read the result from the preceding command. Chaining them together infinitely should work too:expand-text "test" | expand-text - | expand-text - | expand-text - | show-text -You can also specify-for multiple arguments to have the same thing substituted multiple times but not sure that has any practical use.This only makes sense if the previous command outputs an actual result. AFAIK most of them don't, but you can use
expand-textascatbasically so that's handy and the property expansion lets you do silly things likeexpand-text "${seeking}" | set vo -.Main open question right now is: how many types should we even attempt to handle with this? I'm not even sure if any command right now returns anything other than a string in the result. I know in #15845
file-dialogstores a node array so that needs to be implemented here. But that's just a bunch of strings. I'm thinking the node won't ever realistically have anything other than a string or strings in it, but could be wrong.