Thread (16 messages) 16 messages, 2 authors, 2023-05-22

Re: [PATCH 1/9] fetch: drop unused DISPLAY_FORMAT_UNKNOWN enum value

From: Jeff King <hidden>
Date: 2023-05-19 00:13:15

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.
quoted hunk ↗ jump to hunk
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help