On Sun, Mar 4, 2018 at 2:08 PM, Lars Schneider <larsxschnei...@gmail.com> wrote:
>> On 25 Feb 2018, at 20:50, Eric Sunshine <sunsh...@sunshineco.com> wrote:
>> On Sat, Feb 24, 2018 at 11:28 AM,  <lars.schnei...@autodesk.com> wrote:
>>> +               if (!re_src || src_len != re_src_len ||
>>> +                   memcmp(src, re_src, src_len)) {
>>> +                       const char* msg = _("encoding '%s' from %s to %s 
>>> and "
>>> +                                           "back is not the same");
>>> +                       die(msg, path, enc->name, default_encoding);
>>
>> Last two arguments need to be swapped.
>
> Hm. Are you sure? I think it is correct as it is. We are in encode_to_git()
> here and that means we encode *to* "default encoding", no?

Okay. I guess I was just looking at the most recent
reencode_string_len() -- and maybe overlooked the "and back" -- and
was thinking that this error message applied directly to it, but I see
your point about the error saying something about encode_to_git()
overall, in which case I agree with you.

>>> +       test_config core.checkRoundtripEncoding "garbage" &&
>>> +       ! GIT_TRACE=1 git add .gitattributes roundtrip.shift 2>&1 
>>> >/dev/null |
>>> +               grep "Checking roundtrip encoding for SHIFT-JIS" &&
>>> +       test_unconfig core.checkRoundtripEncoding &&
>>
>> The "unconfig" won't take place if the test fails. Instead of
>> test_config/test_unconfig, you could use '-c' to set the config
>> transiently for the git-add operation:
>>
>>    ! GIT_TRACE=1 git -c core.checkRoundtripEncoding=garbage add ...
>
> Agreed. Although test_config (in t/test-lib-functions.sh) automatically
> unsets itself after the test is over.

Yep, so you could get by with that alone. The test_unconfig() simply
isn't needed.

Reply via email to