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

From: Karl Hasselström <kha@treskal.com>
Date: Jan 30, 2008, 07:28 UTC
Message-ID: <20080130072828.GA24648@diana.vm.bytemark.co.uk>
In-Reply-To: <200801292109.37785.kumbayo84@arcor.de>

On 2008-01-29 21:09:37 +0100, Peter Oberndorfer wrote:

Show 6 quoted lines

> Since i personally dislike having a separate config for the editor
> in git/stgit i locally use this patch. unfortunately it makes the
> whole editor searching thing more complex :-( But i am sure it is
> possible to rewrite the code to something easier with some more
> python knowledge :-/ So it is not meant for direct applying, just
> for discussion...

I like the intention. What I'd like to do is to let the code _use_ the stgit.editor variable, but not advertise it in the docs except for noting that "this is deprecated and will go away in the future, but for now it's still effective".

Show 5 quoted lines

> +. the 'GIT_EDITOR' environment variable
>  . the 'stgit.editor' GIT configuration variable
> +. the 'core.editor' GIT configuration variable
> +. the 'VISUAL' environment variable
>  . the 'EDITOR' environment variable

That's a very nice order. (For completeness, the list should end with "vi", though. And I'd like that deprecation note for stgit.editor, unless someone has strong objections.)

Show 16 quoted lines

>      # the editor
> -    editor = config.get('stgit.editor')
> +    editor = None
> +    if 'GIT_EDITOR' in os.environ:
> +        editor = os.environ['GIT_EDITOR']
> +    if not editor:
> +        editor = config.get('stgit.editor')
> +    if not editor:
> +        editor = config.get('core.editor')
>      if editor:
>          pass
> +    elif 'VISUAL' in os.environ:
> +        editor = os.environ['VISUAL']
>      elif 'EDITOR' in os.environ:
>          editor = os.environ['EDITOR']
>      else:

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]

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.

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