Re: [StGit PATCH 4/5] Simplify editor selection logic

From: Karl Hasselström <kha@treskal.com>
Date: Jan 30, 2008, 17:57 UTC
Message-ID: <20080130175757.GA30529@diana.vm.bytemark.co.uk>
In-Reply-To: <76718490801300655r3d1b5b41l2945b4f730faedb2@mail.gmail.com>

On 2008-01-30 09:55:18 -0500, Jay Soffian wrote:

> On Jan 30, 2008 2:28 AM, Karl Hasselström <kha@treskal.com> wrote:

Show 17 quoted lines

> > You could write it kind of like this:
> >
> >   def e(key): return os.environ.get(key, None)
> >   def c(key): return config.get(key)
> >   editor = filter(None, [e('GIT_EDITOR'), c('stgit.editor'), c('core.editor'),
> >                          e('VISUAL'), e('EDITOR'), 'vi'])[0]
>
> Too clever by half if you ask me. Why not just:
>
> editor = (os.environ.get('GIT_EDITOR') or
>           config.get('stgit.editor') or
>           config.get('core.editor') or
>           os.environ.get('VISUAL') or
>           os.environ.get('EDITOR') or
>           'vi')
>
> And be done with it?

Yes. It's more repetitive, but not much longer. With only five options and one default -- if there were more, my version would be nicer (IMHO).

Show 21 quoted lines

> > Of course, if we're going to have code like this in several places
> > (you already mentioned the pager), we could build a function like
> > this:
> >
> >   editor = get_config(['GIT_EDITOR', 'stgit.editor', 'core.editor',
> >                        'VISUAL', 'EDITOR'], default = 'vi')
> >
> > that would differentiate between env variables and conf keys by
> > looking for dots in the name or something.
> 
> def get_config(keys, default=None):
>     rv = default
>     for k in keys:
>         if '.' in k:
>             d = config
>         else:
>             d = os.environ
>         if k in d:
>             rv = d[k]
>             break
>     return rv

'config' isn't a dict, so you have to use config.get, and you can't use 'k in config'. But otherwise yes. So something like this maybe:

def get_config(keys, default = None):
    rv = None
    for k in keys:
        if '.' in k:
            rv = config.get(k)
        else:
            rv = os.environ.get(k, None)
        if rv != None:
            return rv
    return default

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle