Re: [PATCH/RFC/GSoC 16/17] editor: implement git_sequence_editor() and launch_sequence_editor()

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Mar 16, 2016, 18:21 UTC
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:

Show 12 quoted lines

> 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