From: Alec Warner <[email protected]>

We want a default for the config file. We can handle this in main() by simply
setting myconfig to the default. If the user over-rides it with -c, we will take
the user provided value.

Delete a bunch of duplicate code.
Add docstring for parse_config.
Add warning that parse_config modifies global state.
Add TODO's about bad habits, like randomly calling sys.exit() in your code.

Calling stuff 'myFOO' is really annoying, particularly when you have a function 
such
as this and you have variables like:

myconfig
myconf
conf_values

and you have to tell them apart ;)

-A
---
 catalyst | 50 +++++++++++++++++++++-----------------------------
 1 file changed, 21 insertions(+), 29 deletions(-)

diff --git a/catalyst b/catalyst
index edc0b24..47efb90 100755
--- a/catalyst
+++ b/catalyst
@@ -57,11 +57,18 @@ def version():
        print "Copyright 2008-2012 various authors"
        print "Distributed under the GNU General Public License version 2.1\n"
 
-def parse_config(myconfig):
-       # search a couple of different areas for the main config file
-       myconf={}
-       config_file=""
-
+def parse_config(config_path):
+       """Parse the catalyst config file and generate a targetmap.
+
+       TODO(antarus): DANGER: This file modifies global state (conf_values).
+
+       Args:
+               config_path - A path to a catalyst config file.
+       Returns:
+               None
+       """
+       
+       # TODO(antarus): Move this outside of this function.
        confdefaults = {
                "distdir": "/usr/portage/distfiles",
                "hash_function": "crc32",
@@ -75,33 +82,18 @@ def parse_config(myconfig):
                "storedir": "/var/tmp/catalyst",
                }
 
-       # first, try the one passed (presumably from the cmdline)
-       if myconfig:
-               if os.path.exists(myconfig):
-                       print "Using command line specified Catalyst 
configuration file, "+myconfig
-                       config_file=myconfig
-
-               else:
-                       print "!!! catalyst: Could not use specified 
configuration file "+\
-                               myconfig
-                       sys.exit(1)
-
-       # next, try the default location
-       elif os.path.exists("/etc/catalyst/catalyst.conf"):
-               print "Using default Catalyst configuration file, 
/etc/catalyst/catalyst.conf"
-               config_file="/etc/catalyst/catalyst.conf"
 
-       # can't find a config file (we are screwed), so bail out
-       else:
-               print "!!! catalyst: Could not find a suitable configuration 
file"
+       try:
+               print "Trying config file '%s'" % config_path
+               cfg = modules.catalyst.config.ConfigParser(config_path)
+       except EnvironmentError as e:
+               print "Could not read config file '%s', got error '%s'" % 
(config_path, e)
+               # TODO(antarus): it should be discouraged to call sys.exit() 
from functions.
+               # One should call sys.exit() from main()..this isn't main().
                sys.exit(1)
 
-       # now, try and parse the config file "config_file"
        try:
-#              execfile(config_file, myconf, myconf)
-               myconfig = modules.catalyst.config.ConfigParser(config_file)
-               myconf.update(myconfig.get_values())
-
+               myconf.update(cfg.get_values())
        except:
                print "!!! catalyst: Unable to parse configuration file, 
"+myconfig
                sys.exit(1)
@@ -258,7 +250,7 @@ if __name__ == "__main__":
        debug=False
        verbose=False
        fetch=False
-       myconfig=""
+       myconfig="/etc/catalyst/catalyst.conf"
        myspecfile=""
        mycmdline=[]
        myopts=[]
-- 
1.8.1.2


Reply via email to