Johannes Schindelin <johannes.schinde...@gmx.de> writes:
> +static void flush_rewritten_pending(void) {
> + struct strbuf buf = STRBUF_INIT;
> + unsigned char newsha1[20];
> + FILE *out;
> +
> + if (strbuf_read_file(&buf, rebase_path_rewritten_pending(), 82) > 0 &&
> + !get_sha1("HEAD", newsha1) &&
> + (out = fopen(rebase_path_rewritten_list(), "a"))) {
An error in fopen() here ...
> + ...
> + }
> + strbuf_release(&buf);
> +}
> +
> +static void record_in_rewritten(struct object_id *oid,
> + enum todo_command next_command) {
> + FILE *out = fopen(rebase_path_rewritten_pending(), "a");
> +
> + if (!out)
> + return;
... and here are ignored as an insignificant error in the scripted
version, and this one does the same.
> +
> + fprintf(out, "%s\n", oid_to_hex(oid));
> + fclose(out);
> +
> + if (!is_fixup(next_command))
> + flush_rewritten_pending();
> +}
> +
> static int do_pick_commit(enum todo_command command, struct commit *commit,
> struct replay_opts *opts, int final_fixup)
> {
> @@ -1750,6 +1797,17 @@ static int is_final_fixup(struct todo_list *todo_list)
> return 1;
> }
>
> +static enum todo_command peek_command(struct todo_list *todo_list, int
> offset)
> +{
> + int i;
> +
> + for (i = todo_list->current + offset; i < todo_list->nr; i++)
> + if (todo_list->items[i].command != TODO_NOOP)
> + return todo_list->items[i].command;
Makes me wonder, after having commented on 07/34 regarding the fact
that in the end you would end up having three variants of no-op
(i.e. NOOP, DROP and COMMENT), what definition of a "command" this
function uses to return its result, when asked to "peek". I suspect
that this will be updated in a later patch to do "< TODO_NOOP"
instead? If so, then that answers one question in my comment on
07/34, i.e.
If a check for "is it one of the no-op commands?" appears only
here, a single liner comment may be sufficient (but necessary)
to help readers. Otherwise a single-liner helper function
(similar to is_fixup() you have) with a descriptive name would
be better than a single liner comment.
The answer is "no, it is not just there" hence the conclusion is "we
want a helper with a descriptive name".
