On Wed, Mar 19, 2025 at 08:23:35AM +0100, Martin Ågren wrote:
> 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.
> 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
