Re: [PATCH 13/22] sequencer: remember the onelines when parsing the todo file

W dniu 31.08.2016 o 21:10, Junio C Hamano pisze:

Jakub Narębski [off-list ref] writes:

quoted

quoted

diff --git a/sequencer.c b/sequencer.c
index 06759d4..3398774 100644
--- a/sequencer.c
+++ b/sequencer.c

@@ -709,6 +709,8 @@ static int read_and_refresh_cache(struct replay_opts *opts)
 struct todo_item {
 	enum todo_command command;
 	struct commit *commit;
+	const char *arg;
+	int arg_len;

I am not sure what the "commit" field of type "struct commit *" is
for.  It is not needed until it is the commit's turn to be picked or
reverted; if we end up stopping in the middle, parsing the commit
object for later steps will end up being wasted effort.

From what I understand this was what sequencer did before this
series, so it is not a regression (I think; the commit parsing
was in different function, but I think at the same place in
the callchain).

Also, when the sequencer becomes one sequencer to rule them all, the
command set may contain something that does not even mention any
commit at all (think: exec).

The "exec" line is a bit of exception, all other rebase -i commands
take commit as parameter.  It could always use NULL.

So I am not sure if we want a parsed commit there (I would not
object if we kept the texual object name read from the file,
though).  The "one sequencer to rule them all" may even have to say
"now give name ':1' to the result of the previous operation" in one
step and in another later step have an instruction "merge ':1'".
When that happens, you cannot even pre-populate the commit object
when the sequencer reads the file, as the commit has not yet been
created at that point.

True, --preserve-merges rebase is well, different.

Best,
-- 
Jakub Narebski