Re: [PATCH v2 5/5] convert: add filter.<driver>.process option

On Thu, Jul 28, 2016 at 02:10:12PM +0200, Lars Schneider wrote:

quoted

I think that's orthogonal. I just mean that using zero for success puts
you in our usual style, and then accumulating errors can be done with
"|=".

Ah. I guess I was misguided by the way errors are currently handled
in `apply_filter` (success = 1; failure = 0):
https://github.com/git/git/blob/8c6d1f9807c67532e7fb545a944b064faff0f70b/convert.c#L437-L479

I wouldn't like if the different filter protocols would use different
error exit codes. Would it be OK to adjust the existing `apply_filter`
function in a cleanup patch?

Ah, I see.

I think those return codes are a little different. They are not "success
or error", but "did convert or did not convert" (or "would convert" when
no buffer is given). And unless the filter is required, we quietly turn
errors into "did not convert" (and if it is, we die()).

So I'm not sure if changing them is a good idea. I agree with you that
it's probably inviting confusion to have the two sets of filter
functions have opposite return codes. So I think I retract my
suggestion. :)

-Peff