Johannes Schindelin <johannes.schindelin@xxxxxx> writes: > Let's just bite the bullet and rewrite the entire parser; the code now > ... > In particular, we choose to maintain the list of commands in an array > instead of a linked list: this is flexible enough to allow us later on to > even implement rebase -i's reordering of fixup!/squash! commits very > easily (and with a very nice speed bonus, at least on Windows). > > While at it, do not stop at the first problem, but list *all* of the > problems. This will help the user when the sequencer will do `rebase > -i`'s work by allowing to address all issues in one go rather than going > back and forth until the todo list is valid. All sounds sensible. > 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. I just found it in CodingGuidelines, cbcfd4e3ea ("i18n: mention "TRANSLATORS:" marker in Documentation/CodingGuidelines", 2014-04-18). > + 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. Thanks.