Re: [PATCH 07/14] revert: Introduce struct to keep command-line options

From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: Jul 12, 2011, 06:14 UTC
Message-ID: <CALkWK0mnhsCZoz0PLBAEL_Z9MOSfGDYTWj6ToNqyutxvfkfNCA@mail.gmail.com>
In-Reply-To: <20110706120616.GQ15682@elie>

Hi Jonathan,

Sorry about the delayed replay -- I intended to reply to this earlier; I'm not sure why I didn't.

Jonathan Nieder writes:

Show 10 quoted lines

>> My justification: in later steps, we'd want to be able to mix
>> "pick" and "revert" instructions in the same instruction sheet.  This
>> will essentially require the parser to return a commit + a replay_opts
>> struct (which will contain the action information).
>
> Side note: is it intended to support insns like
>
>  pick A..B
>
> ?

I wouldn't point to that instruction specifically, but yes- I plan to support complex instructions in future.

Show 5 quoted lines

> Anyway, the above explanation about the intended use for "struct
> replay_opts" (it needs to be small, I guess?) would be a good thing
> to add to the commit message, too.  Basically, whatever information a
> person needs in order to understand the design is a useful thing to
> add.

I'm not yet sure about this. On a related note, I'd like to ask: will we encounter two commands like "am" and "revert" which have the same command-line option name for two different functionality? If not, I'd probably like to stick to the per-session opts for as long as possible (ie. until someone finds a good usecase + implementation for per-action command-line options).

Show 7 quoted lines

>> Yes, I'm definitely considering exposing parse_args in the future,
>> especially since I want to support command-line options in my
>> instruction sheet.
>
> Hm, I am not sure what to think about this direction (is the git
> sequencer actually just a fast git shell?  in that case, why is pick
> spelled "pick" instead of "cherry-pick"?).  Maybe it's a good thing.

This is a very important question. Yes, it's intentionally named "pick" and not "cherry-pick" because I don't want the Sequencer to merely be a fast git shell. That's part of the reason I don't want arbitrary command-line options on the insn sheet. I want the possibility of accommodating complex instructions in the insn sheet (single instructions that might even involve talking to more than one git command). For the same reason, I also felt that "action" is the most appropriate name for the first word in each line in the insn sheet. What I'd call an "instruction" is an action + the relevant option(s) picked up from the opts sheet.

-- Ram