Re: [PATCH] apply: reload .gitattributes after patching it

On Fri, Aug 09, 2019 at 11:25:52AM +0000, brian m. carlson wrote:

quoted

quoted

+			if (!flush_attributes && patch->new_name) {
+				char *dummy = strip_path_suffix(patch->new_name, GITATTRIBUTES_FILE);

It's a shame that 'strip_path_suffix' doesn't take a 'char *out', and
accept NULL for that (which would save us the assignment and subsequent
'free'). In either case, this is certainly the appropriate function.

Yeah, I felt the same way. We could refactor this out into two separate
functions, one which returns an ssize_t and one which actually
allocates, but I'm not sure it makes a huge amount of sense with just
one caller. The allocation is relatively small, and I've tried to make
sure it's called exactly once per patch so as not to be wasteful and
inefficient.

I think you could do this with:

  size_t len;
  if (strip_suffix(patch->new_name, GITATTRIBUTES_FILE, &len) &&
      len > 0 && is_dir_sep(patch->new_name[len-1]))
          flush_attributes = 1;

Not sure if that is better or worse. It avoids the extra memory
boilerplate, but the logic is a slightly more subtle.

-Peff