Re: [PATCH] ref-filter: treat CRLF as same as LF in find_subpos

From: Junio C Hamano <gitster@pobox.com>
Date: May 23, 2017, 02:01 UTC
Message-ID: <xmqq37bwnxg4.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170522201212.uuas26n6npdebsxg@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> I suspect that's more work because we'd need to refactor pretty.c a bit
> to make the right functionality available. But the end result would be
> much more maintainable.

I actually think the entire codeflow of "find positions and length of threeparts" using find_subpos() and then "copy the length bytes starting position for C_{SUB,BODY,SIG,LINES,...}" must be rethought, if the behavior of pretty.c::pretty_print_commit() is to be matched. With the current code, %(contents:body) and other atoms that are handled in ref-filter.c::grab_sub_body_contents() keep trailing whitespaces on their lines with the current code that copies length bytes starting the position using xmemdupz(). There need to be some code that loses these trailing whiltespaces in the copied result.

While I do not claim that refactoring and reusing code from pretty.c is the only viable way forward, it is clear to me that a patch that updates find_subpos() and changes nothing else falls short X-<.