Skip to content
Permalink
Browse files

options: change option parsing when using a single dash

Addresses dumb things like accidentally overwriting a media file with
e.g. "mpv --log-file test.mkv" (when the user thought that --log-file
was a flag option, when it actually takes a filename). This example will
now print an error. It still works with "-log-file overwritten.mkv", but
prints a warning.

Not sure if I'm being too careful or not "radical" enough. In any case,
both the syntax that stops working and the syntax that produces a
warning now have been discouraged and were called legacy for almost a
decade.
  • Loading branch information
wm4
wm4 committed Jan 7, 2020
1 parent 5b56be0 commit d3cef97ad38fb027262a905bd82e1d3d2549aec7
Showing with 27 additions and 7 deletions.
  1. +8 −0 DOCS/interface-changes.rst
  2. +10 −4 DOCS/man/mpv.rst
  3. +9 −3 options/parse_commandline.c
@@ -24,6 +24,14 @@ Interface changes

::

--- mpv 0.31.1 ---
- change behavior when using legacy option syntax with options that start
with two dashes (``--`` instead of a ``-``). Now, using the recommended
syntax is required for options starting with ``--``, which means an option
value must be strictly passed after a ``=``, instead of as separate
argument. For example, ``--log-file f.txt`` was previously accepted and
behaved like ``--log-file=f.txt``, but now causes an error. Use of legacy
syntax that is still supported now prints a deprecation warning.
--- mpv 0.31.0 ---
- add `--resume-playback-check-mtime` to check consistent mtime when
restoring playback state.
@@ -314,7 +314,7 @@ Legacy option syntax
--------------------

The ``--option=value`` syntax is not strictly enforced, and the alternative
legacy syntax ``-option value`` and ``--option value`` will also work. This is
legacy syntax ``-option value`` and ``-option=value`` will also work. This is
mostly for compatibility with MPlayer. Using these should be avoided. Their
semantics can change any time in the future.

@@ -324,9 +324,15 @@ because ``--fs`` is a flag option that requires no parameter. If an option
changes and its parameter becomes optional, then a command line using the
alternative syntax will break.

Currently, the parser makes no difference whether an option starts with ``--``
or a single ``-``. This might also change in the future, and ``--option value``
might always interpret ``value`` as filename in order to reduce ambiguities.
Until mpv 0.31.0, there was no difference whether an option started with ``--``
or a single ``-``. Newer mpv releases strictly expect that you pass the option
value after a ``=``. For example, before ``mpv --log-file f.txt`` would write
a log to ``f.txt``, but now this command line fails, as ``--log-file`` expects
an option value, and ``f.txt`` is simply considered a normal file to be played
(as in ``mpv f.txt``).

The future plan is that ``-option value`` will not work anymore, and options
with a single ``-`` behave the same as ``--`` options.

Escaping spaces and other special characters
--------------------------------------------
@@ -40,6 +40,7 @@
struct parse_state {
struct m_config *config;
char **argv;
struct mp_log *log; // silent if NULL

bool no_more_opts;
bool error;
@@ -52,6 +53,7 @@ struct parse_state {
// Returns 0 if a valid option/file is available, <0 on error, 1 on end of args.
static int split_opt_silent(struct parse_state *p)
{
struct mp_log *log = p->log ? p->log : mp_null_log;
assert(!p->error);

if (!p->argv || !p->argv[0])
@@ -73,18 +75,22 @@ static int split_opt_silent(struct parse_state *p)

p->is_opt = true;

if (!bstr_eatstart0(&p->arg, "--"))
bool new_opt = bstr_eatstart0(&p->arg, "--");
if (!new_opt)
bstr_eatstart0(&p->arg, "-");

bool ambiguous = !bstr_split_tok(p->arg, "=", &p->arg, &p->param);

bool need_param = m_config_option_requires_param(p->config, p->arg) > 0;

if (ambiguous && need_param) {
if (!p->argv[0])
if (!p->argv[0] || new_opt)
return M_OPT_MISSING_PARAM;
p->param = bstr0(p->argv[0]);
p->argv++;
mp_warn(log, "The legacy option syntax ('-%.*s value') is deprecated "
"and dangerous.\nPlease use '--%.*s=value'.\n",
BSTR_P(p->arg), BSTR_P(p->arg));
}

return 0;
@@ -140,7 +146,7 @@ int m_config_parse_mp_command_line(m_config_t *config, struct playlist *files,

mode = GLOBAL;

struct parse_state p = {config, argv};
struct parse_state p = {config, argv, config->log};
while (split_opt(&p)) {
if (p.is_opt) {
int flags = M_SETOPT_FROM_CMDLINE;

0 comments on commit d3cef97

Please sign in to comment.
You can’t perform that action at this time.