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

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.

-- 
Konstantin Ryabitsev <[email protected]>