Re: [PATCH] Move stripspace() and launch_editor() to strbuf.c
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Nov 11, 2007, 22:29 UTC
- Message-ID
- <Pine.LNX.4.64.0711112227420.4362@racer.site>
- In-Reply-To
- <7vtznsbgcc.fsf@gitster.siamese.dyndns.org>
Hi,
On Sun, 11 Nov 2007, Junio C Hamano wrote:
Show 28 quoted lines
> 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".
All valid points. Will add to my TODO list and repost when done.
Maybe call_editor() instead of edit_buffer()? Since we technically read the file specified by "path" into the buffer "buffer", after having called the editor.
Ciao, Dscho