Re: [PATCH] blame: handle broken commit headers gracefully
On Wed, Apr 17, 2013 at 02:55:29PM -0700, Junio C Hamano wrote: Or you can imagine nastier input strings, like Name -email@host 123456789 - Name ema-il@host 123456789 - Name email@host~ 123456789 - I am afraid that at some point we should salvage as much as we can, which is a worthy goal, becomes a losing proposition. Good point. In the worst cases, even if you cleaned things up, you might even need to allocate a new string (like your middle one), which would make calling split_ident_line a lot more annoying. Probably not worth the effort. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] blame: handle broken commit headers gracefully
On Wed, Apr 17, 2013 at 08:33:54PM +0200, René Scharfe wrote: Minimal patch, test case missing. It's a bit sad that the old commit parser of blame handled Ivan's specific corruption (extra - after email) gracefully because it used the spaces as cutting points instead of and . That may mean there is room for improvement in split_ident_line to be more resilient in removing cruft. With something like: Name email@host- 123456789 - it would obviously be nice to find the date timestamp there, but I wonder what the email field should return? The full broken string, or just email@host? One way is convenient for overlooking problems in broken commits, but I would worry about code paths that are using split_ident_line to verify the quality of the string (like determine_author_info). It's possible we would need a strict and a forgiving mode. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] blame: handle broken commit headers gracefully
Jeff King p...@peff.net writes: On Wed, Apr 17, 2013 at 08:33:54PM +0200, René Scharfe wrote: Minimal patch, test case missing. It's a bit sad that the old commit parser of blame handled Ivan's specific corruption (extra - after email) gracefully because it used the spaces as cutting points instead of and . That may mean there is room for improvement in split_ident_line to be more resilient in removing cruft. With something like: Name email@host- 123456789 - it would obviously be nice to find the date timestamp there, but I wonder what the email field should return? The full broken string, or just email@host? Or you can imagine nastier input strings, like Name -email@host 123456789 - Name ema-il@host 123456789 - Name email@host~ 123456789 - I am afraid that at some point we should salvage as much as we can, which is a worthy goal, becomes a losing proposition. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
