Patrick Steinhardt wrote:
> On Wed, May 10, 2023 at 11:05:19AM -0700, Junio C Hamano wrote:
> > OK, this time the familiar "prepare a variable to its default, let
> > config callback to overwrite it by reading configuration variables,
> > and then let the command line option override it" is used and the
> > result easy to follow. I do not think .display_format is never
> > assigned DISPLAY_FORMAT_UNKNOWN with this change, so [6/9] could
> > lose the value from the enum, I think. The defensive switch
> > statement that has BUG() to notice an erroneous caller that pass
> > values other than DISPLAY_FORMAT_{FULL,COMPACT} is still a good
> > idea.
>
> Ah, right, `DISPLAY_FORMAT_UNKNOWN` isn't really needed anymore. I think
> it's still good to have valid values of the enum start with `1` so that
> it becomes easier to detect cases where we accidentally use a default
> initialized variable. But that can be achieved without giving the
> default value an explicit name.
I think it's standard to have an element like that. It could be UNKNOWN, NULL,
INVALID, or even DEFAULT.
For example, you could have code like this:
if (format == DISPLAY_FORMAT_DEFAULT)
format = DISPLAY_FORMAT_FULL;
This would distinguish cases in which the user did not specify a display format
and we choose a default, from those where the user explicitely chose the format
that happens to be the default.
--
Felipe Contreras
