On Wed, May 17, 2023 at 01:48:42PM +0200, Patrick Steinhardt wrote:
> With 50957937f9 (fetch: introduce `display_format` enum, 2023-05-10), a
> new enumeration was introduced to determine the display format that is
> to be used by git-fetch(1). The `DISPLAY_FORMAT_UNKNOWN` value isn't
> ever used though, and neither do we rely on the explicit `0` value for
> initialization anywhere.
To be slightly pedantic, we'd also want to make sure the we do not rely
on the zero value for reading, like:
if (display_state->format)
....
But having looked over the code, we don't (it's always a switch or
equality with a known name), so this is safe to do.
Thanks for cleaning this up.
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index 849a9be421..9147b700e5 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -58,7 +58,6 @@ enum {
> };
>
> enum display_format {
> - DISPLAY_FORMAT_UNKNOWN = 0,
> DISPLAY_FORMAT_FULL,
> DISPLAY_FORMAT_COMPACT,
> DISPLAY_FORMAT_PORCELAIN,
Just for similar situations in the future, I think we could do:
DISPLAY_FORMAT_FULL = 1,
if we were worried about keeping the zero-behavior the same for existing
callers. But given how new and how limited this code is, I feel
confident that we've checked all of the code, and what you've written
above is preferable.
-Peff
