On 1/28/2022 1:05 PM, Junio C Hamano wrote: > Derrick Stolee <stolee@xxxxxxxxx> writes: > >> My understanding was that this was ejected so we could find the right time >> to lib-ify handle_options() (as Taylor suggested), but we didn't want to do >> that while Scalar was still in a tentative state (in contrib/ with a plan >> to move it out after more is implemented). >> >> The biggest benefits of using handle_options() is for other pre-command >> options such as --exec-path, which I use on a regular basis when testing >> new functionality. >> >> There are other options in handle_options() that might not be appropriate, >> or might be incorrect if we just make handle_options() non-static. For >> example, `scalar --list-cmds=parseopt` wouldn't show the scalar commands >> and would instead show the git commands. >> >> So my feeling is that we should continue to delay this functionality until >> Scalar is more stable, perhaps even until after it moves out of contrib/. >> The need to change handle_options() to work with a new top-level command >> is novel enough to be worth careful scrutiny, but that effort is only >> valuable if the Git community is more committed to having Scalar in the >> tree for the long term. > > The usual caveat that little things tend to accumulate and you can > die from thousands of paper cuts aside, if "run in that directory" > and "pretend these configuration variables are set" are useful > enough features to Scalar, I think that the straight-forward option > parser this patch adds is very much acceptable. > > If we need more options (i.e. when we need to add the third thing), > that would be the best time to see how handle_options() can be > restructured to serve the different set of options git and Scaler > need. > > And this loop, which is too small to be even called "duplicated > implementation", should suffice in the meantime. I have come around to this line of thinking. My response to Johannes' detailed examination of my idea goes into that more deeply. Thanks, -Stolee
