Re: [PATCH 53/83] builtin/apply: make find_header() return -1 instead of die()ing

From: Duy Nguyen <pclouds@gmail.com>
Date: Apr 25, 2016, 13:18 UTC
Message-ID: <CACsJy8B7g8WgWbr4M4LHFyBshN2gi30pL1bCcZcTPB7qtmnWdQ@mail.gmail.com>
In-Reply-To: <1461504863-15946-54-git-send-email-chriscool@tuxfamily.org>

On Sun, Apr 24, 2016 at 8:33 PM, Christian Couder <christian.couder@gmail.com> wrote:

Show 5 quoted lines

> To be compatible with the rest of the error handling in builtin/apply.c,
> find_header() should return -1 instead of calling die().
>
> Unfortunately find_header() already returns -1 when no header is found,
> so let's make it return -2 instead in this case.

I don't think this is a good way to go. Too many magic numbers. I don't have a better option though. Maybe returning names instead of numbers would help a bit.

Show 5 quoted lines

>  test_expect_success 'compare diagnostic; ensure file is still here' '
> -       echo "fatal: git diff header lacks filename information (line 4)" >expected &&
> +       echo "error: git diff header lacks filename information (line 4)" >expected &&
>         test_path_is_file f &&
>         test_cmp expected actual

Not your fault. But this test_cmp should be test_i18ncmp. I don't know if anybody has still been testing with gettext poison though. May probably not worth changing.

-- 
Duy