Re: [PATCH 06/34] sequencer (rebase -i): write the 'done' file

[Johannes Schindelin]

Hi Dennis,

On Wed, 31 Aug 2016, Dennis Kaarsemaker wrote:

> On wo, 2016-08-31 at 10:54 +0200, Johannes Schindelin wrote:
> > In the interactive rebase, commands that were successfully processed are
> > not simply discarded, but appended to the 'done' file instead. This is
> > used e.g. to display the current state to the user in the output of
> > `git status` or the progress.
> 
> Wouldn't it make more sense to have this patch before the ones that
> implement the actual rebase commands?

I waffled about the order so many times that I don't know anymore. The
thing is, while the sequencer is taught incrementally to understand all of
the rebase -i functionality, rebase -i itself is not touched, on purpose.

In the case of the "done" file, my thoughts were: the commands do not need
this file *at all*. In fact, if we did not write the "done" file at all,
the only two types of test failures in the test suite would be 1) git
status' output and 2) the prompt testing for the progress.

So you see, functionally, the "done" file is only relevant to the progress
part of the patch series.

As such, I'd rather keep this patch in the current place, just before
introducing the progress.

> Hmm, and after reading more of this series, I think the same applies to
> some other patches too, e.g. 08/34 and 14/34, so I'm probably missing
> something. So before I make a fool of myself and suggest that the
> implementation of the actual commands should come at the end, maybe you
> could tell me what I'm missing :) 

No, no, don't hesitate to suggest reorderings. I am really thankful for
the discussion we are having, so that the outcome is better than what I
have right now. If the outcome would be the very same patches, but with
more confidence, it would still be better than what I have right now ;-)

Ciao,
Dscho

Re: [PATCH 06/34] sequencer (rebase -i): write the 'done' file

[Dennis Kaarsemaker]

On wo, 2016-08-31 at 10:54 +0200, Johannes Schindelin wrote:
> In the interactive rebase, commands that were successfully processed are
> not simply discarded, but appended to the 'done' file instead. This is
> used e.g. to display the current state to the user in the output of
> `git status` or the progress.

Wouldn't it make more sense to have this patch before the ones that
implement the actual rebase commands?

Hmm, and after reading more of this series, I think the same applies to
some other patches too, e.g. 08/34 and 14/34, so I'm probably missing
something. So before I make a fool of myself and suggest that the
implementation of the actual commands should come at the end, maybe you
could tell me what I'm missing :) 

D.

[PATCH 06/34] sequencer (rebase -i): write the 'done' file

[Johannes Schindelin]

In the interactive rebase, commands that were successfully processed are
not simply discarded, but appended to the 'done' file instead. This is
used e.g. to display the current state to the user in the output of
`git status` or the progress.

Signed-off-by: Johannes Schindelin 
---
 sequencer.c | 20 
 1 file changed, 20 insertions(+)

diff --git a/sequencer.c b/sequencer.c
index 95c31bb..3bff3d9 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -39,6 +39,12 @@ static GIT_PATH_FUNC(rebase_path, "rebase-merge")
  */
 static GIT_PATH_FUNC(rebase_path_todo, "rebase-merge/git-rebase-todo")
 /*
+ * The rebase command lines that have already been processed. A line
+ * is moved here when it is first handled, before any associated user
+ * actions.
+ */
+static GIT_PATH_FUNC(rebase_path_done, "rebase-merge/done")
+/*
  * A script to set the GIT_AUTHOR_NAME, GIT_AUTHOR_EMAIL, and
  * GIT_AUTHOR_DATE that will be used for the commit that is currently
  * being rebased.
@@ -1272,6 +1278,20 @@ static int save_todo(struct todo_list *todo_list, struct 
replay_opts *opts)
todo_path, strerror(errno));
if (commit_lock_file(&todo_lock) < 0)
return error(_("Error wrapping up %s."), todo_path);
+
+   if (is_rebase_i(opts)) {
+   const char *done_path = rebase_path_done();
+   int fd = open(done_path, O_CREAT | O_WRONLY | O_APPEND, 0666);
+   int prev_offset = !next ? 0 :
+   todo_list->items[next - 1].offset_in_buf;
+
+   if (offset > prev_offset && write_in_full(fd,
+   todo_list->buf.buf + prev_offset,
+   offset - prev_offset) < 0)
+   return error(_("Could not write to %s (%s)"),
+   done_path, strerror(errno));
+   close(fd);
+   }
return 0;
 }
 
-- 
2.10.0.rc2.102.g5c102ec