Fix file dialogs on Windows & Macos

.gitignore

@@ -28,6 +28,7 @@ deploy_*.py
 build/
 ui/dist/
 .venv/
+.venv
 .local
 .~*.md
 .ploy

docs/source/changelog.rst

@@ -1,6 +1,11 @@
 Changelog
 =========
 
+0.4.5
+-----
+
+* Fixed some issues with file/directory selection dialogs on Windows and MacOS
+
 0.4.4
 -----
 

docs/source/troubleshooting.rst

@@ -123,8 +123,15 @@ Application runs, but something’s gone wrong
 
   * Refresh the page
 
+* On MacOS, file/directory selection windows don't appear (e.g. when selecting a file to load or a directory to save to)
+
+  * ``tkinter`` `doesn't work out of the box on MacOS <tk_macos_>`_, try installing it via `Homebrew <brew_>`_::
+
+        brew install python-tk
 
 .. _shapeflow-releases: https://github.com/ybnd/shapeflow/releases
 .. _add-path-win10: https://www.architectryan.com/2018/03/17/add-to-the-path-on-windows-10/
 .. _cairo: https://www.cairographics.org/manual/
-.. _cairo-windows: https://github.com/preshing/cairo-windows
+.. _cairo-windows: https://github.com/preshing/cairo-windows
+.. _tk_macos: https://www.python.org/download/mac/tcltk/
+.. _brew: https://brew.sh/

shapeflow/cli.py

@@ -628,15 +628,15 @@ def command(self) -> None:
             self._reclutter()
 
     def _declutter(self) -> None:
-        if os.name == 'nt':
+        if sys.platform == 'win32' or sys.platform == 'cygwin':
             # Pre-emptively create __pycache__ so we can hide it now.
             if not os.path.isdir('__pycache__'):
                 os.mkdir('__pycache__')
 
             for file in self.CLUTTER + glob.glob('.*'):
                 if Path(file).exists():
                     os.system(f'attrib +h "{file}"')
-        elif os.name == 'darwin':
+        elif sys.platform == 'darwin':
             for file in self.CLUTTER:
                 if Path(file).exists():
                     os.system(f'chflags hidden "{file}"')
@@ -645,11 +645,11 @@ def _declutter(self) -> None:
                 f.write('\n'.join(self.CLUTTER))
 
     def _reclutter(self) -> None:
-        if os.name == 'nt':
+        if sys.platform == 'win32' or sys.platform == 'cygwin':
             for file in self.CLUTTER + glob.glob('.*'):
                 if Path(file).exists():
                     os.system(f'attrib -h "{file}"')
-        elif os.name == 'darwin':
+        elif sys.platform == 'darwin':
             for file in self.CLUTTER:
                 if Path(file).exists():
                     os.system(f'chflags nohidden "{file}"')

shapeflow/main.py

@@ -391,6 +391,12 @@ class _Filesystem(object):
     def __init__(self):
         self._history = History()
 
