From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Wed, 16 Mar 2016 18:21:23 GMT
Subject: Re: [PATCH/RFC/GSoC 16/17] editor: implement git_sequence_editor() and launch_sequence_editor()
Message-ID: <alpine.DEB.2.20.1603161918410.4690@virtualbox>
In-Reply-To: <CACRoPnRMOp38vfkQZjmkUqr+urN8NYcNN_oNzHtJqfyTorr1ug@mail.gmail.com>

Hi Paul,

On Wed, 16 Mar 2016, Paul Tan wrote:

> Hi Dscho,
> 
> On Tue, Mar 15, 2016 at 3:00 PM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> > On Sat, 12 Mar 2016, Paul Tan wrote:
> >> ---
> >>  cache.h  |  1 +
> >
> > No need to clutter cache.h with a function that is only to be used by the
> > sequencer. IOW let's make this static in sequencer.c.
> 
> The function needs to be implemented in editor.c

No, *another* function needs to be implemented in editor.c: one that
accepts the editor itself as parameter. You did that, but then you wrapped
it as git_sequencer_editor() and left the *really* useful function *still*
static to editor.c.

Or maybe the best solution would be to simply extend git_editor() to
accept the editor as an additional, first parameter, falling back to the
current behavior if NULL is passed (and then change all callers to pass
NULL).

I guess my preference would be with the latter, that would make for the
most elegant, minimally invasive and most reusable solution.

Ciao,
Dscho