On 4/25/2025 12:27 PM, Junio C Hamano wrote: > Patrick Steinhardt <ps@xxxxxx> writes: > >>>> Is there a reason why we don't use `parse_options()` here? It might make >>>> this tool easier to use and extend going forward, and we wouldn't have >>>> to care about invalid arguments. Right now, we silently accept a >>>> non-integer argument and do the wrong thing. >>> >>> I think that `parse_options()` would be overkill here because: >>> >>> - This is a _mandatory_ argument, not an optional one. >>> >>> - The required data type is `uint32_t`, and `parse_options()` has no >>> support for that. >> >> Support for that has been merged just this week via 2bc5414c411 (Merge >> branch 'ps/parse-options-integers', 2025-04-24). The thing that confused me even with those changes is that this is a _positional_ argument and we don't have a way to say "parse the 1st positional argument into an integer". >>> But you do have a good point in that we may want to validate the data type >>> (even if technically, this is not a user-facing program, it's a test >>> helper that is used under tight control by Git's own test suite). >>> >>> Consequently, I would suggest this fixup instead: >> >> But in any case, I'd be equally fine with your suggestion. > > Yeah, I think we clearly showed our "it's just test helper, whose > callers are supposed to know what they are doing" attitude, but with > proper helpers, it is not too much additional effort to do the right > thing. But with this philosophy in mind I can change the CLI to be of the form "--num-objects <n>" to use the parse-options feature. This should make things more extensible in the future. Thanks, -Stolee
