Re: [PATCH] blame: handle broken commit headers gracefully

Jeff King [off-list ref] writes:

On Wed, Apr 17, 2013 at 08:33:54PM +0200, René Scharfe wrote:

quoted

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 -0000

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 -0000
   Name <ema>-<il@host> 123456789 -0000
   Name <email@host~ 1234>56789 -0000

I am afraid that at some point "we should salvage as much as we
can", which is a worthy goal, becomes a losing proposition.

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 -0000
   Name <ema>-<il@host> 123456789 -0000
   Name <email@host~ 1234>56789 -0000

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