Re: [PATCH 1/3] t7504: document regression: reword no longer calls commit-msg

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Mar 22, 2017, 16:09 UTC
Message-ID: <alpine.DEB.2.20.1703221704500.3767@virtualbox>
In-Reply-To: <CAHGBnuPL9CUincZkFR758KcZL3-Ra=n_fbsc1EQ=vio2scod5w@mail.gmail.com>

Hi Sebastian,

On Wed, 22 Mar 2017, Sebastian Schuberth wrote:

Show 13 quoted lines

> On Wed, Mar 22, 2017 at 4:01 PM, Johannes Schindelin
> <johannes.schindelin@gmx.de> wrote:
> 
> > +# set up fake editor to replace `pick` by `reword`
> > +cat > reword-editor <<'EOF'
> > +#!/bin/sh
> > +mv "$1" "$1".bup &&
> > +sed 's/^pick/reword/' <"$1".bup >"$1"
> > +EOF
> 
> Maybe use
> 
> sed -i 's/^pick/reword/' "$1"

It's not portable, otherwise I would have used it. GNU sed expects an optional extension to be used for a backup file, and that optional extension needs to be specified without whitespace between it and the -i option. BSD sed *requires* an extension to be specified, and it has to be separated using white space (or is that orange space now?).

So even if we could make that call portable somehow (and remember, BSD sed is what OSX uses), there *would be a backup file*.

> here to avoid renaming the input file? Not sure how portable -i for
> sed is, though. Otherwise, maybe remove the file "$1".bup afterwards
> to be clean?

No. The file in question is written to the .git/rebase-merge/ subdirectory, therefore cleaned up after the rebase.

In any case, this is *test* code. So I'd prefer to have the changes to the C code scrutinized a bit more, not the test code as long as it is obvious what it does.

Ciao, Johannes