On 2 October 2017 at 05:49, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Martin Ågren <martin.agren@xxxxxxxxx> writes: > >> ... Instead, require that one of the >> flags is set. Adjust documentation and the assert we already have for >> checking that we don't have too many flags. Add a macro `HAS_SINGLE_BIT` >> (inspired by `HAS_MULTI_BITS`) to simplify this check and similar checks >> in the future. > > I do not have a strong opinion against this approach, but if > something can take only one of two values, wouldn't it make more > sense to express it as a single boolean, I wonder. Then there is no > need to invent a cute HAS_SINGLE_BIT() macro, either. > > "commit and leave it open" cannot be expressed with such a scheme, > but with the HAS_SINGLE_BIT() scheme it can't anyway, so... I did briefly consider renaming `flags` to `commit` and re-#defining the two flags to 0 and 1 (or even updating all the callers to use literal zeros and ones). It felt a bit awkward to downgrade `flags` to a bool -- normally we'd to the reverse change. But maybe I shouldn't have rejected that so easily. If we have a feeling we won't need other flags (or the "don't even close the file") any time soon, maybe it'd be good to tighten things up a bit. Thanks for looking at these.
