Re: [PATCH] Use perl instead of tac

From: Brian Gernhardt <benji@silverinsanity.com>
Date: Apr 28, 2008, 19:13 UTC
Message-ID: <04EB077D-CB35-4253-A9EA-2085890C3639@silverinsanity.com>
In-Reply-To: <slrng1bqhb.25r.joerg@alea.gnuu.de>

On Apr 28, 2008, at 11:15 AM, Jörg Sommer wrote:

Show 5 quoted lines

> Nothing. My intention was not the speed, but the dependency on Perl.  
> But
> your are right. Except from the point that my suggestion is broken,  
> it's
> *much* slower:

[snip]

> And what about something like this: 'tac || rev || perl …'

This would actually be spelled something like:

----- 8< ----- tac=""

tac < /dev/null > /dev/null 2>&1
if test $? != 127; then
    tac=tac
fi

if test -z "$tac"; then
    perl < /dev/null > /dev/null 2>&1
    if test $? != 127; then
       tac="perl -e 'print reverse <>'"
    fi
fi

if test -z "$tac"; then
    die "Couldn't find tac or perl."
fi

(subshell) | $tac | while loop ----- 8< -----

Ugly, no? Plus it adds a dependency on tac OR doesn't solve the perl dependency. I personally think using perl is better than this approach. Even if we're trying to reduce the perl dependency in core, it's there right now so this has zero impact now. The ideal solution is to re-write the loop so that it doesn't need to be reversed in the first place. We can use perl until that rewrite is done.

This loop-reverse-loop construct is _extremely_ ugly, and I'm having problems following it. None of what this function is doing is immediately obvious. It's a good candidate for a comment or two.

Looking over it, we should be able to do this with one loop over the list of commits, doing this for each of them:

if more than one parent
   other_parents=all but first, comma separated
   print merge command (without marks)
else
   print pick command
fi

if commit is tagged
   print tag command
fi

I also dislike the large lists this is carrying around in shell variables. If I'm reading it correctly, the tag list could be replaced by invocations of "git describe --exact-match". The mark list appears to be unavoidable, but significantly smaller than the tag list.

Now that I think about it, the generation of marks could be done by a second loop over the list. Notice what commits need to be marked in loop 1, then add the mark commands in loop 2. Both of these loops would function in the same direction, removing the need for either tac or perl.

~~ Brian