plots: quote and resolve paths properly (iterative#5670)

This commit fixes 3 things:
1. The path printed on the terminal is properly is quoted.  This was
problematic when the output is piped to `xdg-open` if the path contained
spaces or quotes or special characters.

2. The path is resolved. This is not an issue at all, but makes the
output nicer.

3. The path with which the browser is opened with uses relative path.
This is done to solve issue in WSL, that the absolute path is not
accessible outside the WSL for host programs as the linux filesystem
is available on `/wsl$` path.

dvc/command/plots.py

@@ -1,6 +1,5 @@
 import argparse
 import logging
-import os
 
 from dvc.command import completion
 from dvc.command.base import CmdBase, append_doc_link, fix_subparsers
@@ -23,6 +22,8 @@ def _props(self):
         return {k: v for k, v in props.items() if v is not None}
 
     def run(self):
+        from pathlib import Path
+
         if self.args.show_vega:
             if not self.args.targets:
                 logger.error("please specify a target for `--show-vega`")
@@ -41,20 +42,20 @@ def run(self):
                 logger.info(plots[target])
                 return 0
 
-            path = self.args.out or "plots.html"
-            path = os.path.join(os.getcwd(), path)
-
+            rel: str = self.args.out or "plots.html"
+            path = (Path.cwd() / rel).resolve()
             write(path, plots)
-            url = f"file://{path}"
         except DvcException:
             logger.exception("")
             return 1
 
+        assert path.is_absolute()  # as_uri throws ValueError if not absolute
+        url = path.as_uri()
         logger.info(url)
         if self.args.open:
             import webbrowser
 
-            opened = webbrowser.open(url)
+            opened = webbrowser.open(rel)
             if not opened:
                 logger.error("Failed to open. Please try opening it manually.")
                 return 1

tests/func/plots/test_show.py

@@ -708,7 +708,7 @@ def test_show_from_subdir(tmp_dir, dvc, caplog):
     with subdir.chdir(), caplog.at_level(logging.INFO, "dvc"):
         assert main(["plots", "show", "metric.json"]) == 0
 
-    assert f"file://{str(subdir)}" in caplog.text
+    assert subdir.as_uri() in caplog.text
     assert (subdir / "plots.html").exists()
 
 

tests/unit/command/test_plots.py

@@ -1,4 +1,8 @@
 import logging
+import os
+import posixpath
+
+import pytest
 
 from dvc.cli import parse_args
 from dvc.command.plots import CmdPlotsDiff, CmdPlotsShow
@@ -116,12 +120,11 @@ def test_plots_diff_open(tmp_dir, dvc, mocker, caplog):
     )
 
     assert cmd.run() == 0
+    mocked_open.assert_called_once_with("plots.html")
 
-    expected_url = f"file://{tmp_dir / 'plots.html'}"
+    expected_url = posixpath.join(tmp_dir.as_uri(), "plots.html")
     assert expected_url in caplog.text
 
-    mocked_open.assert_called_once_with(expected_url)
-
 
 def test_plots_diff_open_failed(tmp_dir, dvc, mocker, caplog):
     mocked_open = mocker.patch("webbrowser.open", return_value=False)
@@ -132,12 +135,36 @@ def test_plots_diff_open_failed(tmp_dir, dvc, mocker, caplog):
     )
 
     assert cmd.run() == 1
-
-    expected_url = f"file://{tmp_dir / 'plots.html'}"
-    mocked_open.assert_called_once_with(expected_url)
+    mocked_open.assert_called_once_with("plots.html")
 
     error_message = "Failed to open. Please try opening it manually."
+    expected_url = posixpath.join(tmp_dir.as_uri(), "plots.html")
+
     assert caplog.record_tuples == [
         ("dvc.command.plots", logging.INFO, expected_url),
         ("dvc.command.plots", logging.ERROR, error_message),
     ]
+
+
+@pytest.mark.parametrize(
+    "output, expected_url_path",
+    [
+        ("plots file with spaces.html", "plots%20file%20with%20spaces.html"),
+        (os.path.join("dir", "..", "plots.html"), "plots.html"),
+    ],
+    ids=["quote", "resolve"],
+)
+def test_plots_path_is_quoted_and_resolved_properly(
+    tmp_dir, dvc, mocker, caplog, output, expected_url_path
+):
+    cli_args = parse_args(
+        ["plots", "diff", "--targets", "datafile", "--out", output]
+    )
+    cmd = cli_args.func(cli_args)
+    mocker.patch(
+        "dvc.repo.plots.diff.diff", return_value={"datafile": "filledtemplate"}
+    )
+
+    assert cmd.run() == 0
+    expected_url = posixpath.join(tmp_dir.as_uri(), expected_url_path)
+    assert expected_url in caplog.text