Re: [PATCH v2 3/9] rebase -i: comment out squash!/fixup! subjects from squash message

From: Phillip Wood <phillip.wood123@gmail.com>
Date: Jan 21, 2021, 16:58 UTC
Message-ID: <3853a4a0-dca4-4d8c-a9da-6608a9385f38@gmail.com>
In-Reply-To: <CAP8UFD3PRaiCiSfSMaX0FDrEcOz2xv3992meum7qnKve1rK6nw@mail.gmail.com>

Hi Christian and Charvi

On 21/01/2021 15:21, Christian Couder wrote:

Show 35 quoted lines

> On Thu, Jan 21, 2021 at 3:02 PM Charvi Mendiratta <charvi077@gmail.com> wrote:
>>
>> Hi Junio,
>>
>> On Thu, 21 Jan 2021 at 07:08, Junio C Hamano <gitster@pobox.com> wrote:
>>>
>>> Charvi Mendiratta <charvi077@gmail.com> writes:
>>>
>>>> +static size_t subject_length(const char *body)
>>>> +{
>>>> +     size_t i, len = 0;
>>>> +     char c;
>>>> +     int blank_line = 1;
>>>> +     for (i = 0, c = body[i]; c; c = body[++i]) {
>>>> +             if (c == '\n') {
>>>> +                     if (blank_line)
>>>> +                             return len;
>>>> +                     len = i + 1;
>>>> +                     blank_line = 1;
>>>> +             } else if (!isspace(c)) {
>>>> +                     blank_line = 0;
>>>> +             }
>>>> +     }
>>>> +     return blank_line ? len : i;
>>>> +}
>>>
>>> I cannot quite tell what this loop is trying to compute at the first
>>> glance.
>>>
>>
>> Oops, I think Phillip and Christian also pointed in the last revision
>> to look for alternatives to make it easy. I mistook that point and
>> forgot to look at it.
> 
> Yes, please take a look at find_commit_subject() in "commit.c".

That looks like it is taking the commit header and finding the start of the message. We have just the message, I think we probably want to use format_subject() in pretty.c which does what my hard to follow code does with the option to replace newlines in the subject with another character.

Best Wishes

Phillip

Show 25 quoted lines

>>>> +     grep "extra para" commit >actual &&
>>>
>>> I can tell that you want the "extra para" to still remain, but how
>>> does the grep that is not anchored guarantee that?
>>
>> .. but now I think to remove this `grep -v "squash" commit` as also
>> discussed with Phillip earlier that in this test script we are not
>> checking for the commented commit message.
>>
>>> Perhaps look for
>>>
>>>          grep "^extra para" commit
>>>
>>> to ensure that you are not seeing a commented out but somehow failed
>>> to get stripspaced out?
>>>
>> I am not sure, what does failing to get stripspaced mean?
> 
> I think this refers to:
> 
> https://git-scm.com/docs/git-stripspace
> 
> Best,
> Christian.
>