Re: [PATCH 2/8] pretty: simplify if-else to reduce code duplication
- From
Jeff King <peff@peff.net>
- Date
- Mar 24, 2025, 03:50 UTC
- Message-ID
- <20250324035001.GC690093@coredump.intra.peff.net>
- In-Reply-To
- <5f787ddac2d80391feadb8cf6be379fc8e58652f.1742367347.git.martin.agren@gmail.com>
On Wed, Mar 19, 2025 at 08:23:35AM +0100, Martin Ågren wrote:
Show 8 quoted lines
> First we look for "auto,", then we try "always,", then we fall back to > the default, which is to do exactly the same thing as we do for "auto,". > The amount of code duplication isn't huge, but still: reading this code > carefully requires spending at least *some* time on making sure the two > blocks of code are indeed identical. > > Rearrange the checks so that we end with the default case, > opportunistically consuming the "auto," which may or may not be there.
OK. The duplicated lines are not all that long, but I don't mind collapsing the cases, especially with the explanatory comment that's there.
Show 10 quoted lines
> In the "always," case, we don't actually *do* anything, so if we were
> into golfing, we'd just write the whole thing as a single
>
> if (!skip_prefix(begin, "always,", &begin)) {
> ...
> }
>
> If we ever learn something new besides "always," and "auto," we'd need
> to pull things apart again. Plus we still need somewhere to place the
> comment. Let's focus on code de-duplication rather than golfing for now.Yeah, I think what you wrote in the patch is much better than trying to golf further.
So looks good to me.
-Peff