Hi Junio,
On Mon, 10 Oct 2016, Junio C Hamano wrote:
> Johannes Schindelin <johannes.schindelin@xxxxxx> writes:
>
> > if (parent && parse_commit(parent) < 0)
> > - /* TRANSLATORS: The first %s will be "revert" or
> > - "cherry-pick", the second %s a SHA1 */
> > + /*
> > + * TRANSLATORS: The first %s will be a "todo" command like
> > + * "revert" or "pick", the second %s a SHA1.
> > + */
>
> You may want to double check this with i18n folks; IIRC the tool
> that extracts TRANSLATORS: comment was somewhat particular about
> where that magic "TRANSLATORS:" token resides on a comment line and
> that is why we have this multi-line comment formatted in an unusual
> way.
>
> Ahh, no you do not have to bug i18n folks. 47fbfded53 ("i18n: only
> extract comments marked with "TRANSLATORS:"", 2014-04-17) is an
> example of such an adjustment.
Urgh. Thanks for pointing this out to me, though. Will be fixed in the
next iteration.
> > + while ((commit = get_revision(opts->revs))) {
> > + struct todo_item *item = append_new_todo(todo_list);
> > + const char *commit_buffer = get_commit_buffer(commit, NULL);
> > + const char *subject;
> > + int subject_len;
> > +
> > + item->command = command;
> > + item->commit = commit;
> > + item->offset_in_buf = todo_list->buf.len;
> > + subject_len = find_commit_subject(commit_buffer, &subject);
> > + strbuf_addf(&todo_list->buf, "%s %s %.*s\n", command_string,
> > + find_unique_abbrev(commit->object.oid.hash,
> > + DEFAULT_ABBREV),
> > + subject_len, subject);
>
> I am personally fine with this line; two things come to mind:
>
> - This would work just fine as-is with Linus's change to turn
> DEFAULT_ABBREV to -1.
>
> - It appears that it is more fashionable to use
> strbuf_add_unique_abbrev() these days.
Right, I actually looked at this place when I tried to decide where I
could use that function. Somehow I thought I'd not break up the flow here.
But since you asked so nicely, I'll squash this in (I personally find it
uglier, and longer, but it does use strbuf_add_unique_abbrev() now):
-- snipsnap --
@@ -906,11 +904,13 @@ static int walk_revs_populate_todo(struct todo_list
*todo_list,
item->command = command;
item->commit = commit;
item->offset_in_buf = todo_list->buf.len;
+ strbuf_addstr(&todo_list->buf, command);
+ strbuf_addch(&todo_list->buf, ' ');
+ strbuf_add_unique_abbrev(&todo_list->buf,
+ commit->object.oid.hash,
+ DEFAULT_ABBREV);
subject_len = find_commit_subject(commit_buffer, &subject);
- strbuf_addf(&todo_list->buf, "%s %s %.*s\n",
command_string,
- find_unique_abbrev(commit->object.oid.hash,
- DEFAULT_ABBREV),
- subject_len, subject);
+ strbuf_add(&todo_list->buf, subject, subject_len);
unuse_commit_buffer(commit, commit_buffer);
}
return 0;