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

From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: Aug 14, 2011, 16:21 UTC
Message-ID: <CALkWK0kdd=+w4-jiBX8hh1_-mi9US+A4nng4qeoRRNmR5yDpDw@mail.gmail.com>
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