Skip to content

Commit

Permalink
OWSave: Use the same dialog for Windows and macOS
Browse files Browse the repository at this point in the history
  • Loading branch information
@janezd
janezd committed Mar 1, 2019
1 parent 02d49df commit f482387
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 67 deletions.
37 changes: 15 additions & 22 deletions Orange/widgets/data/owsave.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -215,31 +215,33 @@ def _default_valid_filter(self):

# As of Qt 5.9, QFileDialog.setDefaultSuffix does not support double
# suffixes, not even in non-native dialogs. We handle each OS separately.
if sys.platform == "darwin":
# On macOS, is double suffixes are passed to the dialog, they are
# appended multiple times even if already present (QTBUG-44227).
# The only known workaround with native dialog is to use suffix *.*.
# We add the correct suffix after closing the dialog and only then check
if sys.platform in ("darwin", "win32"):
# macOS and Windows native dialogs do not correctly handle double
# extensions. We thus don't pass any suffixes to the dialog and add
# the correct suffix after closing the dialog and only then check
# if the file exists and ask whether to override.
# It is a bit confusing that the user does not see the final name in the
# dialog, but I see no better solution.
def get_save_filename(self): # pragma: no cover
def no_suffix(filt):
return filt.split("(")[0] + "(*.*)"
if sys.platform == "darwin":
def remove_star(filt):
return filt.replace(" (*.", " (.")
else:
def remove_star(filt):
return filt

mac_filters = {no_suffix(f): f for f in self._valid_filters()}
no_ext_filters = {remove_star(f): f for f in self._valid_filters()}
filename = self._initial_start_dir()
while True:
dlg = QFileDialog(
None, "Save File", filename, ";;".join(mac_filters))
None, "Save File", filename, ";;".join(no_ext_filters))
dlg.setAcceptMode(dlg.AcceptSave)
dlg.selectNameFilter(no_suffix(self._default_valid_filter()))
dlg.setOption(QFileDialog.HideNameFilterDetails)
dlg.selectNameFilter(remove_star(self._default_valid_filter()))
dlg.setOption(QFileDialog.DontConfirmOverwrite)
if dlg.exec() == QFileDialog.Rejected:
return "", ""
filename = dlg.selectedFiles()[0]
selected_filter = mac_filters[dlg.selectedNameFilter()]
selected_filter = no_ext_filters[dlg.selectedNameFilter()]
filename = self._replace_extension(
filename, self._extension_from_filter(selected_filter))
if not os.path.exists(filename) or QMessageBox.question(
Expand All @@ -248,15 +250,6 @@ def no_suffix(filt):
"Overwrite?") == QMessageBox.Yes:
return filename, selected_filter

elif sys.platform == "win32":
# TODO: Behaviour of getSaveFileName on Windows is not tested!!!
# Windows native dialog may work correctly; if not, we may do the same
# as for macOS?
def get_save_filename(self): # pragma: no cover
return QFileDialog.getSaveFileName(
self, "Save File", self._initial_start_dir(),
";;".join(self._valid_filters()), self._default_valid_filter())

else: # Linux and any unknown platforms
# Qt does not use a native dialog on Linux, so we can connect to
# filterSelected and to overload selectFile to change the extension
Expand All @@ -279,7 +272,7 @@ def updateDefaultExtension(self, selected_filter):
self.suffix = OWSave._extension_from_filter(selected_filter)
files = self.selectedFiles()
if files and not os.path.isdir(files[0]):
self.selectFile(files[0].split(".")[0])
self.selectFile(files[0])

def selectFile(self, filename):
filename = OWSave._replace_extension(filename, self.suffix)
Expand Down
85 changes: 40 additions & 45 deletions Orange/widgets/data/tests/test_owsave.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
from unittest.mock import patch, Mock
import os
import sys
import re

import scipy.sparse as sp
from AnyQt.QtWidgets import QFileDialog
Expand Down Expand Up @@ -410,27 +409,26 @@ def test_get_save_filename_linux(self):

