Re: [PATCH v5 0/9] fetch: introduce machine-parseable output

[Felipe Contreras <felipe.contreras@gmail.com>]

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