Default "extra" config variable omits all exclude/enable module settings

[ghost]

file: bot.py

if not hasattr(self.config, 'enable'):
    for fn in os.listdir(os.path.join(os.getcwd(), 'modules')):
        if fn.endswith('.py') and not fn.startswith('_'):
            filenames.append(os.path.join(home, 'modules', fn))
else:
    for fn in self.config.enable:
        filenames.append(os.path.join(home, 'modules', fn + '.py'))

if hasattr(self.config, 'extra'):
    for fn in self.config.extra:
        if os.path.isfile(fn):
            filenames.append(fn)
        elif os.path.isdir(fn):
            for n in os.listdir(fn):
                if n.endswith('.py') and not n.startswith('_'):
                    filenames.append(os.path.join(fn, n))

The problem is that, by default, extra includes the same module folder that the first for-loop crawls. And since there's no check if a module is on the exclude/enable list during the second for-loop it just adds them anyway.

[kaneda]

I think this requires a bit more investigation.

[ghost]

Does this look good? I was also thinking of making the latest filename of the same name overwrite the array netry instead of duplicating it, does that sound good?

    def setup(self):
        self.variables = {}

        filenames = []

        module_folders = [os.path.join(home, 'modules')]
        module_folders.extend(getattr(self.config, 'extra', []))
        modules = []

        excluded = getattr(self.config, 'exclude', [])
        enabled = getattr(self.config, 'enable', [])

        for folder in module_folders:
            if os.path.isfile(folder):
                filenames.append(folder)
            elif os.path.isdir(folder):
                for fn in os.listdir(folder):
                    if fn.endswith('.py') and not fn.startswith('_'):
                        name = os.path.basename(fn)[:-3]
                        if name in enabled or not enabled and name not in excluded:
                            filenames.append(os.path.join(folder, fn))

        for filename in filenames:
            name = os.path.basename(filename)[:-3]
            try: module = imp.load_source(name, filename)
            except Exception, e:
                print >> sys.stderr, "Error loading %s: %s (in bot.py)" % (name, e)
            else:
                if hasattr(module, 'setup'):
                    module.setup(self)
                self.register(vars(module))
                modules.append(name)

        if modules:
            print >> sys.stderr, 'Registered modules:', ', '.join(sorted(modules))
        else:
            print >> sys.stderr, "Warning: Couldn't find any modules"

        self.bind_commands()

PS: Sorry for just pasting code in here instead of making pull requests. I'll do that sometime tomorrow.

[kaneda]

@Bekey I think the else with the is a weird thing to do. I'll need to take a closer look once you open a PR.