pylint fixes and bug fixes for CLI/GUI split

svn: r12677

src/cli/arghandler.py

@@ -35,17 +35,13 @@ Module responsible for handling the command line arguments for GRAMPS.
 #-------------------------------------------------------------------------
 import os
 import sys
-import getopt
 from gettext import gettext as _
-import logging
 
 #-------------------------------------------------------------------------
 #
 # gramps modules
 #
 #-------------------------------------------------------------------------
-import const
-import Config
 import RecentFiles
 import Utils
 import gen
@@ -71,14 +67,6 @@ class ArgHandler(object):
         self.sm = sessionmanager
         self.errorfunc = errorfunc
         self.gui = gui
-        self.dbman = CLIDbManager(self.dbstate)
-        self.force_unlock = parser.force_unlock
-        self.open = self.__handle_open_option(parser.open)
-        self.cl = 0
-        self.imports = []
-        self.exports = []
-        self.sanitize_args(parser.imports, parser.exports)
-        self.open_gui = parser.open_gui
         if self.gui:
             self.actions = []
             self.list = False
@@ -88,9 +76,21 @@ class ArgHandler(object):
             self.actions = parser.actions
             self.list = parser.list
             self.list_more = parser.list_more
+        self.open_gui = parser.open_gui
         self.imp_db_path = None
+        self.dbman = CLIDbManager(self.dbstate)
+        self.force_unlock = parser.force_unlock
+        self.cl = 0
+        self.imports = []
+        self.exports = []
 
-    def error(self, string):
+        self.open = self.__handle_open_option(parser.open)
+        self.sanitize_args(parser.imports, parser.exports)
+    
+    def __error(self, string):
+        """
+        Output an error. Uses errorfunc if given, otherwise a simple print
+        """
         if self.errorfunc:
             self.errorfunc(string)
         else:
@@ -111,6 +111,7 @@ class ArgHandler(object):
     def __handle_open_option(self, value):
         """
         Handle the "-O" or "--open" option.
+        Only Family trees or a dir with a family tree can be opened.                  
         """
         if value is None:
             return None
@@ -123,7 +124,7 @@ class ArgHandler(object):
                 sys.exit(0)
             return db_path
         else:
