Merge lp:~phill-ridout/openlp/1209515_2.0 into lp:openlp/2.0

[Phill]

Fixes #1209515 by subclassing QFileDialog and attempting to urldecode any files not found

[Andreas Preikschat (googol-deactivatedaccount)]

No need to add a constructor.

Make the getOpenFileNames method static

review: Needs Fixing

[Andreas Preikschat (googol-deactivatedaccount)]

113 + 'Abbreviated font pointsize unit')ad

review: Needs Fixing

[Phill (phill-ridout)]

review: Needs Fixing

[Phill (phill-ridout)]

Andreas,
How do I make it static?

[Phill (phill-ridout)]

Its asking me to review, presumably because I set it to need fixing before!

review: Approve

[Raoul Snyman (raoul-snyman)]

47 +Provide a work around for a bug in QFileDialog (#1209515)

Can you just be specific and say this is for a Launchpad bug (I might think it is a PyQt4 bug).

63 + file_list = QtCore.QStringList()
73 + file_list.append(QtCore.QString(file))

Why? This is Python, use Python objects.

106 + self.FNFT = unicode(translate('OpenLP.Ui',
108 + self.FNF = unicode(translate('OpenLP.Ui',

What does FNFT and FNF mean? I would prefer self.FileNotFound and self.FileNotFoundMessage

109 + 'File %s not found.\nPlease try selecting it individually.'))

This is horrible. Is there truly no way we can fix this problem?

review: Needs Fixing

[Phill (phill-ridout)]

> 63      + file_list = QtCore.QStringList()
> 73      + file_list.append(QtCore.QString(file))
>
> Why? This is Python, use Python objects.
As I was reimplementing and creating a "slot in" method I wanted to keep the same return type. But if you'd rather I'd use python objects I can. Also should I change the method name from camel case?

Note in the trunk version I have used python objects as we use PyQt API v2 (which automatically casts to python objects)

>
> 106     + self.FNFT = unicode(translate('OpenLP.Ui',
> 108     + self.FNF = unicode(translate('OpenLP.Ui',
>
> What does FNFT and FNF mean? I would prefer self.FileNotFound and self.FileNotFoundMessage

Sure, I was just following convention of the other variables.

> 109     + 'File %s not found.\nPlease try selecting it individually.'))
>
> This is horrible. Is there truly no way we can fix this problem?

Agreed, I included this as a belt and braces approach. I have tested this code with around 2000 files and have not seen this dialogue. I just added it because o wasn't 100% sure we might not have other encoding issues.

The bug is in Qt, select a single file and its fine, but select multiple files and it seems to URL encode the names. I don't see any other way round this!

[Raoul Snyman (raoul-snyman)]

OK, this is fine. Just change the names of the strings please.

[Phill (phill-ridout)]

review: Approve

[Raoul Snyman (raoul-snyman)]

Line 140 should be in there. PEP8 says that there should be 2 blank lines between statements on the module level.

Oh, and even if it says you need to review yourself, please don't.

review: Needs Fixing

[lp:~phill-ridout/openlp/1209515_2.0]

2157. By Phill on 2013-08-17

Space reinstated as per PEP8

2158. By Phill on 2013-08-20

added doc strings

Unmerged revisions

=== modified file 'openlp/core/lib/__init__.py'
--- openlp/core/lib/__init__.py	2013-01-20 18:46:41 +0000
+++ openlp/core/lib/__init__.py	2013-08-17 08:52:16 +0000
@@ -384,6 +384,7 @@
 
 
 from eventreceiver import Receiver
+from filedialog import FileDialog
 from listwidgetwithdnd import ListWidgetWithDnD
 from formattingtags import FormattingTags
 from spelltextedit import SpellTextEdit

=== added file 'openlp/core/lib/filedialog.py'
--- openlp/core/lib/filedialog.py	1970-01-01 00:00:00 +0000
+++ openlp/core/lib/filedialog.py	2013-08-17 08:52:16 +0000
@@ -0,0 +1,55 @@
+# -*- coding: utf-8 -*-
+# vim: autoindent shiftwidth=4 expandtab textwidth=80 tabstop=4 softtabstop=4
+
+###############################################################################
+# OpenLP - Open Source Lyrics Projection                                      #
+# --------------------------------------------------------------------------- #
+# Copyright (c) 2008-2013 Raoul Snyman                                        #
+# Portions copyright (c) 2008-2013 Tim Bentley, Gerald Britton, Jonathan      #
+# Corwin, Samuel Findlay, Michael Gorven, Scott Guerrieri, Matthias Hub,      #
+# Meinert Jordan, Armin Köhler, Erik Lundin, Edwin Lunando, Brian T. Meyer.   #
+# Joshua Miller, Stevan Pettit, Andreas Preikschat, Mattias Põldaru,          #
+# Christian Richter, Philip Ridout, Simon Scudder, Jeffrey Smith,             #
+# Maikel Stuivenberg, Martin Thompson, Jon Tibble, Dave Warnock,              #
+# Frode Woldsund, Martin Zibricky                                             #
+# --------------------------------------------------------------------------- #
+# This program is free software; you can redistribute it and/or modify it     #
+# under the terms of the GNU General Public License as published by the Free  #
+# Software Foundation; version 2 of the License.                              #
+#                                                                             #
+# This program is distributed in the hope that it will be useful, but WITHOUT #
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or       #
+# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for    #
+# more details.                                                               #
+#                                                                             #
+# You should have received a copy of the GNU General Public License along     #
+# with this program; if not, write to the Free Software Foundation, Inc., 59  #
+# Temple Place, Suite 330, Boston, MA 02111-1307 USA                          #
+###############################################################################
+
+"""
+Provide a work around for a bug in QFileDialog <https://bugs.launchpad.net/openlp/+bug/1209515>
+"""
+import os
+import urllib
+
+from PyQt4 import QtCore, QtGui
+
+from openlp.core.lib.ui import UiStrings
+
+class FileDialog(QtGui.QFileDialog):
+    @staticmethod
+    def getOpenFileNames(parent, title, path, filters):
+        files = QtGui.QFileDialog.getOpenFileNames(parent, title, path, filters)
+        file_list = QtCore.QStringList()
+        for file in files:
+            file = unicode(file)
+            if not os.path.exists(file):
+                file = urllib.unquote(file)
+                if not os.path.exists(file):
+                    QtGui.QMessageBox.information(parent,
+                        UiStrings().FileNotFound,
+                        UiStrings().FileNotFoundMessage % file)
+                    continue
+            file_list.append(QtCore.QString(file))
+        return file_list
\ No newline at end of file

=== modified file 'openlp/core/lib/mediamanageritem.py'
--- openlp/core/lib/mediamanageritem.py	2013-01-05 11:31:02 +0000
+++ openlp/core/lib/mediamanageritem.py	2013-08-17 08:52:16 +0000
@@ -35,7 +35,7 @@
 
 from PyQt4 import QtCore, QtGui
 
-from openlp.core.lib import SettingsManager, OpenLPToolbar, ServiceItem, \
+from openlp.core.lib import FileDialog, SettingsManager, OpenLPToolbar, ServiceItem, \
     StringContent, build_icon, translate, Receiver, ListWidgetWithDnD
 from openlp.core.lib.searchedit import SearchEdit
 from openlp.core.lib.ui import UiStrings, create_widget_action, \
@@ -337,7 +337,7 @@
         """
         Add a file to the list widget to make it available for showing
         """
-        files = QtGui.QFileDialog.getOpenFileNames(
+        files = FileDialog.getOpenFileNames(
             self, self.onNewPrompt,
             SettingsManager.get_last_dir(self.settingsSection),
             self.onNewFileMasks)

=== modified file 'openlp/core/lib/ui.py'
--- openlp/core/lib/ui.py	2012-12-30 19:41:24 +0000
+++ openlp/core/lib/ui.py	2013-08-17 08:52:16 +0000
@@ -77,6 +77,10 @@
         self.Error = translate('OpenLP.Ui', 'Error')
         self.Export = translate('OpenLP.Ui', 'Export')
         self.File = translate('OpenLP.Ui', 'File')
+        self.FileNotFound = unicode(translate('OpenLP.Ui',
+            'File Not Found'))
+        self.FileNotFoundMessage = unicode(translate('OpenLP.Ui',
+            'File %s not found.\nPlease try selecting it individually.'))
         self.FontSizePtUnit = translate('OpenLP.Ui', 'pt',
             'Abbreviated font pointsize unit')
         self.Help = translate('OpenLP.Ui', 'Help')

=== modified file 'openlp/core/ui/thememanager.py'
--- openlp/core/ui/thememanager.py	2012-12-30 19:41:24 +0000
+++ openlp/core/ui/thememanager.py	2013-08-17 08:52:16 +0000
@@ -36,7 +36,7 @@
 from xml.etree.ElementTree import ElementTree, XML
 from PyQt4 import QtCore, QtGui
 
-from openlp.core.lib import OpenLPToolbar, get_text_file_string, build_icon, \
+from openlp.core.lib import FileDialog, OpenLPToolbar, get_text_file_string, build_icon, \
     Receiver, SettingsManager, translate, check_item_selected, \
     check_directory_exists, create_thumb, validate_thumb, ImageSource
 from openlp.core.lib.theme import ThemeXML, BackgroundType, VerticalType, \
@@ -420,7 +420,7 @@
         attempting to extract OpenLP themes from those files. This process
         will load both OpenLP version 1 and version 2 themes.
         """
-        files = QtGui.QFileDialog.getOpenFileNames(self,
+        files = FileDialog.getOpenFileNames(self,
             translate('OpenLP.ThemeManager', 'Select Theme Import File'),
             SettingsManager.get_last_dir(self.settingsSection),
             unicode(translate('OpenLP.ThemeManager',

=== modified file 'openlp/plugins/songs/forms/editsongform.py'
--- openlp/plugins/songs/forms/editsongform.py	2013-06-16 17:19:32 +0000
+++ openlp/plugins/songs/forms/editsongform.py	2013-08-17 08:52:16 +0000
@@ -34,7 +34,7 @@
 
 from PyQt4 import QtCore, QtGui
 
-from openlp.core.lib import PluginStatus, Receiver, MediaType, translate, \
+from openlp.core.lib import FileDialog, PluginStatus, Receiver, MediaType, translate, \
     create_separated_list, check_directory_exists
 from openlp.core.lib.ui import UiStrings, set_case_insensitive_completer, \
     critical_error_message_box, find_and_set_in_combo_box
@@ -757,7 +757,7 @@
         Loads file(s) from the filesystem.
         """
         filters = u'%s (*)' % UiStrings().AllFiles
-        filenames = QtGui.QFileDialog.getOpenFileNames(self,
+        filenames = FileDialog.getOpenFileNames(self,
             translate('SongsPlugin.EditSongForm', 'Open File(s)'),
             QtCore.QString(), filters)
         for filename in filenames:

=== modified file 'openlp/plugins/songs/forms/songimportform.py'
--- openlp/plugins/songs/forms/songimportform.py	2012-12-30 19:41:24 +0000
+++ openlp/plugins/songs/forms/songimportform.py	2013-08-17 08:52:16 +0000
@@ -35,7 +35,7 @@
 
 from PyQt4 import QtCore, QtGui
 
-from openlp.core.lib import Receiver, SettingsManager, translate
+from openlp.core.lib import FileDialog, Receiver, SettingsManager, translate
 from openlp.core.lib.ui import UiStrings, critical_error_message_box
 from openlp.core.lib.settings import Settings
 from openlp.core.ui.wizard import OpenLPWizard, WizardStrings
@@ -281,7 +281,7 @@
         if filters:
             filters += u';;'
         filters += u'%s (*)' % UiStrings().AllFiles
-        filenames = QtGui.QFileDialog.getOpenFileNames(self, title,
+        filenames = FileDialog.getOpenFileNames(self, title,
             SettingsManager.get_last_dir(self.plugin.settingsSection, 1),
             filters)
         if filenames: