Re: [PATCH b4] ez: avoid duplicate signatures on resend

Hi!

On 6/9/26 23:20, Konstantin Ryabitsev wrote:
> On Mon, 25 May 2026 01:14:23 -0300, Cássio Gabriel <[email protected]> wrote:
>> diff --git a/src/b4/ez.py b/src/b4/ez.py
>> index 3e81138..4f55f0f 100644
>> --- a/src/b4/ez.py
>> +++ b/src/b4/ez.py
>> @@ -1734,6 +1734,15 @@ def get_cover_subject_body(cover: str) -> Tuple[b4.LoreSubject, str]:
>>      return lsubject, cbody
>>  
>>  
>> +def strip_cover_signature(cbody: str) -> str:
>> +    body = cbody.replace('\r', '').rstrip('\n')
>> +    sigpos = max(body.rfind('\n-- \n'), body.rfind('\n--\n'))
>> +    if sigpos >= 0:
>> +        return body[:sigpos].rstrip('\n')
>> +
>> +    return body
> 
> No, you should drop the '\n--\n' (bare, no trailing space) case from
> the sigpos search. The canonical email signature delimiter is '\n-- \n'
> — that trailing space is load-bearing and intentional and we
> deliberately only recognise the canonical form, and for good reason:
> bare '--' appears legitimately in cover letter prose — markdown
> horizontal rules, CLI flag documentation, SQL comments, and various
> other contexts.  Matching it here can silently truncate the cover body
> in ways the author would never expect, and the conservative posture of
> the existing code is the right one. If someone's cover carries '\n--\n'
> from a non-conformant mailer, a doubled footer on resend is a far
> smaller problem than silently eating half the cover text.
> 
> With the bare '--' case removed the sigpos line simplifies to:
> 
>     sigpos = body.rfind('\n-- \n')
> 
> and the '--' parametrize case in the test should be removed too.
> 
> This is the right fix for the resend path. The previous cbody.strip()
> also stripped leading whitespace, which this helper does not -- a minor
> behaviour change worth looking out for in case it wasn't intentional.
> 
>>
>> diff --git a/src/tests/test_ez.py b/src/tests/test_ez.py
>> index 3d05ef9..5abfea2 100644
>> --- a/src/tests/test_ez.py
>> +++ b/src/tests/test_ez.py
>> @@ -408,3 +408,24 @@ def test_store_cover_preserves_series_notes(prepdir_commit: str) -> None:
>> [ ... skip 18 lines ... ]
>> +        'Cover text.\n\n'
>> +        '---\n'
>> +        'base-commit: abc123\n'
>> +        'change-id: 20260101-test-change-id\n\n'
>> +        'Best regards,'
>> +    )
> 
> The '-- ' parametrize case is correct and should be kept. The '--' case
> should be removed along with the bare-delimiter matching in the helper itself.
> Optionally, adding a no-signature case would lock in that the function returns
> the body unchanged (modulo trailing-newline normalization) when no delimiter
> is present.

Thanks for the review.

I'll respin these suggestions in v2.

-- 
Thanks,
Cássio

-----BEGIN PGP SIGNATURE-----

wnsEABYIACMWIQSrYqI5vIrg1X9eqEjQXT8aWv/ugwUCaizAsQUDAAAAAAAKCRDQXT8aWv/ugwc3
AP43ljx8ZbIO4CRKXjZGXrPTxj/e45lvZWyfucNktjEqUwD8DT6AaYxaDhTZgbJguX33iZVcEpjE
CtD9dBoqpk2EfA0=
=4rPj
-----END PGP SIGNATURE-----