def test_save_file_dialog_enforces_extension_linux(self):
dialog = OWSave.SaveFileDialog(
None, "Save File", "high.txt",
"Bar files (*.bar);;Low files (*.low)")
None, "Save File", "foo.bar",
"Bar files (*.tab);;Low files (*.csv)")

dialog.selectNameFilter("Low files (*.low)")
self.assertTrue(dialog.selectedFiles()[0].endswith("/high.low"))
dialog.selectNameFilter("Low files (*.csv)")
self.assertTrue(dialog.selectedFiles()[0].endswith("/foo.csv"))

dialog.selectFile("high.txt")
print(dialog.selectedFiles()[0])
self.assertTrue(dialog.selectedFiles()[0].endswith("/high.txt.low"))
dialog.selectFile("high.bar")
self.assertTrue(dialog.selectedFiles()[0].endswith("/high.bar.csv"))

dialog.selectNameFilter("Bar files (*.bar)")
self.assertTrue(dialog.selectedFiles()[0].endswith("/high.bar"))
dialog.selectNameFilter("Bar files (*.tab)")
self.assertTrue(dialog.selectedFiles()[0].endswith("/high.bar.tab"))

dialog.selectFile("middle.pkl")
self.assertTrue(dialog.selectedFiles()[0].endswith("/middle.bar"))
self.assertTrue(dialog.selectedFiles()[0].endswith("/middle.tab"))

dialog.filterSelected.emit("Low files (*.low)")
self.assertTrue(dialog.selectedFiles()[0].endswith("/middle.low"))
dialog.filterSelected.emit("Low files (*.csv)")
self.assertTrue(dialog.selectedFiles()[0].endswith("/middle.csv"))

dialog.selectFile("high.tab.gz")
self.assertTrue(dialog.selectedFiles()[0].endswith("/high.low"))
self.assertTrue(dialog.selectedFiles()[0].endswith("/high.csv"))

def test_save_file_dialog_uses_valid_filters_linux(self):
widget = self.widget
Expand All @@ -444,26 +442,35 @@ def test_save_file_dialog_uses_valid_filters_linux(self):
instance.selectNameFilter.assert_called_with("a (*.a)")


@unittest.skipUnless(sys.platform == "darwin", "Test for dialog for macOS")
@unittest.skipUnless(sys.platform in ("darwin", "win32"),
"Test for native dialog on Windows and macOS")
class TestOWSaveDarwinDialog(TestOWSaveBase): # pragma: no cover
@staticmethod
def _mac_filter(filt):
return re.sub(r"\(\*\..*\)", "(*.*)", filt)
if sys.platform == "darwin":
@staticmethod
def remove_star(filt):
return filt.replace(" (*.", " (.")
else:
@staticmethod
def remove_star(filt):
return filt

@patch("Orange.widgets.data.owsave.QFileDialog")
def test_get_save_filename_darwin(self, dlg):
widget = self.widget
widget._initial_start_dir = lambda: "baz"
widget.filters = dict.fromkeys(("a (*.a)", "b (*.b)", "c (*.c)"))
widget.filter = "b (*.b)"
widget.filters = dict.fromkeys(("aa (*.a)", "bb (*.b)", "cc (*.c)"))
widget.filter = "bb (*.b)"
instance = dlg.return_value
instance.exec.return_value = dlg.Accepted = QFileDialog.Accepted
instance.selectedFiles.return_value = ["foo"]
instance.selectedNameFilter.return_value = "a (*.*)"
self.assertEqual(widget.get_save_filename(), ("foo.a", "a (*.a)"))
instance.selectedNameFilter.return_value = self.remove_star("aa (*.a)")
self.assertEqual(widget.get_save_filename(), ("foo.a", "aa (*.a)"))
self.assertEqual(dlg.call_args[0][2], "baz")
self.assertEqual(dlg.call_args[0][3], "a (*.*);;b (*.*);;c (*.*)")
instance.selectNameFilter.assert_called_with("b (*.*)")
self.assertEqual(
dlg.call_args[0][3],
self.remove_star("aa (*.a);;bb (*.b);;cc (*.c)"))
instance.selectNameFilter.assert_called_with(
self.remove_star("bb (*.b)"))

