[PATCH RFC v2 20/25] review-tui: add a "Find older revisions" action
Christian Brauner <[email protected]>
| Newsgroups | org.kernel.linux.tools |
|---|---|
| Message-ID | <[email protected]> |
Offer discover_older_revisions() from the tracker's action menu, wherever manual revision linking is offered. The search runs in a lore worker; on success the list reloads, deferring to the DB mtime poll when a modal is up. The documentation also gains the "Link a revision" line the partial block never listed, although that action has been offered for partial series all along. Signed-off-by: Christian Brauner (Amutable) <[email protected]> --- docs/maintainer/review.rst | 9 ++++ src/b4/review_tui/_tracking_app.py | 96 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 105 insertions(+) diff --git a/docs/maintainer/review.rst b/docs/maintainer/review.rst index ca063269..5ddd9dd2 100644 --- a/docs/maintainer/review.rst +++ b/docs/maintainer/review.rst @@ -252,6 +252,8 @@ actions depend on the series status: * ``[s]`` **Snooze** — defer until a date, duration, or git tag * ``[U]`` **Upgrade** — switch to a newer revision (when available) * ``[l]`` **Link a revision** — manually associate a revision by message-id +* ``[o]`` **Find older revisions** — search lore for versions posted before + the series was tracked * ``[A]`` **Abandon** / ``[x]`` **Archive** **Partial** (some patches applied, remainder still in review): @@ -262,6 +264,9 @@ actions depend on the series status: * ``[w]`` **Mark as waiting** — waiting on a new revision * ``[s]`` **Snooze** — defer until later * ``[U]`` **Upgrade** — switch to a newer revision (when available) +* ``[l]`` **Link a revision** — manually associate a revision by message-id +* ``[o]`` **Find older revisions** — search lore for versions posted before + the series was tracked * ``[A]`` **Abandon** / ``[x]`` **Archive** **New / gone:** @@ -270,6 +275,8 @@ actions depend on the series status: * ``[U]`` **Upgrade** — switch to a newer revision (new only, when available) * ``[s]`` **Snooze** — defer until later (new only) * ``[l]`` **Link a revision** — manually associate a revision by message-id (new only) +* ``[o]`` **Find older revisions** — search lore for versions posted before + the series was tracked (new only) * ``[A]`` **Abandon** **Waiting:** @@ -277,6 +284,8 @@ actions depend on the series status: * ``[U]`` **Upgrade** — switch to the newer revision (when available) * ``[r]`` **Review** — return to reviewing * ``[l]`` **Link a revision** — manually associate a revision by message-id +* ``[o]`` **Find older revisions** — search lore for versions posted before + the series was tracked * ``[A]`` **Abandon** / ``[x]`` **Archive** **Snoozed:** diff --git a/src/b4/review_tui/_tracking_app.py b/src/b4/review_tui/_tracking_app.py index f0ad512a..999513ca 100644 --- a/src/b4/review_tui/_tracking_app.py +++ b/src/b4/review_tui/_tracking_app.py @@ -32,6 +32,7 @@ from typing import ( Union, ) +from rich.markup import escape from rich.text import Text as RichText from textual.app import App, ComposeResult from textual.binding import Binding @@ -45,6 +46,7 @@ import b4.mbox import b4.review import b4.review.tracking import b4.ty +import liblore from b4.review._review import NO_COVER_NOTE from b4.review_tui._common import ( QUIT_BINDINGS, @@ -65,6 +67,7 @@ from b4.review_tui._common import ( resolve_styles, run_lore_worker, suspend_and_edit, + worker_cancelled, ) from b4.review_tui._modals import ( QUEUE_BUSY, @@ -105,6 +108,7 @@ _ACTION_SHORTCUTS: Dict[str, str] = { 'unsnooze': 'u', 'upgrade': 'U', 'link': 'l', + 'discover': 'o', 'thank': 't', 'abandon': 'A', 'archive': 'x', @@ -514,6 +518,32 @@ def _resolve_worktree_take_conflict( return True +def _discovery_error_notice(error: str) -> str: + """Message for a discovery run that came back with an error. + + *error* is a lore exception's text, so it is escaped rather than + trusted: notify() renders Rich markup, and an unescaped bracket in it + is either swallowed as a style tag -- taking the diagnostic the message + exists to carry -- or, when the text holds a '[/...]' path fragment, + raises MarkupError inside the toast render. See _conflicts_notice. + """ + return f'Older-revision search failed: {escape(error)}' + + +def _conflicts_notice(conflicts: List[int]) -> str: + """Message for versions a discovery run found another series tracking. + + The ``[l]`` is escaped because notify() renders Rich markup: unescaped + it is parsed as a style tag, and the one key the message exists to name + is dropped from what the maintainer actually reads. + """ + clist = ', '.join(f'v{r}' for r in conflicts) + return ( + f'{clist} already tracked as a separate series' + f' — use {escape("[l]")} to link and absorb' + ) + + def _format_snooze_until(value: str) -> str: """Format a snoozed_until value for display. @@ -1089,6 +1119,44 @@ class TrackingApp(LoreNodeShutdownMixin, CheckRunnerMixin, App[Optional[str]]): elif event.state == WorkerState.ERROR: self.notify('Could not fetch series', severity='error') return + if event.worker.name == '_discover_older': + if event.state == WorkerState.SUCCESS: + result = event.worker.result or {} + error = result.get('error') + found = result.get('found', 0) + conflicts = result.get('conflicts') or [] + if error == 'offline': + # Not a failure, just nothing to search with. + self.notify('Offline — cannot search for older revisions') + elif error: + self.notify(_discovery_error_notice(str(error)), severity='error') + elif found: + # found is len(revisions), so the list is never empty here. + rlist = ', '.join(f'v{r}' for r in result.get('revisions') or []) + self.notify(f'Found and added: {rlist}') + # Reload so the new revisions show up right away; if a + # modal is up, the DB mtime poll picks it up instead. + if len(self.app.screen_stack) == 1: + if self._selected_series: + self._focus_change_id = self._selected_series.get( + 'change_id' + ) + self._invalidate_caches() + self._load_series() + elif not conflicts: + # Only when there is nothing else to say. A run that + # found versions and skipped every one of them as a + # conflict reports 0 found, and saying "none found" + # ahead of the list of them contradicts itself. + self.notify('No older revisions found') + if conflicts: + self.notify(_conflicts_notice(conflicts), severity='warning') + elif event.state == WorkerState.ERROR: + if isinstance(event.worker.error, liblore.OperationCancelledError): + # Shutting down or navigating away, not a failure. + return + self.notify('Older-revision search failed', severity='error') + return if event.worker.name != '_startup_rescan': return if event.state == WorkerState.SUCCESS: @@ -1516,6 +1584,7 @@ class TrackingApp(LoreNodeShutdownMixin, CheckRunnerMixin, App[Optional[str]]): actions.append(('upgrade', 'Upgrade to newer revision')) if status == 'new': actions.append(('link', 'Manually link a revision')) + actions.append(('discover', 'Find older revisions')) actions.append(('abandon', 'Abandon series')) if status == 'new': actions.append(('waiting', 'Mark as waiting on new revision')) @@ -1548,6 +1617,7 @@ class TrackingApp(LoreNodeShutdownMixin, CheckRunnerMixin, App[Optional[str]]): actions.append(('thank', 'Send thank-you')) if status in ('reviewing', 'replied', 'partial', 'waiting'): actions.append(('link', 'Manually link a revision')) + actions.append(('discover', 'Find older revisions')) # 'Return to reviewing' sits just above the abandon/archive block # rather than at the top of the menu. if status in ('accepted', 'partial', 'thanked'): @@ -1572,6 +1642,7 @@ class TrackingApp(LoreNodeShutdownMixin, CheckRunnerMixin, App[Optional[str]]): 'thank': self.action_thank, 'upgrade': self.action_update_revision, 'link': self.action_link_revision, + 'discover': self.action_discover_older, 'archive': self.action_archive, 'waiting': self.action_waiting, 'snooze': self.action_snooze, @@ -3957,6 +4028,31 @@ class TrackingApp(LoreNodeShutdownMixin, CheckRunnerMixin, App[Optional[str]]): ), ) + def action_discover_older(self) -> None: + """Search lore for older revisions of the selected series.""" + if not self._selected_series or not self._identifier: + return + series = dict(self._selected_series) + config = b4.get_main_config() + linkmask = str(config.get('linkmask', '')) + topdir = b4.git_get_toplevel() + identifier = self._identifier + self.notify('Searching lore for older revisions…') + + def _discover() -> Dict[str, Any]: + # The search machinery logs to the console; keep it from + # scribbling over the TUI. + with _quiet_worker(): + return b4.review.tracking.discover_older_revisions( + identifier, + series, + linkmask, + topdir=topdir, + cancel_cb=worker_cancelled, + ) + + run_lore_worker(self, _discover, name='_discover_older') + def action_link_revision(self) -> None: """Manually link another revision to the selected series by msgid. -- 2.53.0