player/command: implement skippable arguments in commands

[Dudemanguy]

Optional arguments already exist, but they're not exactly skippable. If you exclude one of these arguments, everything else afterwards also needs to be excluded. In c678033, the arguments of loadfile were changed so a new index argument (optional) must always exist before you can pass the options argument. This naturally breaks the old syntax and to be fair, it is a bit weird to do something like "loadfile file.mkv append -1 options=value".

So let's make the old commands compatibile again by making the index argument skippable and adding functionality to support this. The actual command itself (cmd_loadfile) is responsible for handling this sanely.

TODO:

• See if there's a decent way to handle the bogus log messages aside from silencing in every single type. But otherwise it should work. Edit: Found an easy way to do this which should be fine.

[github-actions]

Download the artifacts for this pull request:

Windows

• mpv-i686-w64-mingw32
• mpv-x86_64-w64-mingw32

macOS

• mpv-macos-12-intel
• mpv-macos-13-intel
• mpv-macos-14-arm

[sfan5]

Honestly adding this seems weird. Why not just revert the incompatible change?
And don't we have named arguments (maybe only from ipc?)? That'd solve the compat and usability concerns.

[Dudemanguy]

What do you mean? Skipping default/implied values (like lots of cli tools do) sounds reasonable to me in certain cases if it makes sense for the command. Named arguments do exist and would indeed not have any issues in the first place, but one would need to be using mp.command_native or something similar in order to use that.

Why not just revert the incompatible change?

How would you implement the index needed for insert-at? You could move it to the end of the argument list I suppose but then you would need make some kind of special string value for the options argument so that no options are applied and then put the index argument after it. Or alternatively just not have the feature at all I guess and continue to use append with a playlist-move or something to order the list.

[na-na-hi]

How would you implement the index needed for insert-at?

I would suggest implementing a general mechanism of keyword arguments in commands (something like command foo1:bar1 foo2:bar2 for example), deprecate optional positional arguments altogether, and require all new arguments being keyword arguments. This way there will be no more backwards compatibility issues and it also allows arguments to be reordered.

[Dudemanguy]

I would suggest implementing a general mechanism of keyword arguments in commands

On its own, this is probably OK if someone wanted to add this so you could use named arguments in more contexts.

deprecate optional positional arguments altogether

I would be opposed to this however. It would break everything for mostly purely theoretical gain. The loadfile change is the first time in years someone actually changed a command in a way that wasn't exactly easy to deal with.

[na-na-hi]

It would break everything for mostly purely theoretical gain.

That's not my plan. I meant to suggest marking the maximum number of arguments of all current commands so that the existing commands won't be broken, and the parser can use this information to determine from which position it considers to be keyword arguments. So to use the keyword arguments for existing commands with optional positional arguments, all positional arguments must have some dummy values before the keyword arguments can be specified.

[Dudemanguy]

Sorry I interpreted that as eventually removing the old syntax for the original optional positional arguments at some undefined point in the future. So the proposal would be to have the old syntax live forever for those arguments essentially and only the new optional positional arguments would have to conform to the new syntax?

There's an argument for user-friendliness to be had here though. Having a simple string that's just parsed as a command without needing to do any other leg work is nice. Like console.lua does this for example so you would be making people type more things (could be added to the completion I guess but still) for these theoretical new arguments.

[na-na-hi]

Another idea is to keep the existing command syntax and introduce a new command keyword-command, which takes a command as the first argument and then an arbitrary number of keyword arguments. This would execute the command with these keyword arguments set. The existing commands won't need to be modified and new positional arguments can still be added, but would require all previous positional arguments be set with dummy values to access the last argument, so the keyword-command method is preferred for that.

[avih]

AFAICT, this PR accepts both of these forms as valid, and "does the right thing":

loadfile file.mkv append options=value

and

loadfile file.mkv append -1

(Dudemanguy) they're different option types under the hood so options=value will fail when trying to be parsed as an integer

This is a very non-generic thing. It only works in this case because the options value can never be parsed as an integer (because it has "="),

But this doesn't scale. What if the optional arg after index was an arbitrary string where "-1" would be a valid value? you wouldn't be able to tell whether that -1 is the index or the next optional string argument.

This is not a good design IMHO, and it's definitely not generic for arbitrary type of the skippable argument.

[Dudemanguy]

Strings indeed can't work like this so nevermind.

[sfan5]

How would you implement the index needed for insert-at?

If nothing else works, a new command called loadfile-insert? dunno.