Jeff King <peff@xxxxxxxx> writes: > Subject: [PATCH] shortlog: drop custom strset implementation > > We can use the strset recently added in strmap.h instead. Note that this > doesn't have a "check_and_add" function. We can easily write the same > thing using separate "contains" and "adds" calls. This is slightly less > efficient, in that it hashes the string twice, but for our use here it > shouldn't be a big deal either way. > > I did leave it as a separate helper function, though, since we use it in > three separate spots (some of which are in the middle of a conditional). It makes sense, but check_dup() sounds like its use pattern would be if (check_dup(it) == NO_DUP) add(it); where it is more like "add it but just once". By the way, is a strset a set or a bag? If it makes the second add an no-op, perhaps your check_dup() is what strset_add() should do itself? What builtin/shortlog.c::check_dup() does smells like it is a workaround for the lack of a naturally-expected feature.
