* [PATCH b4 v2 0/4] Improve email quote trimming
@ 2026-09-07 19:22 Jason Gunthorpe
2026-09-07 19:22 ` [PATCH b4 v2 1/4] review: trim trailing quoted when adding a tag Jason Gunthorpe
` (3 more replies)
0 siblings, 4 replies; 5+ messages in thread
From: Jason Gunthorpe @ 2026-09-07 19:22 UTC (permalink / raw)
To: tools
Patches to make the automatic trimming more versatile and work in
more cases.
I would, broadly, like to edit the messages without editing or removing
any of the existing quoting. This is because I tend to make a few passes
at things and once quoted text is deleted it is a huge PITA to bring it
back.
Introduce a >--cut-- marker which tells b4 to cut the quoting and then b4
can reassemble the message similarly to if the editor macros had been
used.
Along the way fix a few corner cases in the email construction that
resulted in sub optimal email generation.
It took a while to get this generating emails the way I think it should,
I've included several of the cases that didn't obviously work as permanent
tests.
Include some very basic support for emacs to generate and highlight the cut marker.
I've opted to leave the existing editor support for cutting out quoted lines
alone, but I think it would be better if it could use editor folding and the
cut marker instead. That's far beyond me though :)
v2:
- Rebase
- Change cut to >--cut--
- Add some basic emacs support
- Consider mixed > and | quotes when trimming the reply for a trailer
v1: https://patch.msgid.link/r/0-v1-5f6173f1d4bf+3e9-trimming_jgg@nvidia.com
Jason Gunthorpe (4):
review: trim trailing quoted when adding a tag
review: discard blank lines between | and > quotes when trimming
review: add >--cut-- inline marker to snip quoted reply context
review: emacs: highlighting and keystroke for >--cut--
docs/maintainer/review.rst | 24 +++
docs/releases.rst | 9 +-
misc/emacs/b4-review-mode.el | 32 ++++
src/b4/review/_review.py | 150 ++++++++++++---
src/b4/review_tui/_review_app.py | 10 +-
src/tests/test_review.py | 317 +++++++++++++++++++++++++++++++
src/tests/test_tui_review.py | 69 +++++++
7 files changed, 586 insertions(+), 25 deletions(-)
base-commit: c363dd22f0f87689b63d6d32fb65bf07b73d6e66
--
2.43.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH b4 v2 1/4] review: trim trailing quoted when adding a tag
2026-09-07 19:22 [PATCH b4 v2 0/4] Improve email quote trimming Jason Gunthorpe
@ 2026-09-07 19:22 ` Jason Gunthorpe
2026-09-07 19:22 ` [PATCH b4 v2 2/4] review: discard blank lines between | and > quotes when trimming Jason Gunthorpe
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Jason Gunthorpe @ 2026-09-07 19:22 UTC (permalink / raw)
To: tools
If some comments are added and then a review tag is placed the trailing
quote that is normally trimmed is instead retained with the trailer
landing at the end of the message. Instead place it right after the last
user comment. Skip both > and | quoted sections.
Assisted-by: LLM
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
src/b4/review/_review.py | 18 +++++++++++++++---
src/tests/test_review.py | 34 ++++++++++++++++++++++++++++++++++
2 files changed, 49 insertions(+), 3 deletions(-)
diff --git a/src/b4/review/_review.py b/src/b4/review/_review.py
index a37533230f484a..96f3e5bbd54130 100644
--- a/src/b4/review/_review.py
+++ b/src/b4/review/_review.py
@@ -3297,13 +3297,25 @@ def _insert_trailer_in_reply(reply_text: str, trailer: str) -> str:
"""Append *trailer* as its own bare line in the reply buffer.
The line is added at the end of the maintainer's content, before any
- trailing ``-- `` signature, grouped with any trailers already there. The
- maintainer can move it afterwards — placement is just a sensible default.
+ trailing quoted context or ``-- `` signature, grouped with any trailers
+ already there. Keeping a trailing quote below the trailer lets the
+ ordinary send-time trimming discard it. The maintainer can move the
+ trailer afterwards — placement is just a sensible default.
"""
lines = _normalize_line_endings(reply_text).split('\n')
sig_idx = next((i for i, ln in enumerate(lines) if ln == '-- '), len(lines))
head = lines[:sig_idx]
tail = lines[sig_idx:]
+ quoted_tail: List[str] = []
+ quote_start = len(head)
+ while quote_start and (
+ head[quote_start - 1].startswith(('>', '|'))
+ or not head[quote_start - 1].strip()
+ ):
+ quote_start -= 1
+ if any(line.startswith('>') for line in head[quote_start:]):
+ quoted_tail = head[quote_start:]
+ head = head[:quote_start]
while head and not head[-1].strip():
head.pop()
# Separate from non-trailer content with a blank line so the trailers
@@ -3311,7 +3323,7 @@ def _insert_trailer_in_reply(reply_text: str, trailer: str) -> str:
if head and not _BARE_TRAILER_RE.match(head[-1]):
head.append('')
head.append(trailer)
- return '\n'.join(head + tail)
+ return '\n'.join(head + quoted_tail + tail)
def _remove_trailer_from_reply(reply_text: str, name: str) -> str:
diff --git a/src/tests/test_review.py b/src/tests/test_review.py
index 460cfb9b65196d..eb7e36c52eeebb 100644
--- a/src/tests/test_review.py
+++ b/src/tests/test_review.py
@@ -909,6 +909,40 @@ class TestReplyTrailerEditing:
# No blank line inserted between the two trailers.
assert 'Reviewed-by: Me <me@x.com>\nTested-by: Me <me@x.com>' in out
+ def test_insert_before_trailing_quote(self) -> None:
+ buf = 'Please fix this.\n\n> context left below my message\n> +old code\n'
+ trailer = 'Reviewed-by: Me <me@x.com>'
+ out = review._insert_trailer_in_reply(buf, trailer)
+ assert out == (
+ 'Please fix this.\n\n'
+ 'Reviewed-by: Me <me@x.com>\n'
+ '\n'
+ '> context left below my message\n'
+ '> +old code\n'
+ )
+ assert review._trim_quoted_reply(out) == 'Please fix this.\n\n' + trailer
+
+ def test_insert_before_trailing_quote_with_external_review(self) -> None:
+ buf = (
+ 'Please fix this.\n'
+ '\n'
+ '> context before external feedback\n'
+ '\n'
+ '| Bot <bot@example.com>:\n'
+ '|\n'
+ '| An external finding.\n'
+ '\n'
+ '> context after external feedback\n'
+ )
+ trailer = 'Reviewed-by: Me <me@x.com>'
+
+ out = review._insert_trailer_in_reply(buf, trailer)
+
+ # The external-review block is scaffolding inside one trailing quoted
+ # run. Adding a trailer must not split that run and make the quoted
+ # context before the external finding survive send-time trimming.
+ assert review._trim_quoted_reply(out) == 'Please fix this.\n\n' + trailer
+
def test_remove_drops_matching_bare_line(self) -> None:
buf = 'Thanks!\n\nReviewed-by: Me <me@x.com>\nAcked-by: Me <me@x.com>'
out = review._remove_trailer_from_reply(buf, 'reviewed-by')
--
2.43.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH b4 v2 2/4] review: discard blank lines between | and > quotes when trimming
2026-09-07 19:22 [PATCH b4 v2 0/4] Improve email quote trimming Jason Gunthorpe
2026-09-07 19:22 ` [PATCH b4 v2 1/4] review: trim trailing quoted when adding a tag Jason Gunthorpe
@ 2026-09-07 19:22 ` Jason Gunthorpe
2026-09-07 19:22 ` [PATCH b4 v2 3/4] review: add >--cut-- inline marker to snip quoted reply context Jason Gunthorpe
2026-09-07 19:22 ` [PATCH b4 v2 4/4] review: emacs: highlighting and keystroke for >--cut-- Jason Gunthorpe
3 siblings, 0 replies; 5+ messages in thread
From: Jason Gunthorpe @ 2026-09-07 19:22 UTC (permalink / raw)
To: tools
When b4 constructs the email to respond to it inserts the | quoted (eg
sashiko) text in the > flow with blank lines between sections. When it
generates the final email it strips the | quoted text but retains the
blank lines it added. Thus the email ends up with random blank lines
inside the quoted text blocks.
Remove the blank lines along with the | blocks, being careful not to
disturb user text.
Assisted-by: LLM
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
src/b4/review/_review.py | 52 +++++++++++++++++++++++++++++++++++++---
src/tests/test_review.py | 26 ++++++++++++++++++++
2 files changed, 75 insertions(+), 3 deletions(-)
diff --git a/src/b4/review/_review.py b/src/b4/review/_review.py
index 96f3e5bbd54130..054be896053e87 100644
--- a/src/b4/review/_review.py
+++ b/src/b4/review/_review.py
@@ -3383,6 +3383,54 @@ def _sync_reply_trailers(reply_text: str, selected: List[str], identity: str) ->
return reply_text
+def _strip_external_reviewer_lines(lines: List[str]) -> List[str]:
+ """Remove read-only external-review blocks without leaving quote gaps.
+
+ Rendering puts bare blank separators around ``|``-prefixed external
+ comments. When both sides of a separator are quoted diff, it belongs to
+ that presentation block and must leave with the comment. A separator
+ after maintainer text is retained exactly, since it is part of their reply
+ layout rather than the external review's quote padding.
+ """
+ result: List[str] = []
+ index = 0
+ while index < len(lines):
+ if not lines[index].startswith('|'):
+ result.append(lines[index])
+ index += 1
+ continue
+
+ # A leading separator belongs to an external block only when it
+ # separates that block from quoted diff. Do not alter the same blank
+ # after the maintainer's own text.
+ previous = len(result) - 1
+ while previous >= 0 and not result[previous].strip():
+ previous -= 1
+ if previous >= 0 and result[previous].startswith('>'):
+ del result[previous + 1 :]
+
+ # Treat adjacent external blocks as one block. Their intervening
+ # blanks are presentation padding too.
+ while index < len(lines) and lines[index].startswith('|'):
+ index += 1
+ blank_end = index
+ while blank_end < len(lines) and not lines[blank_end].strip():
+ blank_end += 1
+ if blank_end < len(lines) and lines[blank_end].startswith('|'):
+ index = blank_end
+ continue
+ if blank_end < len(lines) and lines[blank_end].startswith('>'):
+ # The following quoted context is already separated by its
+ # quote prefix; do not leave the external block's gap behind.
+ index = blank_end
+ else:
+ result.extend(lines[index:blank_end])
+ index = blank_end
+ break
+
+ return result
+
+
def _trim_quoted_reply(buffer: str) -> str:
"""Prepare a hand-edited reply buffer for sending.
@@ -3395,9 +3443,7 @@ def _trim_quoted_reply(buffer: str) -> str:
maintainer left in place anywhere above their final comment is kept
exactly as written; nothing is collapsed, reordered, or relocated.
"""
- lines = [
- line for line in _strip_instruction_header(buffer) if not line.startswith('|')
- ]
+ lines = _strip_external_reviewer_lines(_strip_instruction_header(buffer))
# Drop the trailing quoted/blank run below the maintainer's last comment.
end = len(lines)
while end > 0 and (lines[end - 1].startswith('>') or not lines[end - 1].strip()):
diff --git a/src/tests/test_review.py b/src/tests/test_review.py
index eb7e36c52eeebb..a2c8703024674c 100644
--- a/src/tests/test_review.py
+++ b/src/tests/test_review.py
@@ -861,6 +861,32 @@ class TestTrimQuotedReply:
def test_empty_buffer(self) -> None:
assert review._trim_quoted_reply('') == ''
+ def test_removes_external_gaps_inside_quoted_diff(self) -> None:
+ buffer = (
+ '> first quoted line\n'
+ '\n'
+ '| sashiko.dev <sashiko@sashiko.dev>:\n'
+ '|\n'
+ '| An external finding.\n'
+ '|\n'
+ '| via: https://sashiko.dev/#/message/example\n'
+ '\n'
+ '> second quoted line\n'
+ '\n'
+ '| another reviewer <reviewer@example.com>:\n'
+ '|\n'
+ '| Another external finding.\n'
+ '\n'
+ '> third quoted line\n'
+ 'My maintainer comment.\n'
+ )
+ assert review._trim_quoted_reply(buffer) == (
+ '> first quoted line\n'
+ '> second quoted line\n'
+ '> third quoted line\n'
+ 'My maintainer comment.'
+ )
+
class TestParseReplyTrailers:
"""Tests for _parse_reply_trailers() — derived trailer display index."""
--
2.43.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH b4 v2 3/4] review: add >--cut-- inline marker to snip quoted reply context
2026-09-07 19:22 [PATCH b4 v2 0/4] Improve email quote trimming Jason Gunthorpe
2026-09-07 19:22 ` [PATCH b4 v2 1/4] review: trim trailing quoted when adding a tag Jason Gunthorpe
2026-09-07 19:22 ` [PATCH b4 v2 2/4] review: discard blank lines between | and > quotes when trimming Jason Gunthorpe
@ 2026-09-07 19:22 ` Jason Gunthorpe
2026-09-07 19:22 ` [PATCH b4 v2 4/4] review: emacs: highlighting and keystroke for >--cut-- Jason Gunthorpe
3 siblings, 0 replies; 5+ messages in thread
From: Jason Gunthorpe @ 2026-09-07 19:22 UTC (permalink / raw)
To: tools
When adding review comments to a message it is very inconvenient to
permanently lose the quotable text. Often I will make several passes over
a series and the nice b4 behavior of keeping everything in draft
encourages coming back and making revisions and further comments.
However, if the quoted text has been erased that isn't possible.
Instead of erasing in the editor provide a full line marker '>--cut--' for
b4 to automagically do the trimming similar to how it trims unused quote
at the end. Trailing text after user lines and till the marker is removed
similar to the trailing text.
This way the message can be sliced and minimally quoted appropriately with
no loss of quoted text. Everything can be selectively undone by the user.
Assisted-by: LLM
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
docs/maintainer/review.rst | 24 +++
docs/releases.rst | 9 +-
src/b4/review/_review.py | 82 ++++++++--
src/b4/review_tui/_review_app.py | 10 +-
src/tests/test_review.py | 257 +++++++++++++++++++++++++++++++
src/tests/test_tui_review.py | 69 +++++++++
6 files changed, 431 insertions(+), 20 deletions(-)
diff --git a/docs/maintainer/review.rst b/docs/maintainer/review.rst
index ca0632696b1b0b..7ac39322ea2b0c 100644
--- a/docs/maintainer/review.rst
+++ b/docs/maintainer/review.rst
@@ -584,6 +584,30 @@ cyan, external ``|`` comments visually bracketed, and your own comments
in the default foreground. Spell checking is limited to your own
comment lines.
+**Trimming quoted context (any editor)**
+
+No editor plugin is required to prune a run of quoted context. Put the
+following marker alone on a line after the quoted lines you want to discard::
+
+ >--cut--
+
+When the reply is sent, b4 removes the contiguous run of quoted lines above
+the marker, stopping at your last unquoted note, and replaces it with the
+same breadcrumb used by the Vim and Emacs trimming commands::
+
+ > [ ... 42 lines skipped ... ]
+
+Quoted context below the marker is left untouched. Only ``>``-quoted lines
+contribute to the count, so your own notes are always boundaries and can
+never be discarded. If you type the marker while the cursor is still in the
+quoted flow, ``> >--cut--`` is accepted as an equivalent spelling.
+
+Hand-typed markers and the Vim/Emacs commands produce identical output and
+coalesce with each other. If a plugin-generated breadcrumb is immediately
+above ``>--cut--``, its recorded count is absorbed into the new breadcrumb instead
+of leaving two markers. This works in both the main review reply editor and
+the follow-up quick-reply editor.
+
*Vim*
Copy or symlink the files into your Vim configuration::
diff --git a/docs/releases.rst b/docs/releases.rst
index 4bff1b05645728..2b16efb72d7ace 100644
--- a/docs/releases.rst
+++ b/docs/releases.rst
@@ -40,6 +40,14 @@ the full reference.
Several notable improvements to the ``b4 review`` workflow shipped this
cycle, building on the v0.15 foundation.
+**Inline snip markers in review replies**
+
+In any editor, put ``>--cut--`` alone on a line to discard the run of quoted
+context above it back to your last note. B4 replaces the run with the same
+``> [ ... NN lines skipped ... ]`` breadcrumb produced by the Vim and Emacs
+review helpers. The marker also works in follow-up quick replies, and existing
+breadcrumbs coalesce into its skipped-line count.
+
**Manual revision linking**
When automatic revision discovery fails — for example, a submitter sent
@@ -628,4 +636,3 @@ reviewing patches, testing, and contributing code:
- Rob Herring
- Tamir Duberstein
- Toke Høiland-Jørgensen
-
diff --git a/src/b4/review/_review.py b/src/b4/review/_review.py
index 054be896053e87..0674bced0d5640 100644
--- a/src/b4/review/_review.py
+++ b/src/b4/review/_review.py
@@ -2028,6 +2028,9 @@ def _render_quoted_diff_with_comments(
'# trailer (Reviewed-by:, etc.) you place inline. The only change made',
'# on send is to drop quoted diff left below your last comment; quoted',
'# context you keep above a comment is sent as-is. Trim freely.',
+ '# To discard a run of quoted context without deleting it by hand, put',
+ '# ">--cut--" alone on a line -- everything quoted above it back to your last',
+ '# note is dropped when you send.',
'#',
]
current_a_file = ''
@@ -2038,7 +2041,7 @@ def _render_quoted_diff_with_comments(
def _insert(key: Tuple[str, int]) -> None:
for text, attr, prov in comment_map.pop(key, []):
- if result:
+ if result and result[-1]:
result.append('')
if attr:
# External reviewer comment — render with | prefix
@@ -2122,10 +2125,10 @@ def _extract_editor_comments(
) -> List[Dict[str, Any]]:
"""Extract comments from the quoted-diff editor format.
- Strips the leading ``#`` instruction header and external reviewer
- comments (``|`` prefix), then delegates to
- :func:`_extract_comments_from_quoted_reply` which handles the
- ``> ``-quoted diff with unquoted comment format.
+ Strips the leading ``#`` instruction header, resolves snip markers, and
+ then removes external reviewer comments (``|`` prefix) before delegating
+ to :func:`_extract_comments_from_quoted_reply`, which handles the ``> ``-
+ quoted diff with unquoted comment format.
When *diff_text* is provided, runs :func:`_resolve_comment_positions`
to correct diff comment positions when the user has trimmed quoted
@@ -2133,12 +2136,9 @@ def _extract_editor_comments(
message), runs :func:`_resolve_message_positions` to re-anchor
``:message`` comments after editor re-wrapping of the quoted body.
"""
- filtered: List[str] = []
- for line in _strip_instruction_header(edited_text):
- # Strip external reviewer comment blocks (| prefix)
- if line.startswith('|'):
- continue
- filtered.append(line)
+ filtered = _strip_instruction_header(edited_text)
+ filtered = _apply_snip_markers(filtered)
+ filtered = _strip_external_reviewer_lines(filtered)
comments = _extract_comments_from_quoted_reply(
'\n'.join(filtered), capture_preamble=True
)
@@ -3285,6 +3285,8 @@ def _parse_reply_trailers(buffer: str) -> List[str]:
return [lt.as_string() for lt in found]
+_SNIP_MARKER_RE = re.compile(r'^\s*(?:> )?>--cut--\s*$')
+_SKIP_BREADCRUMB_RE = re.compile(r'^> \[ \.\.\. (\d+) lines skipped \.\.\. \]$')
_BARE_TRAILER_RE = re.compile(r'^\s*([\w-]+):\s')
# The trailer names the TUI trailer menu offers. The menu (and
@@ -3431,19 +3433,65 @@ def _strip_external_reviewer_lines(lines: List[str]) -> List[str]:
return result
+def _apply_snip_markers(lines: List[str]) -> List[str]:
+ """Resolve standalone ``>--cut--`` directives in a filtered reply buffer.
+
+ Each marker replaces the contiguous quoted/external-review/blank run
+ immediately above it with the same breadcrumb emitted by the Vim and
+ Emacs review helpers. The external-review blocks are snippable so their
+ surrounding separators can be removed without touching maintainer
+ spacing. Unquoted lines are hard boundaries, so maintainer comments can
+ never be consumed. Existing breadcrumbs contribute their recorded count,
+ allowing editor-generated and hand-typed trims to coalesce.
+ """
+ result: List[str] = []
+ for line in lines:
+ if not _SNIP_MARKER_RE.fullmatch(line):
+ result.append(line)
+ continue
+
+ boundary = len(result) - 1
+ while boundary >= 0 and (
+ not result[boundary].strip() or result[boundary].startswith(('>', '|'))
+ ):
+ boundary -= 1
+
+ snipped = result[boundary + 1 :]
+ skipped = 0
+ for quoted in snipped:
+ if not quoted.startswith('>'):
+ continue
+ breadcrumb = _SKIP_BREADCRUMB_RE.fullmatch(quoted)
+ skipped += int(breadcrumb.group(1)) if breadcrumb else 1
+
+ if skipped:
+ del result[boundary + 1 :]
+ first_snippable = next(
+ index
+ for index, snipped_line in enumerate(snipped)
+ if snipped_line.startswith(('>', '|'))
+ )
+ result.extend(snipped[:first_snippable])
+ result.append(f'> [ ... {skipped} lines skipped ... ]')
+
+ return result
+
+
def _trim_quoted_reply(buffer: str) -> str:
"""Prepare a hand-edited reply buffer for sending.
The maintainer's text is sent as written. The only changes are to strip
b4's own scaffolding — the leading ``#`` instruction header (see
:func:`_strip_instruction_header`) and ``| `` read-only external-reviewer
- lines — and to drop the run of quoted diff at the very
- bottom of the message that has no comment after it, the usual courtesy of
- trimming quoted material below your last reply. Quoted context the
- maintainer left in place anywhere above their final comment is kept
- exactly as written; nothing is collapsed, reordered, or relocated.
+ lines — resolve any standalone ``>--cut--`` snip markers — and drop the run
+ of quoted diff at the very bottom of the message that has no comment after
+ it, the usual courtesy of trimming quoted material below your last reply.
+ Other quoted context the maintainer left in place anywhere above their
+ final comment is kept exactly as written; nothing is collapsed, reordered,
+ or relocated.
"""
- lines = _strip_external_reviewer_lines(_strip_instruction_header(buffer))
+ lines = _apply_snip_markers(_strip_instruction_header(buffer))
+ lines = _strip_external_reviewer_lines(lines)
# Drop the trailing quoted/blank run below the maintainer's last comment.
end = len(lines)
while end > 0 and (lines[end - 1].startswith('>') or not lines[end - 1].strip()):
diff --git a/src/b4/review_tui/_review_app.py b/src/b4/review_tui/_review_app.py
index e1082e94c54f4a..d7a6bda1c3b4e5 100644
--- a/src/b4/review_tui/_review_app.py
+++ b/src/b4/review_tui/_review_app.py
@@ -1850,7 +1850,12 @@ class ReviewApp(LoreNodeShutdownMixin, CheckRunnerMixin, App[None]):
orig_author = entry.get('fromname', '') or entry.get('fromemail', '')
body = entry.get('body', '')
quoted = '\n'.join(f'> {line}' for line in body.splitlines())
- editor_text = f'On {orig_date}, {orig_author} wrote:\n{quoted}\n\n'
+ editor_text = (
+ '# Put ">--cut--" alone on a line to discard quoted context above it\n'
+ '# back to your last note; b4 resolves it when you send.\n'
+ '#\n'
+ f'On {orig_date}, {orig_author} wrote:\n{quoted}\n\n'
+ )
result = suspend_and_edit(
self, editor_text.encode(), 'reply.eml', topdir=self._topdir
@@ -1872,7 +1877,8 @@ class ReviewApp(LoreNodeShutdownMixin, CheckRunnerMixin, App[None]):
def _send_followup_reply(self, entry: Dict[str, Any], text: str) -> None:
"""Build and immediately send a quick reply to a follow-up message."""
- msg = entry['lmsg'].make_reply(text)
+ reply_text = b4.review._trim_quoted_reply(text)
+ msg = entry['lmsg'].make_reply(reply_text)
try:
with self.suspend():
smtp, fromaddr = b4.get_smtp(dryrun=self._email_dryrun)
diff --git a/src/tests/test_review.py b/src/tests/test_review.py
index a2c8703024674c..c8ff5136bc4595 100644
--- a/src/tests/test_review.py
+++ b/src/tests/test_review.py
@@ -136,6 +136,12 @@ class TestRenderQuotedDiffWithComments:
)
assert 'My comment' in result
assert '| Ext comment' in result
+ lines = result.splitlines()
+ own_index = lines.index('My comment')
+ external_index = lines.index('| Ext <ext@example.com>:')
+ # _insert() terminates the own comment with one blank line, which is
+ # already enough to separate it from the external review.
+ assert lines[own_index + 1 : external_index] == ['']
def test_cross_file_comments(self) -> None:
"""Comments in different files render correctly."""
@@ -888,6 +894,257 @@ class TestTrimQuotedReply:
)
+class TestSnipMarker:
+ """Tests for resolving hand-typed quoted-context snip markers."""
+
+ def test_discards_quote_run_above_and_keeps_quote_below(self) -> None:
+ lines = [
+ 'My first note.',
+ '> first quoted line',
+ '> second quoted line',
+ '',
+ '>--cut--',
+ '> quoted context below the marker',
+ 'My second note.',
+ ]
+ assert _review._apply_snip_markers(lines) == [
+ 'My first note.',
+ '> [ ... 2 lines skipped ... ]',
+ '> quoted context below the marker',
+ 'My second note.',
+ ]
+
+ def test_snip_removes_external_comment_separators(self) -> None:
+ buffer = (
+ 'My first note.\n'
+ '\n'
+ '> quoted context before the external review\n'
+ '\n'
+ '| sashiko.dev <sashiko@sashiko.dev>:\n'
+ '|\n'
+ '| An external finding.\n'
+ '|\n'
+ '| via: https://sashiko.dev/#/message/example\n'
+ '\n'
+ '> quoted context after the external review\n'
+ '>--cut--\n'
+ '> quoted context below the marker\n'
+ 'My second note.\n'
+ )
+ assert review._trim_quoted_reply(buffer) == (
+ 'My first note.\n'
+ '\n'
+ '> [ ... 2 lines skipped ... ]\n'
+ '> quoted context below the marker\n'
+ 'My second note.'
+ )
+
+ def test_snip_keeps_spacing_before_external_review(self) -> None:
+ buffer = (
+ 'My maintainer comment.\n'
+ '\n'
+ '| sashiko.dev <sashiko@sashiko.dev>:\n'
+ '|\n'
+ '| An external finding.\n'
+ '|\n'
+ '| via: https://sashiko.dev/#/message/example\n'
+ '\n'
+ '> quoted context after the external review\n'
+ '>--cut--\n'
+ '> quoted context below the marker\n'
+ 'My second note.\n'
+ )
+ assert review._trim_quoted_reply(buffer) == (
+ 'My maintainer comment.\n'
+ '\n'
+ '> [ ... 1 lines skipped ... ]\n'
+ '> quoted context below the marker\n'
+ 'My second note.'
+ )
+
+ def test_snip_keeps_maintainer_spacing_before_first_quote(self) -> None:
+ lines = [
+ 'My first paragraph.',
+ 'My second paragraph.',
+ '',
+ '> first quoted line',
+ '',
+ '> second quoted line',
+ '>--cut--',
+ '> quoted context below the marker',
+ 'My later note.',
+ ]
+ assert _review._apply_snip_markers(lines) == [
+ 'My first paragraph.',
+ 'My second paragraph.',
+ '',
+ '> [ ... 2 lines skipped ... ]',
+ '> quoted context below the marker',
+ 'My later note.',
+ ]
+
+ @pytest.mark.parametrize(
+ ('lines', 'expected'),
+ [
+ (['', '>--cut--', 'My note.'], ['', 'My note.']),
+ (
+ ['My note.', '', '>--cut--', '> quote below', 'Another note.'],
+ ['My note.', '', '> quote below', 'Another note.'],
+ ),
+ ],
+ )
+ def test_marker_with_nothing_to_discard(
+ self, lines: List[str], expected: List[str]
+ ) -> None:
+ assert _review._apply_snip_markers(lines) == expected
+
+ def test_coalesces_existing_breadcrumb_count(self) -> None:
+ lines = [
+ 'My note.',
+ '> one more quoted line',
+ '> [ ... 42 lines skipped ... ]',
+ '>--cut--',
+ '> kept below',
+ 'Later note.',
+ ]
+ assert _review._apply_snip_markers(lines) == [
+ 'My note.',
+ '> [ ... 43 lines skipped ... ]',
+ '> kept below',
+ 'Later note.',
+ ]
+
+ def test_multiple_markers_never_cross_comment_boundaries(self) -> None:
+ lines = [
+ 'First comment.',
+ '> first run',
+ '>--cut--',
+ '> kept after first marker',
+ 'Adopted reviewer comment.',
+ '> second run one',
+ '> second run two',
+ '> >--cut--',
+ '> kept after second marker',
+ 'Final comment.',
+ ]
+ assert _review._apply_snip_markers(lines) == [
+ 'First comment.',
+ '> [ ... 1 lines skipped ... ]',
+ '> kept after first marker',
+ 'Adopted reviewer comment.',
+ '> [ ... 2 lines skipped ... ]',
+ '> kept after second marker',
+ 'Final comment.',
+ ]
+
+ @pytest.mark.parametrize('marker', ['>--cut--', '> >--cut--'])
+ def test_accepted_spellings_resolve_identically(self, marker: str) -> None:
+ lines = ['My note.', '> one', '> two', marker, '> kept', 'Later note.']
+ assert _review._apply_snip_markers(lines) == [
+ 'My note.',
+ '> [ ... 2 lines skipped ... ]',
+ '> kept',
+ 'Later note.',
+ ]
+
+ def test_marker_substring_is_not_a_directive(self) -> None:
+ lines = ['My note.', '> quoted', 'Do not treat >--cut-- here as a marker.']
+ assert _review._apply_snip_markers(lines) == lines
+
+ def test_trim_composes_scaffolding_snip_and_trailing_quote(self) -> None:
+ buffer = (
+ '# instructions\n'
+ '| External reviewer note\n'
+ 'My first note.\n'
+ '> old context one\n'
+ '> old context two\n'
+ '>--cut--\n'
+ '> context kept below the marker\n'
+ 'My second note.\n'
+ '> trailing untouched quote\n'
+ )
+ assert review._trim_quoted_reply(buffer) == (
+ 'My first note.\n'
+ '> [ ... 2 lines skipped ... ]\n'
+ '> context kept below the marker\n'
+ 'My second note.'
+ )
+
+ def test_diff_comment_after_marker_resolves_against_real_diff(self) -> None:
+ edited = (
+ '# instructions\n'
+ 'On today, Author wrote:\n'
+ '> diff --git a/f.c b/f.c\n'
+ '> index 1111111..2222222 100644\n'
+ '> --- a/f.c\n'
+ '> +++ b/f.c\n'
+ '> @@ -1,2 +1,2 @@\n'
+ '> -old first\n'
+ '> +new first\n'
+ '> keep first\n'
+ '>--cut--\n'
+ '> @@ -10,2 +10,2 @@\n'
+ '> -old target\n'
+ '> +new target\n'
+ '\n'
+ 'Fix the target.\n'
+ '> keep target\n'
+ )
+ real_diff = (
+ 'diff --git a/f.c b/f.c\n'
+ 'index 1111111..2222222 100644\n'
+ '--- a/f.c\n'
+ '+++ b/f.c\n'
+ '@@ -1,2 +1,2 @@\n'
+ '-old first\n'
+ '+new first\n'
+ ' keep first\n'
+ '@@ -10,2 +10,2 @@\n'
+ '-old target\n'
+ '+new target\n'
+ ' keep target\n'
+ )
+ comments = review._extract_editor_comments(edited, diff_text=real_diff)
+ assert comments == [
+ {
+ 'path': 'b/f.c',
+ 'line': 10,
+ 'text': 'Fix the target.',
+ 'content': '+new target',
+ }
+ ]
+
+ def test_message_comment_after_marker_resolves_against_real_text(self) -> None:
+ edited = (
+ '# instructions\n'
+ 'On today, Author wrote:\n'
+ '> First body line.\n'
+ '> Second body line.\n'
+ '>--cut--\n'
+ '> Target body line.\n'
+ '\n'
+ 'Comment on the target.\n'
+ '> Last body line.\n'
+ )
+ message = (
+ 'Subject\n'
+ '\n'
+ 'First body line.\n'
+ 'Second body line.\n'
+ 'Target body line.\n'
+ 'Last body line.\n'
+ )
+ comments = review._extract_editor_comments(edited, message_text=message)
+ assert comments == [
+ {
+ 'path': review.COMMIT_MESSAGE_PATH,
+ 'line': 3,
+ 'text': 'Comment on the target.',
+ 'content': 'Target body line.',
+ }
+ ]
+
+
class TestParseReplyTrailers:
"""Tests for _parse_reply_trailers() — derived trailer display index."""
diff --git a/src/tests/test_tui_review.py b/src/tests/test_tui_review.py
index cd808ec8b7bf63..5124842b6c914b 100644
--- a/src/tests/test_tui_review.py
+++ b/src/tests/test_tui_review.py
@@ -9,6 +9,7 @@ Tests the shell-return reconciliation logic that detects and handles
cosmetic commit edits (e.g. reworded subjects via git rebase -i).
"""
+import email.message
import json
from typing import Any, Dict, List, Tuple
from unittest import mock
@@ -362,6 +363,74 @@ class TestReplyVerbatim:
)
+class TestFollowupSnipMarker:
+ """Quick follow-up replies expose and resolve the snip marker."""
+
+ def test_compose_includes_marker_instructions(self, gitdir: str) -> None:
+ branch, _shas = _create_review_branch_with_patches(
+ gitdir, 'followup-snip-instructions', ['patch 1']
+ )
+ app = ReviewApp(_build_session(gitdir, branch))
+ entry = {
+ 'date': 'Thu, 1 Jan 2026 12:00:00 +0000',
+ 'fromname': 'Reviewer',
+ 'fromemail': 'reviewer@example.com',
+ 'body': 'First line.\nSecond line.',
+ }
+
+ with mock.patch(
+ 'b4.review_tui._review_app.suspend_and_edit', return_value=None
+ ) as edit:
+ app._compose_followup_reply(entry)
+
+ editor_text = edit.call_args.args[1].decode()
+ assert editor_text.startswith('# Put ">--cut--" alone on a line')
+ assert '# back to your last note; b4 resolves it when you send.' in editor_text
+ assert '> First line.\n> Second line.' in editor_text
+
+ def test_send_resolves_marker_and_drops_trailing_quote(self, gitdir: str) -> None:
+ import contextlib
+
+ branch, _shas = _create_review_branch_with_patches(
+ gitdir, 'followup-snip-send', ['patch 1']
+ )
+ session = _build_session(gitdir, branch)
+ session['email_dryrun'] = True
+ app = ReviewApp(session)
+ lmsg = mock.Mock()
+ outgoing = email.message.EmailMessage()
+ lmsg.make_reply.return_value = outgoing
+ entry = {'lmsg': lmsg, 'fromemail': 'reviewer@example.com'}
+ buffer = (
+ '# Put ">--cut--" alone on a line to trim quoted context.\n'
+ 'On today, Reviewer wrote:\n'
+ '> old context one\n'
+ '> old context two\n'
+ '>--cut--\n'
+ '> context kept below the marker\n'
+ 'My reply.\n'
+ '> trailing untouched quote\n'
+ )
+
+ with (
+ mock.patch.object(
+ app, 'suspend', side_effect=lambda: contextlib.nullcontext()
+ ),
+ mock.patch.object(app, 'notify'),
+ mock.patch('b4.get_smtp', return_value=(None, 'me@example.com')),
+ mock.patch('b4.send_mail', return_value=0) as send_mail,
+ ):
+ app._send_followup_reply(entry, buffer)
+
+ lmsg.make_reply.assert_called_once_with(
+ 'On today, Reviewer wrote:\n'
+ '> [ ... 2 lines skipped ... ]\n'
+ '> context kept below the marker\n'
+ 'My reply.'
+ )
+ assert send_mail.call_args.args[1] == [outgoing]
+
+
class TestReconcileAfterShell:
"""Tests for _reconcile_after_shell tracking fixup."""
--
2.43.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH b4 v2 4/4] review: emacs: highlighting and keystroke for >--cut--
2026-09-07 19:22 [PATCH b4 v2 0/4] Improve email quote trimming Jason Gunthorpe
` (2 preceding siblings ...)
2026-09-07 19:22 ` [PATCH b4 v2 3/4] review: add >--cut-- inline marker to snip quoted reply context Jason Gunthorpe
@ 2026-09-07 19:22 ` Jason Gunthorpe
3 siblings, 0 replies; 5+ messages in thread
From: Jason Gunthorpe @ 2026-09-07 19:22 UTC (permalink / raw)
To: tools
Add a default highlighting in yellow for the cut marker and bind an
automatic insertion to C-c C-x.
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
misc/emacs/b4-review-mode.el | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/misc/emacs/b4-review-mode.el b/misc/emacs/b4-review-mode.el
index c2342ff29bdf4b..81b49079efd630 100644
--- a/misc/emacs/b4-review-mode.el
+++ b/misc/emacs/b4-review-mode.el
@@ -111,6 +111,11 @@ whole-line deletion worth a breadcrumb.")
"Face for instruction lines (# ...)."
:group 'b4-review)
+(defface b4-review-cut-marker
+ '((t :foreground "yellow" :weight bold))
+ "Face for a \">--cut--\" snip marker line."
+ :group 'b4-review)
+
(defface b4-review-quote-background
'((t :inherit default :extend t))
"Background face for prefixed lines (`> ' quotes, `| ' external comments,
@@ -125,6 +130,17 @@ on every line — without it, the fill stops at the last character."
"Background face for the reviewer's own, unprefixed comment lines."
:group 'b4-review)
+(defface b4-review-cut-background
+ '((t :inherit default :extend t))
+ "Background face for a \">--cut--\" snip marker line.
+Applied the same way as `b4-review-quote-background' /
+`b4-review-own-background' so the marker line can be set off with its own
+background, e.g.:
+
+ (custom-set-faces
+ \\='(b4-review-cut-background ((t :background \"dark red\"))))"
+ :group 'b4-review)
+
(defvar b4-review-font-lock-keywords
`(
;; Order matters — earlier rules take priority in Emacs font-lock
@@ -153,6 +169,10 @@ on every line — without it, the fill stops at the last character."
(2 'b4-review-diff-file)
(3 'b4-review-diff-file))
+ ;; Snip marker, quoted or bare — checked before the plain-quoted catch-all
+ ;; below so it isn't swallowed by that more general rule.
+ ("^\\(?:> \\)?>--cut--[ \t]*$" (0 'b4-review-cut-marker))
+
;; Plain quoted lines (catch-all for > lines not matched above)
("^\\(> \\)\\(.*\\)$"
(1 'b4-review-quote-prefix)
@@ -173,6 +193,9 @@ on every line — without it, the fill stops at the last character."
;; itself, and `:extend t' on these two faces (see their defface above)
;; is what makes the display fill the rest of the line's width with that
;; face's background instead of stopping at the last glyph.
+ ;; Checked ahead of the general quote-background rule below so its
+ ;; :background wins over `b4-review-quote-background' on the marker line.
+ ("^\\(?:> \\)?>--cut--[ \t]*\n?" (0 'b4-review-cut-background append))
("^>.*\n?" (0 'b4-review-quote-background append))
("^|.*\n?" (0 'b4-review-quote-background append))
("^#.*\n?" (0 'b4-review-quote-background append))
@@ -419,11 +442,20 @@ the comment text in place under the same diff line, ready to edit."
(when lines
(insert (mapconcat #'identity lines "\n") "\n")))))
+(defun b4-review-add-cut ()
+ "Insert a cut marker that will remove itself and everything
+before until the previous user comment. The current line is
+placed in the removed area before the cut marker."
+ (interactive)
+ (end-of-line)
+ (insert "\n>--cut--"))
+
(defvar b4-review-mode-map
(let ((map (make-sparse-keymap)))
(define-key map (kbd "C-c C-k") #'b4-review-delete-hunk)
(define-key map (kbd "C-c C-b") #'b4-review-delete-hunks-before)
(define-key map (kbd "C-c C-a") #'b4-review-adopt-comment)
+ (define-key map (kbd "C-c C-x") #'b4-review-add-cut)
map)
"Keymap for `b4-review-mode'.")
--
2.43.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-09-07 19:23 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-07 19:22 [PATCH b4 v2 0/4] Improve email quote trimming Jason Gunthorpe
2026-09-07 19:22 ` [PATCH b4 v2 1/4] review: trim trailing quoted when adding a tag Jason Gunthorpe
2026-09-07 19:22 ` [PATCH b4 v2 2/4] review: discard blank lines between | and > quotes when trimming Jason Gunthorpe
2026-09-07 19:22 ` [PATCH b4 v2 3/4] review: add >--cut-- inline marker to snip quoted reply context Jason Gunthorpe
2026-09-07 19:22 ` [PATCH b4 v2 4/4] review: emacs: highlighting and keystroke for >--cut-- Jason Gunthorpe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox