Hi Junio,

On Mon, 10 Oct 2016, Junio C Hamano wrote:

> Johannes Schindelin <johannes.schinde...@gmx.de> 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;

Reply via email to