Subject: Re: [PATCH v2 3/3] pkt-line: do not chomp newlines for sideband messages

From: Junio C Hamano <[email-masked]>
Date: Wed, 04 Oct 2023 13:05:33 -0700

Jiang Xin <[email-masked]> writes:

> On Tue, Sep 26, 2023 at 4:48 PM Oswald Buddenhagen
> <[email-masked]> wrote:
>>
>> >Jiang Xin <[email-masked]> writes:
>> >
>> >> +++ b/pkt-line.c
>> >> @@ -462,8 +462,33 @@ enum packet_read_status packet_read_with_status(int fd, char **src_buffer,
>> >>      }
>> >> +                    case 2:
>> >> +                            /* fallthrough */
>> >> +                    case 3:
>> >
>> while not entirely unprecedented, it's unnecessary and even
>> counter-productive to annotate directly adjacent cases with fallthrough.
>
> I see in "blame.c" there are directly adjacent cases like below. I
> will remove the fallthrough statement.
>
>         case 'A':
>         case 'T':
>                 /* Did not exist in parent, or type changed */
>                 break;

Yeah, it is far clearer to understand if it is written without the
"fallthru" comment between the cases and instead a comment that
explains both cases after them (exactly like the example you found
in "blame.c").  When we want "fallthru" comment is if we had some
processing specific to the earlier case ('A' or '2') that is not
done in the later case ('T' or '3'), in which case we may want to
explicitly say we did not forget to "break" by adding the "fallthru"
comment.  But it does not apply here.

Thanks.