Jeff King venit, vidit, dixit 18.02.2011 05:16: > On Thu, Feb 17, 2011 at 08:44:42AM +0100, Michael J Gruber wrote: > >> --- a/builtin/patch-id.c >> +++ b/builtin/patch-id.c >> @@ -73,6 +73,8 @@ int get_one_patchid(unsigned char *next_sha1, git_SHA_CTX *ctx) >> p += 7; >> else if (!memcmp(line, "From ", 5)) >> p += 5; >> + else if (!memcmp(line, "\\ ", 2) && strlen(line)>=12) >> + continue; > > Wow, that's pretty obscure. I wonder if the test should be factored out > into line_is_no_newline_at_end_of_file() (or surely there is some more > sensible name), and used by both apply and patch-id. Along with a nice > comment (which I see apply already has) describing what in the world the > magic number 12 means. So, where do you suggest it should go? As far as I can see, we have two places reading these markers (the above builtins) and two places writing them (diff.c, xdiff/xutils.c), at least for git-core. (git-gui's diff.tcl has its own strict check etc.) I would opt for putting a more detailed explanation in the commit message (reasoning + ref. to apply.c) rather then trying to factor this. After all, our code is largely undocumented as far as inline documentation goes (it has frustrated me a few times), and the more comprehensive documentation are the commit messages which git blame points you to. Michael -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html