Re: [PATCH 1/2] lib-rebase.sh: fake-editor.sh: Allow checking of commit header(s) in $GIT_EDITOR

[Nazri Ramliy <ayiehere@gmail.com>]

On Mon, Jul 12, 2010 at 2:08 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Nazri Ramliy <ayiehere@gmail.com> writes:
>
>> +test -z "$EXPECT_HEADER" ||
>> +     (
>> +             grep '^pick' < "$1" | cut -d' ' -f3- > commit_headers.$$ &&
>
> Sending output from grep to cut does not sound very cool; wouldn't a
> single "sed" invocation more appropriate?

If I were to use a single "sed" invocation here's how it's going to look like:

	sed -e "s/^pick [0-9a-f]\+//" < "$1" > commit_headers.$$

But doing so reminds me of this:

On Sat, Jul 10, 2010 at 6:30 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Nazri Ramliy <ayiehere@gmail.com> writes:
>
>> +                     sed -i "s/^\([a-z]\+\) [0-9a-f]\+ /\1 $REPLACE_COMMIT_ID /" \
>
> This is not portable. Escaping an ERE element with a backslash does not
> make it suitable for use in BRE that sed uses.

Aren't we back to square one? Or am I missing something?

>> +             diff "$EXPECT_HEADER" commit_headers.$$ > /dev/null
>
> Is "test_cmp" inappropriate here for some reason?

It seems appropriate, but this will require a ". ./test-lib.sh" from inside
lib-rebase.sh, which fails because at that point we are already in the trash
directory for the test, the solution is to do a ". $TEST_DIRECTORY/test-lib.sh"
instead but that gives more errors due to test-lib.sh assumes that it is always
being "sourced" when $CWD is $TEST_DIRECTORY, i think.

>
>> +     ) ||
>
> Do you need a subshell for this, or just a grouping {} sufficient?

Yes a grouping {} is sufficient.

Note: The only user of the feature that this patch provides is my patch to
t3404-rebase-interactive.sh
(1278896663-3922-2-git-send-email-ayiehere@gmail.com),
the rationale of which is discussed in 7vbpadfd4r.fsf@alter.siamese.dyndns.org.

I'll send a reply to that one in a bit.


nazri.