Re: [PATCH] Move stripspace() and launch_editor() to strbuf.c

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 11, 2007, 21:52 UTC
Message-ID: <7vtznsbgcc.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <Pine.LNX.4.64.0711111728380.4362@racer.site>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> These functions are already declared in strbuf.h, so it is only
> logical to move their implementations to the corresponding file.
> Particularly, since strbuf.h is in LIB_H, but both functions
> were missing from libgit.a.

I think this makes sense for stripspace(), but I have trouble with launch_editor().

I do not object to have a function in strbuf API that takes a buffer, allows the end user to interactively edit its content and returns the updated content. The function was perfectly fine as a special purpose helper for git-commit and git-tag, but if you look at the current launch_editor(), it is not suitable as a generic strbuf library function:

 * "Launch" feels as if we are initiating an async operation and
   returning from the function without waiting for its
   completion, but this is not "launch" but "launch, wait and
   return the resulting string".  Probably this should be called
   edit_buffer() or something like that.

 * Instead of dying, it should return exit code and have the
   caller choose to die or take any alternative action.  The
   library function definitely should not say "if you are in an
   environment where we cannot let you interactively edit, use
   -F or -m option".