From: Nazri Ramliy Date: Tue, 13 Jul 2010 00:36:59 GMT Subject: Re: [PATCH 1/2] lib-rebase.sh: fake-editor.sh: Allow checking of commit header(s) in $GIT_EDITOR Message-ID: In-Reply-To: <7v7hl1fd0n.fsf@alter.siamese.dyndns.org> On Mon, Jul 12, 2010 at 2:08 PM, Junio C Hamano wrote: > Nazri Ramliy 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 wrote: > Nazri Ramliy 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.