Re: [GSoC][PATCH v5 04/20] rebase -i: rewrite the edit-todo functionality in C

Hi Phillip,

Le 08/08/2018 à 18:04, Phillip Wood a écrit :

quoted

quoted

quoted

+int edit_todo_list(unsigned flags)
+{
+    struct strbuf buf = STRBUF_INIT;
+    const char *todo_file = rebase_path_todo();
+    FILE *todo;
+
+    if (strbuf_read_file(&buf, todo_file, 0) < 0)
+        return error_errno(_("could not read '%s'."), todo_file);
+
+    strbuf_stripspace(&buf, 1);
+    todo = fopen_or_warn(todo_file, "w");

This truncates the existing file, if there are any errors writing the
new one then the user has lost the old one. write_message() in
sequencer.c avoids this problem by writing a new file and then renaming
it if the write is successful, maybe it is worth exporting it so it can
be used elsewhere.

quoted

+    if (!todo) {
+        strbuf_release(&buf);
+        return 1;
+    }
+
+    strbuf_write(&buf, todo);
+    fclose(todo);

There needs to be some error checking after the write and the close
(using write_message() would mean you only have to check for errors in
one place)

Right.  Should I find a new nawe for write_message()?

That might be a good idea, I'm not sure what it should be though, maybe
write_file()?, perhaps someone else might have a better suggestion.

write_file() already exists in wrapper.c.  I wondered, as this make use
of the lockfile API, perhaps it could be moved to lockfile.{c,h}, and
renamed to something like write_file_with_lock().

Cheers,
Alban