-            self.error( _('Error: Input family tree "%s" does not exist.\n'
+            self.__error( _('Error: Input family tree "%s" does not exist.\n'
                     "If gedcom, gramps-xml or grdb, use the -i option to "
                     "import into a family tree instead.") % value)
             sys.exit(0)
@@ -131,11 +132,12 @@ class ArgHandler(object):
     def __handle_import_option(self, value, format):
         """
         Handle the "-i" or "--import" option.
+        Only Files supported by a plugin can be imported, so not Family Trees
         """
         fname = value
         fullpath = os.path.abspath(os.path.expanduser(fname))
         if not os.path.exists(fullpath):
-            self.error(_('Error: Import file %s not found.') % fname)
+            self.__error(_('Error: Import file %s not found.') % fname)
             sys.exit(0)
         
         if format is None:
@@ -153,7 +155,7 @@ class ArgHandler(object):
         if plugin_found:
             self.imports.append((fname, format))
         else:
-            self.error(_('Error: Unrecognized type: "%(format)s" for '
+            self.__error(_('Error: Unrecognized type: "%(format)s" for '
                     'import file: %(filename)s') \
                   % {'format' : format, 
                      'filename' : fname})
@@ -162,21 +164,21 @@ class ArgHandler(object):
     def __handle_export_option(self, value, format):
         """
         Handle the "-e" or "--export" option.  
-        Note: only the CLI version has export                          
+        Note: this can only happen in the CLI version                          
         """
         if self.gui:
             return
         fname = value
         fullpath = os.path.abspath(os.path.expanduser(fname))
         if os.path.exists(fullpath):
-            self.error(_("WARNING: Output file already exist!\n"
+            self.__error(_("WARNING: Output file already exist!\n"
                     "WARNING: It will be overwritten:\n   %(name)s") % \
                     {'name' : fullpath})
             answer = None
             while not answer:
                 answer = raw_input(_('OK to overwrite? (yes/no) '))
             if answer.upper() in ('Y', 'YES', _('YES')):
-                self.error( _("Will overwrite the existing file: %s") 
+                self.__error( _("Will overwrite the existing file: %s") 
                                 % fullpath)
             else:
                 sys.exit(0)
@@ -196,7 +198,7 @@ class ArgHandler(object):
         if plugin_found:
             self.exports.append((fullpath, format))
         else:
-            self.error(_("ERROR: Unrecognized format for export file %s") 
+            self.__error(_("ERROR: Unrecognized format for export file %s") 
                             % fname)
             sys.exit(0)
         
@@ -226,14 +228,16 @@ class ArgHandler(object):
     # Overall argument handler: 
     # sorts out the sequence and details of operations
     #-------------------------------------------------------------------------
-    def handle_args_gui(self, dbman):
+    def handle_args_gui(self):
         """
         method to handle the arguments that can be given for a GUI session.
-        Returns the filename of the family tree that should be openend
+        Returns the filename of the family tree that should be openend if 
+        user just passed a famtree or a filename
             1/no options: a family tree can be given, if so, this name is tested
                         and returned. If a filename, it is imported in a new db
                         and name of new db returned
-            2/an open option can have been given
+            2/an open and/or import option can have been given, if so, this 
+                is handled, and None is returned
             
         """
         if self.open_gui:
@@ -259,6 +263,8 @@ class ArgHandler(object):
                 except:
                     title = db_path
                 RecentFiles.recent_files(db_path, title)
+                self.open = db_path
+                self.__open_action()
             else:
                 sys.exit(0)
             return db_path
@@ -267,8 +273,9 @@ class ArgHandler(object):
         #  open argument, and perhaps some import arguments
         self.__open_action()
         self.__import_action()
+        return None
     
-    def handle_args_cli(self, climan):
+    def handle_args_cli(self):
         """
         Depending on the given arguments, import or open data, launch
         session, write files, and/or perform actions.
@@ -315,13 +322,21 @@ class ArgHandler(object):
         sys.exit(0)
 
     def __import_action(self):
+        """
+        Take action for all given to import files. Note: Family trees are not
+            supported.
+        If a fam tree is open, the import happens on top of it. If not open, 
+        a new family tree is created, and the import done. If this is CLI, 
+        the created tree is deleted at the end (as some action will have 
+        happened that is now finished), if this is GUI, it is opened.
+        """
         if self.imports:
             self.cl = bool(self.exports or self.actions or self.cl)
 
             if not self.open:
                 # Create empty dir for imported database(s)
                 if self.gui:
-                    self.imp_db_path, title = self.dbman._create_new_db_cli()
+                    self.imp_db_path, title = self.dbman.create_new_db_cli()
                 else:
                     self.imp_db_path = Utils.get_empty_tempdir("import_dbdir")
                 
@@ -341,6 +356,10 @@ class ArgHandler(object):
                 self.cl_import(imp[0], imp[1])
 
     def __open_action(self):
+        """
+        Take action on a Fam tree dir to open. It will be opened in the 
+        sessionmanager
+        """
         if self.open:
             # Family Tree to open was given. Open it 
             # Then go on and process the rest of the command line arguments.
@@ -356,15 +375,18 @@ class ArgHandler(object):
                 sys.exit(0)
 
     def check_db(self, dbpath, force_unlock = False):
+        """
+        Test a given family tree path if it can be opened.
+        """
         # Test if not locked or problematic
         if force_unlock:
             self.dbman.break_lock(dbpath)
         if self.dbman.is_locked(dbpath):
-            print _("Database is locked, cannot open it!")
+            self.__error((_("Database is locked, cannot open it!") + '\n' +
-            print _("  Info: %s") % find_locker_name(dbpath)
+                          _("  Info: %s")) % find_locker_name(dbpath))
             return False
         if self.dbman.needs_recovery(dbpath):
-            print _("Database needs recovery, cannot open it!")
+            self.__error( _("Database needs recovery, cannot open it!"))
             return False
         return True
 
@@ -449,8 +471,9 @@ class ArgHandler(object):
                             options_class(self.dbstate.db, name, category, 
                                           options_str_dict)
                         else:
-                            cl_report(self.dbstate.db, name, category, report_class, 
+                            cl_report(self.dbstate.db, name, category, 
-                                      options_class, options_str_dict)
+                                      report_class, options_class,
+                                      options_str_dict)
                         return
                 # name exists, but is not in the list of valid report names
                 msg = "Unknown report name."

src/cli/argparser.py

@@ -33,7 +33,6 @@ Module responsible for handling the command line arguments for GRAMPS.
 # Standard python modules
 #
 #-------------------------------------------------------------------------
-import os
 import sys
 import getopt
 from gettext import gettext as _
@@ -250,6 +249,9 @@ class ArgParser(object):
         return True
     
     def print_help(self):
+        """
+        If the user gives the --help or -h option, print the output to terminal
+        """
         if self.help:
             print _HELP
             sys.exit(0)

src/cli/clidbman.py

@@ -43,8 +43,6 @@ from gettext import gettext as _
 import logging
 LOG = logging.getLogger(".clidbman")
 
-from gtk import STOCK_DIALOG_ERROR, STOCK_OPEN
-
 #-------------------------------------------------------------------------
 #
 # gramps modules
@@ -74,6 +72,18 @@ class CLIDbManager(object):
     Database manager without GTK functionality, allows users to create and
     open databases
     """
+    ICON_NONE     = 0
+    ICON_RECOVERY = 1
+    ICON_LOCK     = 2
+    ICON_OPEN     = 3
+    
+    ICON_MAP = {
+                ICON_NONE : None,
+                ICON_RECOVERY : None,
+                ICON_LOCK : None,
+                ICON_OPEN : None,
+               }
+    
     def __init__(self, dbstate):
         self.dbstate = dbstate
         self.msg = None
@@ -166,7 +176,7 @@ class CLIDbManager(object):
                 name = file(path_name).readline().strip()
 
                 (tval, last) = time_val(dirpath)
-                (enable, stock_id) = icon_values(dirpath, self.active, 
+                (enable, stock_id) = self.icon_values(dirpath, self.active, 
                                                  self.dbstate.db.is_open())
 
                 if (stock_id == 'gramps-lock'):
@@ -206,7 +216,7 @@ class CLIDbManager(object):
         """
         print _('Import finished...')
 
-    def _create_new_db_cli(self, title=None):
+    def create_new_db_cli(self, title=None):
         """
         Create a new database.
         """
@@ -237,7 +247,7 @@ class CLIDbManager(object):
         """
         Create a new database, do extra stuff needed
         """
-        return self._create_new_db_cli(title)
+        return self.create_new_db_cli(title)
 
     def import_new_db(self, filename, callback):
         """
@@ -296,6 +306,20 @@ class CLIDbManager(object):
         if os.path.exists(os.path.join(dbpath, "lock")):
             os.unlink(os.path.join(dbpath, "lock"))
     
+    def icon_values(self, dirpath, active, is_open):
+        """
+        If the directory path is the active path, then return values
+        that indicate to use the icon, and which icon to use.
+        """
+        if os.path.isfile(os.path.join(dirpath,"need_recover")):
+            return (True, self.ICON_MAP[self.ICON_RECOVERY])
+        elif dirpath == active and is_open:
+            return (True, self.ICON_MAP[self.ICON_OPEN])
+        elif os.path.isfile(os.path.join(dirpath,"lock")):
+            return (True, self.ICON_MAP[self.ICON_LOCK])
+        else:
+            return (False, self.ICON_MAP[self.ICON_NONE])
+
 def make_dbdir(dbdir):
     """
     Create the default database directory, as defined by dbdir
@@ -347,20 +371,6 @@ def time_val(dirpath):
         last = _("Never")
     return (tval, last)
 
-def icon_values(dirpath, active, is_open):
-    """
-    If the directory path is the active path, then return values
-    that indicate to use the icon, and which icon to use.
-    """
-    if os.path.isfile(os.path.join(dirpath,"need_recover")):
-        return (True, STOCK_DIALOG_ERROR)
-    elif dirpath == active and is_open:
-        return (True, STOCK_OPEN)
-    elif os.path.isfile(os.path.join(dirpath,"lock")):
-        return (True, 'gramps-lock')
-    else:
-        return (False, "")
-
 def find_locker_name(dirpath):
     """
     Opens the lock file if it exists, reads the contexts which is "USERNAME"

src/cli/grampscli.py

@@ -21,6 +21,13 @@
 
 # $Id:gramps_main.py 9912 2008-01-22 09:17:46Z acraphae $
 
+"""
+Provides the startcli function, which the main program calls for CLI
+execution of GRAMPS.
+
+Provides also two small base classes: CLIDbLoader, CLIManager
+"""
+
 #-------------------------------------------------------------------------
 #
 # Python modules
@@ -30,6 +37,9 @@ from gettext import gettext as _
 import os
 import sys
 
+import logging
+
+LOG = logging.getLogger(".grampscli")
 #-------------------------------------------------------------------------
 #
 # GRAMPS  modules
@@ -40,7 +50,8 @@ import Config
 import const
 import Errors
 import DbState
-from gen.db import GrampsDBDir
+from gen.db import (GrampsDBDir, FileVersionDeclineToUpgrade)
+import gen.db.exceptions
 from gen.plug import PluginManager
 import GrampsCfg
 import RecentFiles
@@ -51,20 +62,34 @@ import RecentFiles
 #
 #-------------------------------------------------------------------------
 class CLIDbLoader(object):
+    """
+    Base class for Db loading action inside a dbstate. Only the minimum is 
+    present needed for CLI handling
+    """
     def __init__(self, dbstate):
         self.dbstate = dbstate
     
-    def _warn(title, warnmessage):
+    def _warn(self, title, warnmessage):
+        """
+        Issue a warning message. Inherit for GUI action
+        """
         print _('WARNING: %s') % warnmessage
     
-    def _errordialog(title, errormessage):
+    def _errordialog(self, title, errormessage):
         """
         Show the error. A title for the error and an errormessage
+        Inherit for GUI action
         """
         print _('ERROR: %s') % errormessage
         sys.exit(1)
     
     def _dberrordialog(self, msg):
+        """
+        Show a database error. 
+        @param: msg : an error message
+        @type: string
+        @note: Inherit for GUI action
+        """
         self._errordialog( '', _("Low level database corruption detected") 
             + '\n' +
             _("GRAMPS has detected a problem in the underlying "
@@ -73,9 +98,17 @@ class CLIDbLoader(object):
               'click on the Repair button') + '\n\n' + str(msg))
     
     def _begin_progress(self):
+        """
+        Convenience method to allow to show a progress bar if wanted on load
+        actions. Inherit if needed
+        """
         pass
     
     def _pulse_progress(self, value):
+        """
+        Convenience method to allow to show a progress bar if wantedon load
+        actions. Inherit if needed
+        """
         pass
 
     def read_file(self, filename):
@@ -114,7 +147,7 @@ class CLIDbLoader(object):
         try:
             self.dbstate.db.load(filename, self._pulse_progress, mode)
             self.dbstate.db.set_save_path(filename)
-        except gen.db.FileVersionDeclineToUpgrade:
+        except FileVersionDeclineToUpgrade:
             self.dbstate.no_database()
         except gen.db.exceptions.FileVersionError, msg:
             self.dbstate.no_database()
@@ -128,7 +161,7 @@ class CLIDbLoader(object):
             self._dberrordialog(msg)
         except Exception:
             self.dbstate.no_database()
-            _LOG.error("Failed to open database.", exc_info=True)
+            LOG.error("Failed to open database.", exc_info=True)
         return True
 
 #-------------------------------------------------------------------------
@@ -139,8 +172,10 @@ class CLIDbLoader(object):
 
 class CLIManager(object):
     """
-    A reduced viewmanager suitable for cli actions. 
+    Sessionmanager for GRAMPS. This is in effect a reduced viewmanager 
-    Aim is to manage a database on which to work
+    instance (see gui/viewmanager), suitable for CLI actions. 
+    Aim is to manage a dbstate on which to work (load, unload), and interact
+    with the plugin session
     """
     def __init__(self, dbstate, setloader):
         self.dbstate = dbstate
@@ -157,7 +192,7 @@ class CLIManager(object):
         """
         self._read_recent_file(path)
     
-    def _errordialog(title, errormessage):
+    def _errordialog(self, title, errormessage):
         """
         Show the error. A title for the error and an errormessage
         """
@@ -168,11 +203,11 @@ class CLIManager(object):
         """
         Called when a file needs to be loaded
         """
-        # A recent database should already have a directory 
+        # A recent database should already have a directory If not, do nothing,
-        # If not, do nothing, just return. This can be handled better if family tree
+        #  just return. This can be handled better if family tree delete/rename
-        # delete/rename also updated the recent file menu info in DisplayState.py
+        #  also updated the recent file menu info in DisplayState.py
         if not  os.path.isdir(filename):
-            self.errordialog(
+            self._errordialog(
                     _("Could not load a recent Family Tree."), 
                     _("Family Tree does not exist, as it has been deleted."))
             return
@@ -194,9 +229,9 @@ class CLIManager(object):
         The method called after load of a new database. 
         Here only CLI stuff is done, inherit this method to add extra stuff
         """
-        self._post_load_newdb_nongui(filename, filetype, title)
+        self._post_load_newdb_nongui(filename, title)
     
-    def _post_load_newdb_nongui(self, filename, filetype, title=None):
+    def _post_load_newdb_nongui(self, filename, title=None):
         """
         Called after a new database is loaded.
         """
@@ -231,7 +266,8 @@ class CLIManager(object):
         Config.set(Config.RECENT_FILE, filename)
 
         try:
-            self.dbstate.change_active_person(self.dbstate.db.find_initial_person())
+            self.dbstate.change_active_person(
+                                    self.dbstate.db.find_initial_person())
         except:
             pass
         
@@ -279,6 +315,6 @@ def startcli(errors, argparser):
     handler = ArgHandler(dbstate, argparser, climanager)
     # create a manager to manage the database
     
-    handler.handle_args_cli(climanager)
+    handler.handle_args_cli()
     
     sys.exit(0)

src/gui/dbloader.py

@@ -75,10 +75,10 @@ class DbLoader(CLIDbLoader):
         self.uistate = uistate
         self.import_info = None
     
-    def _warn(title, warnmessage):
+    def _warn(self, title, warnmessage):
         WarningDialog(title, warnmessage)
     
-    def _errordialog(title, errormessage):
+    def _errordialog(self, title, errormessage):
         """
         Show the error. 
         In the GUI, the error is shown, and a return happens

src/gui/dbman.py

@@ -107,6 +107,12 @@ class DbManager(CLIDbManager):
     Database Manager. Opens a database manager window that allows users to
     create, rename, delete and open databases.
     """
+    ICON_MAP = {
+                CLIDbManager.ICON_NONE : '',
+                CLIDbManager.ICON_RECOVERY : gtk.STOCK_DIALOG_ERROR,
+                CLIDbManager.ICON_LOCK : 'gramps-lock',
+                CLIDbManager.ICON_OPEN : gtk.STOCK_OPEN,
+               }
 
     def __init__(self, dbstate, parent=None):
         """
@@ -680,7 +686,7 @@ class DbManager(CLIDbManager):
         """
         Create a new database, append to model
         """
-        new_path, title = self._create_new_db_cli(title)
+        new_path, title = self.create_new_db_cli(title)
         path_name = os.path.join(new_path, NAME_FILE)
         (tval, last) = time_val(new_path)
         node = self.model.append(None, [title, new_path, path_name, 

src/gui/viewmanager.py

@@ -1093,7 +1093,7 @@ class ViewManager(CLIManager):
         The method called after load of a new database. 
         Inherit CLI method to add GUI part
         """
-        self._post_load_newdb_nongui(filename, filetype, title)
+        self._post_load_newdb_nongui(filename, title)
         self._post_load_newdb_gui(filename, filetype, title)
         
     def _post_load_newdb_gui(self, filename, filetype, title=None):