Re: [PATCH v1 1/3] replace: add --graft option

[Jeff King <peff@peff.net>]

On Fri, May 23, 2014 at 01:05:40PM -0700, Junio C Hamano wrote:

> I just read read_graft_line(); it allows an empty line (both
> length-0 before the terminating LF or CRLF, and a line with
> isspace() only) and ignore them, so "grep '^[^#]'" is not
> sufficient.

Thanks, I missed the space trimming. I think:

  grep '^[^# \t]'

would be enough, though I am not sure that "\t" is portable over a raw
tab.

> > ...this number-based parsing safe, though it would miss removing a stray
> > parent line elsewhere in the commit. It still feels rather magical to
> > me, as we are depending on unspoken format guarantees defined inside
> > parse_commit_buffer.
> 
> Do you mean "we have a carved-in-stone rule that all 'parent ' lines
> must come immediately after a single 'tree ' line, and that is
> implemented in parse_commit_buffer().  The above code follows the
> exact same rule.  It would be nice to have some mechanism to tell
> somebody who wants to update the former that s/he must update this
> new code at the same time"?

Yes, exactly.

> Perhaps a more future-proof way to write Christian's code may be:
> 
>     - find "tree ";
> 
>     - splice the new parents in immediately after that "tree " line;
> 
>     - starting from the end of these new parents, scan up to the end
>       of the header line-by-line, and splice out any line that
>       begins with "parent ".
> 
> which may not be too bad.

Sounds familiar:

  http://article.gmane.org/gmane.comp.version-control.git/249575

-Peff