Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DOCS/man/input.rst: remove input commands subject to change heading #15276

Merged
merged 1 commit into from
Nov 8, 2024

Conversation

guidocella
Copy link
Contributor

Most of the commands list here are several years old and we will probably never change them. And it is already stated in the description of individual commands if they are deprecated or subject to change.

Furthermore, it is easy to add new commands at the end of this section by accident. I added load-config-file and load-input-conf here without realizing it's a separate section, and I assume this was also the case for begin-vo-dragging.

Most of the commands list here are several years old and we will
probably never change them. And it is already stated in the description
of individual commands if they are deprecated or subject to change.

Furthermore, it is easy to add new commands at the end of this section
by accident. I added load-config-file and load-input-conf here without
realizing it's a separate section, and I assume this was also the case
for begin-vo-dragging.
@sfan5 sfan5 merged commit 06ab962 into mpv-player:master Nov 8, 2024
6 checks passed
@guidocella guidocella deleted the commands-change branch November 8, 2024 14:27
@na-na-hi
Copy link
Contributor

na-na-hi commented Nov 8, 2024

The section exists for a reason. Input commands are subject to further scrutiny when changing compared to options and properties, and any input commands not listed in "subject to change" section cannot be broken in ANY circumstances, regardless of prior notice. This is outlined in compatibility.rst:

Critical and central parts of the command interface have the strictest requirements. It may not be reasonable to break them, and other means to achieve some change have to be found. For example, the "seek" command is a bit of a mess, but since changing it would likely affect almost every user, it may be impossible to break at least the commonly used syntax.

The "subject to change" section on the other hand is meant for the commands which are still subject to change and can be broken. By removing this section, all commands now cannot be broken for any reason anymore.

and I assume this was also the case for begin-vo-dragging.

I added that command and I was very intentional to add the command to the "subject to change" section, for possibility of further improvments or even removal.

Instead of this blanket transition, the commands should be evaluated in a case-by-case basis, and only move the commands which are sure won't change again to the main section.

@sfan5
Copy link
Member

sfan5 commented Nov 8, 2024

I did not know this section in compatibility.rst existed and it probably explains how the loadfile debacle happened.

Yet I'm not sure if we absolutely need that notice in input.rst, since apparently we have some sort of "core" set with stronger guarantees anyway.

@guidocella
Copy link
Contributor Author

Individual command descriptions already say if they are deprecated (enable-section, disable-section, define-section) or subject to change (overlay-add, osd-overlay, drop-buffers, dump-cache, ab-loop-dump-cache, ab-loop-dump-cache), we don't need a separate section for it. That secition didn't make sense as it currently was, else we could have changed some of the most common commands like cycle-values, script-message and script-binding, but not the most obscure commands in the previous section, like stop.

@na-na-hi
Copy link
Contributor

na-na-hi commented Nov 8, 2024

That secition didn't make sense as it currently was, else we could have changed some of the most common commands like cycle-values, script-message and script-binding, but not the most obscure commands in the previous section, like stop.

So just move these commands to the section where they are considered stable, like when I already suggested above? I don't get why you want to remove this section when fixing it by moving them to the stable section would achieve the same thing and is more clear than the current state. The commands need to evaluated in a case-by-case basis, and not all commands in that section need to have explicit language in the descriptions, like my intention for begin-vo-dragging.

@guidocella
Copy link
Contributor Author

That would just be moving everything without the notice except begin-vo-dragging above, it is a smaller change to just remove the heading. I think having the experimental notice in the individual command is harder to miss because if I search a command's documentation I just read wherever it is and don't notice whether it is under "Input Commands that are Possibly Subject to Change" or not, in fact I put load-config-file and load-input-conf docs there because they were next to load-script and didn't realize that was under "Input Commands that are Possibly Subject to Change". It is the same for options described as experimental, there is no separate section of experimental options. Regardless this is not important and we should decide on a case-by-case basis whether to change commands, not because they were under one section of the docs or under another.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants