Re: [PATCH] xdiff: rename "struct group" to "struct xdlgroup"

From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Sep 27, 2016, 13:18 UTC
Message-ID: <9fb14a41-00b3-f8d1-d8d2-8aa41261492e@alum.mit.edu>
In-Reply-To: <20160927043733.u3emlanbipu2cn5h@sigill.intra.peff.net>

On 09/27/2016 06:37 AM, Jeff King wrote:

Show 15 quoted lines

> Commit e8adf23 (xdl_change_compact(): introduce the concept
> of a change group, 2016-08-22) added a "struct group" type
> to xdiff/xdiffi.c. But the POSIX system header "grp.h"
> already defines "struct group" (it is part of the getgrnam
> interface). This happens to work because the new type is
> local to xdiffi.c, and the xdiff code includes a relatively
> small set of system headers. But it will break compilation
> if xdiff ever switches to using git-compat-util.h.  It can
> also probably cause confusion with tools that look at the
> whole code base, like coccinelle or ctags.
> 
> Let's resolve by giving the xdiff variant a scoped name,
> which is closer to other xdiff types anyway (e.g.,
> xdlfile_t, though note that xdiff is fond if typedefs when
> Git usually is not).

Makes sense to me. I didn't try to adhere to xdiff conventions too tightly because I don't think that project is alive anymore, so I don't expect we'll be upstreaming anything [1]. But this change definitely makes sense.

Thanks, Michael

[1] Though I've since learned that libgit2 also bases their diff code on xdiff, so if we avoid changing things gratuitously there is more chance that our two projects can benefit from each other's improvements whenever they are also licensed compatibly.