Hi,
Jonathan Nieder writes:
> Ramkumar Ramachandra wrote:
>> Change the way the instruction parser works, allowing arbitrary
>> (action, operand) pairs to be parsed.
>
> Parse the instruction list in .git/sequencer/todo as a list
> of (action, operand) pairs, instead of assuming all instructions
> use the same action.
Fixed. Thanks :)
>> @@ -517,7 +517,8 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)
>> /* TRANSLATORS: The first %s will be "revert" or
>> "cherry-pick", the second %s a SHA1 */
>> return error(_("%s: cannot parse parent commit %s"),
>> - action_name(opts), sha1_to_hex(parent->object.sha1));
>> + action == REPLAY_REVERT ? "revert" : "cherry-pick",
>> + sha1_to_hex(parent->object.sha1));
>
> My first thought was "why stop using the helper function action_name"?
> But now I see that it previously came from "opts" (i.e., the command
> line) and now comes from the todo file.
>
> The command name there was never really important except when
> cherry-pick or revert is being called by a script, and the message
> indicates which command was having trouble parsing the commit. If I
> am using "git cherry-pick --continue" to continue after a failed
> revert, I suspect action_name(opts) ["cherry-pick: "] would actually be
> more sensible than the command name corresponding to the particular
> pick/revert line.
You're right. Removed this hunk.
> Maybe something like
>
> len = strchrnul(p, '\n') - p;
> if (len > 255)
> len = 255;
> return error(_("Unrecognized action: %.*s"), (int) len, p);
>
> would do.
Excellent idea! I fixed the buffer overflow message to do this too.
By the way, shouldn't error() do this?
> Could we can make this error message more clearly suggest that it's
> giving context to the error above it? For example, something vaguely
> like
>
> error: unrecognized action: reset c78a78c9 Going back
> error: on line 7
Good suggestion. Fixed.
> Does a "cherry-pick --continue" in this scenario skip the first commit
> in the todo list? Should it?
It shouldn't and it doesn't. See how read_populate_opts and
read_populate_todo are called before the segment that drops the first
commit.
-- 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