On Thu, Nov 5, 2020 at 12:25 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > 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 strset is a set; there is no way to get duplicate entries. > 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. Is the expectation that strset_add() would return a boolean for whether a new entry was added?
