Hi Liam,
On Thu, 25 May 2017, Liam Beguin wrote:
> Johannes Schindelin <johannes.schindelin@xxxxxx> writes:
>
> > diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
> > index 821058d452d..9444c8d6c60 100644
> > --- a/builtin/rebase--helper.c
> > +++ b/builtin/rebase--helper.c
> > @@ -24,6 +24,10 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix)
> > ABORT),
> > OPT_CMDMODE(0, "make-script", &command,
> > N_("make rebase script"), MAKE_SCRIPT),
> > + OPT_CMDMODE(0, "shorten-sha1s", &command,
> > + N_("shorten SHA-1s in the todo list"), SHORTEN_SHA1S),
> > + OPT_CMDMODE(0, "expand-sha1s", &command,
> > + N_("expand SHA-1s in the todo list"), EXPAND_SHA1S),
>
> Since work is being done to convert to `struct object_id` would it
> not be best to use a more generic name instead of 'sha1'?
> maybe something like {shorten,expand}-hashs
Good point. You suggest the use of "ids" later, and I think that is an
even better name: what we try to do here is to expand/reduce the commit
*identifiers*. The fact that they are hexadecimal representations of
hashes is an implementation detail.
> > diff --git a/sequencer.c b/sequencer.c
> > index 88819a1a2a9..201d45b1677 100644
> > --- a/sequencer.c
> > +++ b/sequencer.c
> > @@ -2437,3 +2437,60 @@ int sequencer_make_script(int keep_empty, FILE *out,
> > strbuf_release(&buf);
> > return 0;
> > }
> > +
> > +
> > +int transform_todo_ids(int shorten_sha1s)
> > +{
> > + const char *todo_file = rebase_path_todo();
> > + struct todo_list todo_list = TODO_LIST_INIT;
> > + int fd, res, i;
> > + FILE *out;
> > +
> > + strbuf_reset(&todo_list.buf);
> > + fd = open(todo_file, O_RDONLY);
> > + if (fd < 0)
> > + return error_errno(_("could not open '%s'"), todo_file);
> > + if (strbuf_read(&todo_list.buf, fd, 0) < 0) {
> > + close(fd);
> > + return error(_("could not read '%s'."), todo_file);
> > + }
> > + close(fd);
> > +
> > + res = parse_insn_buffer(todo_list.buf.buf, &todo_list);
> > + if (res) {
> > + todo_list_release(&todo_list);
> > + return error(_("unusable instruction sheet: '%s'"), todo_file);
>
> As you pointed out last time, the name of the "todo script" can be a
> source of confusion. The migration to C could be a good opportunity to
> clarify this.
True. This was simply a copy-edited part, and I should have caught that.
> I don't know which is the preferred name but we could go with
> "todo list" as it is the most common across the code base.
Yep, my next iteration will use that term.
> > + }
> > +
> > + out = fopen(todo_file, "w");
> > + if (!out) {
> > + todo_list_release(&todo_list);
> > + return error(_("unable to open '%s' for writing"), todo_file);
> > + }
> > + for (i = 0; i < todo_list.nr; i++) {
> > + struct todo_item *item = todo_list.items + i;
> > + int bol = item->offset_in_buf;
> > + const char *p = todo_list.buf.buf + bol;
> > + int eol = i + 1 < todo_list.nr ?
> > + todo_list.items[i + 1].offset_in_buf :
> > + todo_list.buf.len;
> > +
> > + if (item->command >= TODO_EXEC && item->command != TODO_DROP)
> > + fwrite(p, eol - bol, 1, out);
> > + else {
> > + const char *sha1 = shorten_sha1s ?
> > + short_commit_name(item->commit) :
> > + oid_to_hex(&item->commit->object.oid);
>
> We could also use 'hash' or 'ids' here instead of 'sha1'.
Absolutely!
Thank you,
Johannes