Jonathan Nieder wrote:
> Ramkumar Ramachandra wrote:
>
>> I wrote that too quickly. I can't stand seeing so many strcmp() calls
>> all over my codebase -- look at the number of instances of matching
>> opts->command to REPLAY_CMD_*.
>
> If the number of such in sequencer.c is greater than 0, it's probably
> a bug. Why would the sequencer change behavior based on its caller's
> name?
In the super-final version, yes- I agree. However, we're not there
yet -- this patch is more of a transition, to make the life of
"revert: allow mixing "pick" and "revert" actions" easier. Once the
painful move to sequencer.c is completed, we can think about all these
things. Right now, I'm only focusing on the move, and everything that
should logically precede it (in my opinion).
> Is this what you're talking about?
>
> if (opts->action == CHERRY_PICK)
> error(_("Your local changes would be overwritten by cherry-pick."));
> else
> error(_("Your local changes would be overwritten by revert."));
1. Yes, error messages like this.
2. Since I haven't modified function prototypes to include a
"replay_action" at the "revert: decouple sequencer actions from
builtin commands" stage, functions like do_pick_commit() have to rely
on opts->command! That's the entire point: I want to show how this
"opts->command" is changing to "action" in most places in "revert:
allow mixing "pick" and "revert" actions".
I'd rationalize that taking care of (1) is not urgent- we can do it
after we move to sequencer.c. As for (2), we have little choice at
this point- either make it a string like it's supposed to be in the
super-final version, and struggle with the ugliness of strcmp() now,
or make it an enum now; I choose the latter.
Thanks for making me explain this.
-- Ram
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html