Re: [PATCH v3 1/9] rebase -i: generate the script via rebase--helper
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Apr 27, 2017, 04:31 UTC
- Message-ID
- <xmqqvapqo4i8.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <c44a15ed1f1015d7e9377e18610a0c428786995b.1493207864.git.johannes.schindelin@gmx.de>
Johannes Schindelin <johannes.schindelin@gmx.de> writes:
Show 34 quoted lines
> diff --git a/sequencer.c b/sequencer.c
> index 77afecaebf0..e858a976279 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -2388,3 +2388,48 @@ void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag)
>
> strbuf_release(&sob);
> }
> +
> +int sequencer_make_script(int keep_empty, FILE *out,
> + int argc, const char **argv)
> +{
> + char *format = xstrdup("%s");
> + struct pretty_print_context pp = {0};
> + struct strbuf buf = STRBUF_INIT;
> + struct rev_info revs;
> + struct commit *commit;
> +
> + init_revisions(&revs, NULL);
> + revs.verbose_header = 1;
> + revs.max_parents = 1;
> + revs.cherry_pick = 1;
> + revs.limited = 1;
> + revs.reverse = 1;
> + revs.right_only = 1;
> + revs.sort_order = REV_SORT_IN_GRAPH_ORDER;
> + revs.topo_order = 1;
> +
> + revs.pretty_given = 1;
> + git_config_get_string("rebase.instructionFormat", &format);
> + get_commit_format(format, &revs);
> + free(format);
> + pp.fmt = revs.commit_format;
> + pp.output_encoding = get_log_output_encoding();All of the above feels like inviting unnecessary future breakages by knowing too much about the implementation the current version of revision.c happens to use. A more careful implementation would be to allocate our own av[] and prepare "--reverse", "--left-right", "--cherry-pick", etc. to be parsed by setup_revisions() call we see below. The parsing is not an expensive part of the operation anyway, and that way we do not have to worry about one less thing.
Show 10 quoted lines
> + if (setup_revisions(argc, argv, &revs, NULL) > 1)
> + return error(_("make_script: unhandled options"));
> +
> + if (prepare_revision_walk(&revs) < 0)
> + return error(_("make_script: error preparing revisions"));
> +
> + while ((commit = get_revision(&revs))) {
> + strbuf_reset(&buf);
> + if (!keep_empty && is_original_commit_empty(commit))
> + strbuf_addf(&buf, "%c ", comment_line_char);Presumably callers of this function (which does not exist yet at this step) are expected to have done the configuration dance to prepare comment_line_char to whatever the end-user specified?
Show 8 quoted lines
> + strbuf_addf(&buf, "pick %s ", oid_to_hex(&commit->object.oid)); > + pretty_print_commit(&pp, commit, &buf); > + strbuf_addch(&buf, '\n'); > + fputs(buf.buf, out); > + } > + strbuf_release(&buf); > + return 0; > +}
Other than that, this looks reasonable.
Show 14 quoted lines
> diff --git a/sequencer.h b/sequencer.h > index f885b68395f..83f2943b7a9 100644 > --- a/sequencer.h > +++ b/sequencer.h > @@ -45,6 +45,9 @@ int sequencer_continue(struct replay_opts *opts); > int sequencer_rollback(struct replay_opts *opts); > int sequencer_remove_state(struct replay_opts *opts); > > +int sequencer_make_script(int keep_empty, FILE *out, > + int argc, const char **argv); > + > extern const char sign_off_header[]; > > void append_signoff(struct strbuf *msgbuf, int ignore_footer, unsigned flag);