Re: [PATCH 3/7] revert: Make commit descriptions in insn sheet optional

From: Ramkumar Ramachandra <artagnon@gmail.com>
To: Jonathan Nieder <jrnieder@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>,
	Git List <git@vger.kernel.org>,
	Christian Couder <chriscool@tuxfamily.org>,
	Daniel Barkalow <barkalow@iabervon.org>,
	Jeff King <peff@peff.net>
Subject: Re: [PATCH 3/7] revert: Make commit descriptions in insn sheet optional
Date: Sun, 14 Aug 2011 21:51:21 +0530	[thread overview]
Message-ID: <CALkWK0kdd=+w4-jiBX8hh1_-mi9US+A4nng4qeoRRNmR5yDpDw@mail.gmail.com> (raw)
In-Reply-To: <20110814160923.GL18466@elie.gateway.2wire.net>

Hi Jonathan,

Jonathan Nieder writes:
> If I understand correctly, the current code makes it perfectly possible
> to use commands like
>
>        pick origin/master^^2~5

I was thinking exactly the same thing a few hours ago!  I was
wondering what to do about such a tree'ish specification, and I came
to the conclusion that we should discourage this atleast for normal
users.  Why?  Because the tree'ish has to be invariant over the entire
sequencer operation*: something like "pick HEAD~3" is clearly flawed
since the picking operation updates HEAD.

* Some of us might find some really clever uses for a tree'ish that
refers to different commits during the course of the sequencer
operation, and I'm alright with that.  It might just be something
that's too complicated for many users to grok, and we can protect them
by not advertising this tree'ish operand feature.

> when that is more convenient to type.  So I wonder whether 40 is actually
> a good limit.  Another possibility would be to do something like
>
>        static struct strbuf cmit = STRBUF_INIT;

I don't like this; the sequencer will just end up drinking a lot of
memory for no reason if the insn sheet is horribly corrupted.  I think
40 is a very reasonable limit, even for the most complex tree'ish I
can dream of.

Thanks.

-- Ram