Merge lp:~maikels/openlp/myfixes into lp:openlp

[Maikel Stuivenberg]

[Raoul Snyman (raoul-snyman)]

Looks good, just a few changes before we'll accept it:

1. Don't break strings up, otherwise we end up with lots of smaller strings to be translated; see below.
2. Use my wording, as below.
3. I haven't seen any usage of QMessageBox passing strings. Please can you update your code to look like mine below.
4. Place your comments immediately under your if statements. If your comment is only one line long, use # comments. """ is generally used for docstrings.
5. There's no need for a "Cancel" option on the confirmation dialog - either they overwrite the theme, or they don't save it at all.

    result = QtGui.QMessageBox.Yes
    if os.path.exists(theme_file):
        result = QtGui.QMessageBox.question(
            self,
            translate(u'ThemeManager',u'Theme Exists'),
            translate(u'ThemeManager',u'A theme with this name already exists, would you like to overwrite it?'),
            (QtGui.QMessageBox.Yes | QtGui.QMessageBox.No),
            QtGui.QMessageBox.No)
    if result == QtGui.QMessageBox.Yes:
        # Save the theme, overwriting the existing theme if necessary.
        outfile = open(theme_file, u'w')
        outfile.write(theme_xml)
        outfile.close()
        if image_from is not None and image_from != image_to:
            shutil.copyfile(image_from, image_to)
            self.generateAndSaveImage(self.path, name, theme_xml)
            self.loadThemes()
    else:
        # Don't close the dialog - allow the user to change the name of the theme or to cancel the theme dialog completely.
        return False

review: Needs Fixing

Unmerged revisions

=== modified file 'openlp/core/ui/amendthemeform.py'
--- openlp/core/ui/amendthemeform.py	2009-08-26 05:00:19 +0000
+++ openlp/core/ui/amendthemeform.py	2009-08-27 16:21:00 +0000
@@ -135,8 +135,8 @@
                 unicode(self.theme.display_horizontalAlign), unicode(self.theme.display_verticalAlign),
                 unicode(self.theme.display_wrapStyle))
         theme = new_theme.extract_xml()
-        self.thememanager.saveTheme(theme_name, theme, save_from, save_to)
-        return QtGui.QDialog.accept(self)
+        if self.thememanager.saveTheme(theme_name, theme, save_from, save_to) is not False:
+            return QtGui.QDialog.accept(self)
 
     def loadTheme(self, theme):
         log.debug(u'LoadTheme %s', theme)
 
=== modified file 'openlp/core/ui/thememanager.py'
--- openlp/core/ui/thememanager.py	2009-08-26 05:00:19 +0000
+++ openlp/core/ui/thememanager.py	2009-08-28 18:34:33 +0000
@@ -329,13 +329,29 @@
         if os.path.exists(theme_dir) == False:
             os.mkdir(os.path.join(self.path, name))
         theme_file = os.path.join(theme_dir, name + u'.xml')
-        outfile = open(theme_file, u'w')
-        outfile.write(theme_xml)
-        outfile.close()
-        if image_from is not None and image_from != image_to:
-            shutil.copyfile(image_from,  image_to)
-        self.generateAndSaveImage(self.path, name, theme_xml)
-        self.loadThemes()
+        log.debug(theme_file)
+ 
+        result = QtGui.QMessageBox.Yes
+        if os.path.exists(theme_file):
+            result = QtGui.QMessageBox.question(
+                self,
+                translate(u'ThemeManager',u'Theme Exists'),
+                translate(u'ThemeManager',u'A theme with this name already exists, would you like to overwrite it?'),
+                (QtGui.QMessageBox.Yes | QtGui.QMessageBox.No),
+                QtGui.QMessageBox.No)
+        if result == QtGui.QMessageBox.Yes:
+            # Save the theme, overwriting the existing theme if necessary.
+            outfile = open(theme_file, u'w')
+            outfile.write(theme_xml)
+            outfile.close()
+            if image_from is not None and image_from != image_to:
+                shutil.copyfile(image_from, image_to)
+            
+            self.generateAndSaveImage(self.path, name, theme_xml)
+            self.loadThemes()
+        else:
+            # Don't close the dialog - allow the user to change the name of the theme or to cancel the theme dialog completely.
+            return False
 
     def generateAndSaveImage(self, dir, name, theme_xml):
         log.debug(u'generateAndSaveImage %s %s %s', dir, name, theme_xml)