instance.exec.return_value = dlg.Rejected = QFileDialog.Rejected
self.assertEqual(widget.get_save_filename(), ("", ""))
Expand All @@ -482,7 +489,7 @@ def test_save_file_dialog_enforces_extension_darwin(self, dlg):
instance = dlg.return_value
instance.exec.return_value = QFileDialog.Accepted

instance.selectedNameFilter.return_value = self._mac_filter(filter1)
instance.selectedNameFilter.return_value = self.remove_star(filter1)
instance.selectedFiles.return_value = ["foo"]
self.assertEqual(widget.get_save_filename()[0], "foo.tab")
instance.selectedFiles.return_value = ["foo.pkl"]
Expand All @@ -494,7 +501,7 @@ def test_save_file_dialog_enforces_extension_darwin(self, dlg):
instance.selectedFiles.return_value = ["foo.bar"]
self.assertEqual(widget.get_save_filename()[0], "foo.bar.tab")

instance.selectedNameFilter.return_value = self._mac_filter(filter2)
instance.selectedNameFilter.return_value = self.remove_star(filter2)
instance.selectedFiles.return_value = ["foo"]
self.assertEqual(widget.get_save_filename()[0], "foo.csv.gz")
instance.selectedFiles.return_value = ["foo.pkl"]
Expand Down Expand Up @@ -525,7 +532,7 @@ def selected_files():
instance = dlg.return_value
instance.exec.return_value = QFileDialog.Accepted
instance.selectedFiles = selected_files
instance.selectedNameFilter.return_value = self._mac_filter(filter1)
instance.selectedNameFilter.return_value = self.remove_star(filter1)

attempts = -1
msgbox.question.return_value = msgbox.Yes = 1
Expand All @@ -538,26 +545,15 @@ def selected_files():
@patch("Orange.widgets.data.owsave.QFileDialog")
def test_save_file_dialog_uses_valid_filters_darwin(self, dlg):
widget = self.widget
widget._valid_filters = lambda: ["a (*.a)", "b (*.b)"]
widget._default_valid_filter = lambda: "a (*.a)"
widget._valid_filters = lambda: ["aa (*.a)", "bb (*.b)"]
widget._default_valid_filter = lambda: "aa (*.a)"
instance = dlg.return_value
instance.exec.return_value = dlg.Rejected = QFileDialog.Rejected
widget.get_save_filename()
self.assertEqual(dlg.call_args[0][3], "a (*.*);;b (*.*)")
instance.selectNameFilter.assert_called_with("a (*.*)")


@unittest.skipUnless(sys.platform == "win32", "Test for dialog for Windows")
class TestOWSaveWindowsDialog(TestOWSaveBase): # pragma: no cover
@patch("Orange.widgets.data.owsave.QFileDialog.getSaveFileName")
def test_save_file_dialog_uses_valid_filters_windows(self, dlg):
widget = self.widget
widget._valid_filters = lambda: ["a (*.a)", "b (*.b)"]
widget._default_valid_filter = lambda: "a (*.a)"
widget.get_save_filename()
call_args = dlg.call_args
self.assertEqual(call_args[0][3], "a (*.a);;b (*.b)")
self.assertEqual(call_args[0][4], "a (*.a)")
self.assertEqual(
dlg.call_args[0][3], self.remove_star("aa (*.a);;bb (*.b)"))
instance.selectNameFilter.assert_called_with(
self.remove_star("aa (*.a)"))


class TestOWSaveUtils(unittest.TestCase):
Expand Down Expand Up @@ -592,6 +588,5 @@ def test_extension_from_filter(self):
OWSave._extension_from_filter("Description (.foo.bar)"), ".foo.bar")



if __name__ == "__main__":
unittest.main()

0 comments on commit f482387

Please sign in to comment.