Jeff King <peff@xxxxxxxx> writes: > It's a lot of patches, but hopefully they're all pretty straightforward > to read. Yes, quite a lot of changes. I didn't see anything questionable in there. As to the "patch-id" thing, I find the alternate one slightly easier to read. Also, exactly because this is not a performance critical codepath, it may be better if patch_id_add_string() filtered out whitespaces; that would allow the source to express things in more natural way, e.g. patch_id_addf(&ctx, "new file mode"); patch_id_addf(&ctx, "%06o", p->two->mode); patch_id_addf(&ctx, "--- /dev/null"); patch_id_addf(&ctx, "+++ b/%.*s", len2, p->two->path); Or I may be going overboard by bringing "addf" into the mix X-<.