Re: [PATCH v8 7/7] convert: add round trip check based on 'core.checkRoundtripEncoding'

On Sun, Mar 4, 2018 at 2:08 PM, Lars Schneider [off-list ref] wrote:

quoted

On 25 Feb 2018, at 20:50, Eric Sunshine [off-list ref] wrote:
On Sat, Feb 24, 2018 at 11:28 AM,  [off-list ref] wrote:

quoted

+               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.

quoted

quoted

+       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.