On Wed, Apr 16, 2025 at 11:50:42AM -0700, Junio C Hamano wrote: > Patrick Steinhardt <ps@xxxxxx> writes: > > > We have two generic ways to parse integers in the "parse-options" > > subsystem: > > > > - `OPTION_INTEGER` parses a signed integer. > > > > - `OPTION_MAGNITUDE` parses an unsigned integer, but it also > > interprets suffixes like "k" or "g". > > > > Notably missing is a middle ground that parses unsigned integers without > > interpreting suffixes. Introduce a new `OPTION_UNSIGNED` option type to > > plug this gap. This option type will be used in subsequent commits. > > This takes a turn in a bit unexpected direction. Because the way to > spell a scaled quantity is unambiguous in the sense that anything > that used to be parsed with OPT_INTEGER() couldn't have had anything > but "^-?[0-9]+$", an obvious alternative is to teach OPTION_INTEGER > to always allow the scaling suffix if the user wants to use one, > without adding a new "only numbers but this one does not even allow > a sign" variant. Yes, that would be the alternative indeed. I wanted to have this patch so that I can adapt callsites that accidentally pass unsigned integers to the signed option without introducing any kind of incompatibility. So I was mostly aiming for 1:1 compatibility. But I'm equally fine with extending `OPT_INTEGER()` to support parsing magnitudes. I guess it's even preferable: less overhead on our side, and more consistency for our users. > Seriously, are there good candidates for an option where we want to > absolutely refuse to take scaling suffix and insist only on a bare > number? I cannot think of any specific reason. Patrick
