-
Notifications
You must be signed in to change notification settings - Fork 3.2k
command: add msg command #16854
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
base: master
Are you sure you want to change the base?
command: add msg command #16854
Conversation
bd9b474 to
575a3b4
Compare
|
Download the artifacts for this pull request: Windows |
|
Can't this be a flag parameter for |
|
I feel that the correct approach here would be to implement a
@sfan5 mentioned a concern that currently all mpv commands are logged, and this can be annoying, and if indeed it is, then maybe this command could use an exception from auto-logging the log command itself. |
I feel like logging primitives are lower level than commands. Especially because commands are already logging by its own, so this would spam more than it should. Either way, I don't have strong opinion about this, if script authors prefer command, so be it. Though, for me it would be natural to expose native logging functions, same as lua/js already does. |
Scripting API's won't change. Just under the hood it will use this command instead of C code which is currently duplicated in lua.c and javascript.c. |
|
This allows C plugins, IPC clients, and libmpv to write log messages. Closes: mpv-player#14551 Closes: mpv-player#16707
This may be useful for IPC clients and libmpv users, which do not have descriptive client names (`ipc_%d`, `main`), or may want to log using a different prefixes for different components.
575a3b4 to
81d0404
Compare
|
|
||
| This line of Lua prints "foo \\{bar}" on the OSD. | ||
|
|
||
| ``msg <level> <message> [...]`` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not extend print-text command with optional arguments? It can be aliased to msg if preferred, but both do the same thing.
|
|
||
| .. note:: Lua and JS code should use the provided ``mp.msg`` modules. | ||
|
|
||
| ``msg-prefix <level> <prefix> <message> [...]`` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want that? I would say we don't let clients to change their prefix.
Or it may be additional arg in msg, but instead of replacing the existing prefix, it should create sub logger, to form <client_name>/<perfix> form.
| if (level < 0) | ||
| return; | ||
|
|
||
| struct mp_log *log = mp_log_new(NULL, mpctx->log, cmd->cmd->sender); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer to create logger during client init and set it in cmd, same as cmd->cmd->sender. sender field can be made a struct with char *name and mp_log log.
| talloc_free(log); | ||
| } | ||
|
|
||
| static void cmd_msg_prefix(void *p) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is duplicated code above, it should be made single function/command.
This allows C plugins, IPC clients, and libmpv to write log messages.
Closes: #14551
Closes: #16707