Re: [PATCH v2 5/6] trailer: rename *_DEFAULT enums to *_UNSPECIFIED
- From
- Linus Arver <linusa@google.com>
- Date
- Sep 26, 2023, 05:30 UTC
- Message-ID
- <owlywmwdsdr0.fsf@fine.c.googlers.com>
- In-Reply-To
- <xmqqil820z25.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
Show 18 quoted lines
> Linus Arver <linusa@google.com> writes: > >> ... I prefer the >> WHERE_UNSPECIFIED as in this patch because the WHERE_DEFAULT is >> ambiguous on its own (i.e., WHERE_DEFAULT could mean that we either use >> the default value WHERE_END in default_conf_info, or it could mean that >> we fall back to the configuration variables (where it could be something >> else)). > > Yup. "Turning something that is left UNSPECIFIED after command line > options and configuration files are processed into the hardcoded > DEFAULT" is one mental model that is easy to explain. > > I however am not sure if it is easier than "Setting something to > hardcoded DEFAULT before command line options and configuration > files are allowed to tweak it, and if nobody touches it, then it > gets the hardcoded DEFAULT value in the end", which is another valid > mental model, though.
True.
> If both can be used, I'd personally prefer > the latter, and reserve the "UNSPECIFIED to DEFAULT" pattern to > signal that we are dealing with a case where the simpler pattern > without UNSPECIFIED cannot solve.
SGTM.
Show 21 quoted lines
> The simpler pattern would not work, when the default is defined > depending on a larger context. Imagine we have two Boolean > variables, A and B, where A defaults to false, and B defaults to > some value derived from the value of A (say, opposite of A). > > In the most natural implementation, you'd initialize A to false and > B to unspecified, let command line options and configuration > variables to set them to true or false, and after all that, you do > not have to tweak A's value (it will be left to false that is the > default unless the user or the configuration gave an explicit > value), but you need to check if B is left unspecified and tweak it > to true or false using the final value of A. > > For a variable with such a need like B, we cannot avoid having > "unspecified". If you initialize it to false (or true), after the > command line and the configuration files are read and you find B is > set to false (or true), you cannot tell if the user or the > configuration explicitly set B to false (or true), in which case you > do not want to futz with its value based on what is in A, or it is > false (or true) only because nobody touched it, in which case you > need to compute its value based on what is in A.
Thanks for the illustrative example! I don't think we have a case of a "B" variable here for trailers.
> And that is why I asked if we need to special case "the user did not > touch and the variable is left untouched" in the trailer subsystem.
I think the answer is "no, we don't need to special case". I'll be dropping this patch in the next re-roll.
> Thanks.