From d3cef97ad38fb027262a905bd82e1d3d2549aec7 Mon Sep 17 00:00:00 2001
From: wm4 <wm4@nowhere>
Date: Tue, 7 Jan 2020 23:08:45 +0100
Subject: 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.
---
 DOCS/interface-changes.rst  |  8 ++++++++
 DOCS/man/mpv.rst            | 14 ++++++++++----
 options/parse_commandline.c | 12 +++++++++---
 3 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/DOCS/interface-changes.rst b/DOCS/interface-changes.rst
index e6e1eb317c..2c3cae76de 100644
--- a/DOCS/interface-changes.rst
+++ b/DOCS/interface-changes.rst
@@ -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.
diff --git a/DOCS/man/mpv.rst b/DOCS/man/mpv.rst
index a014388877..676cd1897f 100644
--- a/DOCS/man/mpv.rst
+++ b/DOCS/man/mpv.rst
@@ -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
 --------------------------------------------
diff --git a/options/parse_commandline.c b/options/parse_commandline.c
index e89239c33b..f5ff330230 100644
--- a/options/parse_commandline.c
+++ b/options/parse_commandline.c
@@ -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,7 +75,8 @@ 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);
@@ -81,10 +84,13 @@ static int split_opt_silent(struct parse_state *p)
     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;
-- 
cgit v1.2.3