+        if not filedialog.ok:
+            log.error(
+                f"File dialog was resolved to {filedialog.__class__.__name__} "
+                f"but this implementation doesn't work on this system"
+            )
+
     @api.fs.select_video.expose()
     def select_video(self) -> Optional[str]:
         """Open a video selection dialog

shapeflow/util/__init__.py

@@ -8,9 +8,8 @@
 from pathlib import Path
 import json
 from logging import Logger
-from distutils.util import strtobool
-from functools import wraps, lru_cache
-from typing import Any, Generator, Optional, Union
+from functools import wraps
+from typing import Generator, Optional, Union
 from collections import namedtuple
 import threading
 import queue
@@ -277,9 +276,9 @@ def open_path(path: str) -> None:
     if os.path.isfile(path):
         path = os.path.dirname(os.path.realpath(path))
 
-    if os.name == 'nt':  # Windows
+    if sys.platform == 'win32':
         os.startfile(path)  # type: ignore
-    elif os.name == 'darwin':  # MacOS
+    elif sys.platform == 'darwin':  # MacOS
         subprocess.Popen(['open', path])
     else:  # Something else, probably has xdg-open
         subprocess.Popen(['xdg-open', path])

shapeflow/util/filedialog.py

@@ -46,7 +46,13 @@ def _save(self, **kwargs) -> Optional[str]:
 
 
 class _SubprocessTkinter(_FileDialog):
-    ok = True
+    def __init__(self):
+        try:
+            from tkinter import Tk
+            import tkinter.filedialog
+            self.ok = True
+        except ModuleNotFoundError:
+            pass
 
     def _load(self, **kwargs) -> Optional[str]:
         return self._call('--load', self._to_args(kwargs))
@@ -74,21 +80,16 @@ def _to_args(self, kwargs: dict) -> list:
         return args
 
 
-
-def _has_zenity():
-    try:
-        return not sp.call(['zenity', '--version'], stdout=sp.DEVNULL)
-    except FileNotFoundError:
-        return False
-
-
 class _Zenity(_FileDialog):
     _map = {
         'title': '--title',
         'pattern': '--file-filter',
     }
     def __init__(self):
-        self.ok = _has_zenity()
+        try:
+            self.ok = not sp.call(['zenity', '--version'], stdout=sp.DEVNULL)
+        except FileNotFoundError:
+            pass
 
     def _load(self, **kwargs) -> Optional[str]:
         return self._call(self._compose(False, kwargs))
@@ -120,13 +121,14 @@ def _call(self, command: List[str]) -> Optional[str]:
 
 
 class _Windows(_FileDialog):
-    GetOpenFileNameW: Any
-    GetSaveFileNameW: Any
+    _open_w: Any
+    _save_w: Any
 
     def __init__(self):
-        from win32gui import GetOpenFileNameW, GetSaveFileNameW
-        self.GetOpenfileNameW = GetOpenFileNameW
-        self.GetSaveFileNameW = GetSaveFileNameW
+        import win32gui
+        self._open_w = win32gui.GetOpenFileNameW
+        self._save_w = win32gui.GetSaveFileNameW
+        self.ok = True
 
     def _resolve(self, method: str, kwargs: dict) -> dict:
         kwargs = super()._resolve(method, kwargs)
@@ -137,29 +139,31 @@ def _resolve(self, method: str, kwargs: dict) -> dict:
         return kwargs
 
     def _load(self, **kwargs) -> Optional[str]:
-        file, _, _ = self.GetOpenFileNameW(**kwargs)
+        file, _, _ = self._open_w(**kwargs)
         return file
 
     def _save(self, **kwargs) -> Optional[str]:
-        file, _, _ = self.GetSaveFileNameW(**kwargs)
+        file, _, _ = self._save_w(**kwargs)
         return file
 
 
 filedialog: _FileDialog
+"""Cross-platform file dialog.
+
+Tries to use `zenity <https://help.gnome.org/users/zenity/stable/>`_
+on Linux if available, because ``tkinter`` looks fugly in GNOME don't @ me.
+
+Uses win32gui dialogs on Windows.
+
+Falls back to ``tkinter`` to support any other platform.
+For MacOS this may mean that you have to install tkinter via
+``brew install tkinter-python`` for these dialogs to work.
+"""
 
 if os.name != "nt":
-    # try using zenity by default
     filedialog = _Zenity()
-    """Cross-platform file dialog.
-
-    Tries to use `zenity <https://help.gnome.org/users/zenity/stable/>`_
-    if available, because ``tkinter`` looks fugly in GNOME don't @ me.
-
-    Defaults to ``tkinter`` to support basically any platform.
-    """
-    # if zenity doesn't work (e.g. it's not installed),
-    # default to tkinter.
-    if not filedialog.ok:
-        filedialog = _SubprocessTkinter()
 else:
     filedialog = _Windows()
+
+if not filedialog.ok:
+    filedialog = _SubprocessTkinter()

shapeflow/util/tk-filedialog.py

@@ -22,9 +22,9 @@
     path = None
 
     if args.load or (not args.load and not args.save):
-        out = tkinter.filedialog.askopenfilename(**d)
+        path = tkinter.filedialog.askopenfilename(**d)
     elif args.save:
-        out = tkinter.filedialog.asksaveasfilename(**d)
+        path = tkinter.filedialog.asksaveasfilename(**d)
 
     if isinstance(path, str):
         print(path)  # can be read with `out, err = p.communicate()`

test/test_util.py

@@ -1,35 +1,51 @@
 import unittest
-from unittest.mock import patch
+from unittest.mock import patch, MagicMock
 
 import os
 import json
 import tkinter
 import tkinter.filedialog
 import subprocess
+import sys
+import platform
 
+if platform.system() == "Windows":
+    import win32gui
+    _win32gui = win32gui
 
-from shapeflow.util.filedialog import _SubprocessTkinter, _Zenity
+win32gui = MagicMock()
+win32gui.GetOpenFileNameW = MagicMock(return_value=(b'...', None, None))
+win32gui.GetSaveFileNameW = MagicMock(return_value=(b'...', None, None))
+sys.modules["win32gui"] = win32gui
+
+from shapeflow.util.filedialog import _SubprocessTkinter, _Zenity, _Windows
 from shapeflow.util.from_venv import _VenvCall, _WindowsVenvCall, from_venv
 
 
 class FileDialogTest(unittest.TestCase):
-    kw = [
-        'title',
-        'pattern',
-        'pattern_description',
-    ]
-    kwargs = {k:k for k in kw}
+    kwargs = {
+        'title': 'title',
+        'pattern': 'pattern1 pattern2 pattern3',
+        'pattern_description': 'pattern_description',
+    }
+
+    @classmethod
+    def tearDownClass(cls) -> None:
+        if platform.system() == "Windows":
+            sys.modules["win32gui"] = _win32gui
+
 
     @patch('subprocess.Popen')
     def test_tkinter_load(self, Popen):
         Popen.return_value.communicate.return_value = (b'...', 0)
         _SubprocessTkinter().load(**self.kwargs)
 
-        self.assertCountEqual(
+        self.assertSequenceEqual(
             [
                 'python', 'shapeflow/util/tk-filedialog.py', '--load',
-                '--title', 'title', '--pattern', 'pattern',
-                '--pattern_description', 'pattern_description'
+                '--pattern', 'pattern1 pattern2 pattern3',
+                '--pattern_description', 'pattern_description',
+                '--title', 'title',
             ],
             Popen.call_args[0][0]
         )
@@ -39,11 +55,12 @@ def test_tkinter_save(self, Popen):
         Popen.return_value.communicate.return_value = (b'...', 0)
         _SubprocessTkinter().save(**self.kwargs)
 
-        self.assertCountEqual(
+        self.assertSequenceEqual(
             [
                 'python', 'shapeflow/util/tk-filedialog.py', '--save',
-                '--title', 'title', '--pattern', 'pattern',
-                '--pattern_description', 'pattern_description'
+                '--pattern', 'pattern1 pattern2 pattern3',
+                '--pattern_description', 'pattern_description',
+                '--title', 'title',
             ],
             Popen.call_args[0][0]
         )
@@ -53,23 +70,27 @@ def test_tkinter_save(self, Popen):
     def test_zenity_load(self, Popen):
         Popen.return_value.communicate.return_value = (b'...', 0)
         _Zenity().load(**self.kwargs)
-
-        c = 'zenity --file-selection --title title --file-filter pattern'
-
-        self.assertCountEqual(
-            c.split(' '),
+        self.assertSequenceEqual(
+            [
+                'zenity', '--file-selection',
+                '--file-filter', 'pattern1 pattern2 pattern3',
+                '--title', 'title',
+            ],
             Popen.call_args[0][0]
         )
 
+
     @patch('subprocess.Popen')
     def test_zenity_save(self, Popen):
         Popen.return_value.communicate.return_value = (b'...', 0)
         _Zenity().save(**self.kwargs)
 
-        c = 'zenity --file-selection --save --title title --file-filter pattern'
-
-        self.assertCountEqual(
-            c.split(' '),
+        self.assertSequenceEqual(
+            [
+                'zenity', '--file-selection', '--save',
+                '--file-filter', 'pattern1 pattern2 pattern3',
+                '--title', 'title',
+            ],
             Popen.call_args[0][0]
         )
 
@@ -98,6 +119,28 @@ def test_zenity_save_cancel(self, Popen):
         with self.assertRaises(ValueError):
             _Zenity().save(**self.kwargs)
 
+    def test_windows_load(self):
+        _Windows().load(**self.kwargs)
+
+        self.assertDictEqual(
+            {
+              "Title": "title",
+              "Filter": "pattern_description\0pattern1;pattern2;pattern3\0"
+            },
+            win32gui.GetOpenFileNameW.call_args.kwargs
+        )
+
+    def test_windows_save(self):
+        _Windows().save(**self.kwargs)
+
+        self.assertDictEqual(
+            {
+              "Title": "title",
+              "Filter": "pattern_description\0pattern1;pattern2;pattern3\0"
+            },
+            win32gui.GetSaveFileNameW.call_args.kwargs
+        )
+
 
 ENV = '.venv-name'
 PYTHON = 'python4.2.0'