diff --git a/CHANGELOG.md b/CHANGELOG.md index 7938cad..3a2be6e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,11 @@ All notable changes to this project are documented here. Versions follow way, grey none, `—` nothing to sign off. Folders count what is underneath. - **Right-click a row** to show that file in Explorer, or copy its full path. - **Syntax colouring** in the diff panes for C/H and ARXML/XML. +- **Find in the open file** (`Ctrl+F`) — steps through every line that names + what you typed, on either side, and keeps the search when you move to + another file. +- **`Hide identical`** leaves only the files with a difference in the tree. + Verdicts, counts and the exported report are unchanged. ### Changed @@ -47,9 +52,15 @@ All notable changes to this project are documented here. Versions follow `Release notes` and `About`; `Export report` sits next to `Review mode`; per-file navigation moved into the diff header. Two-line landing screen, a `Ready` status chip, and the CURRENT folder's name as the window title. +- **A finished scan opens on the first change**, instead of on an empty pane. +- **`F7` / `F8` walk the whole compare.** At the end of a file they carry on + into the next one with something to review, and round again at the end. ### Fixed +- A report that cannot be written — missing folder, or the file open in + another program — now says so and exits `2` (compare incomplete) instead of + printing a Python error and exiting like an ordinary run with changes. - Added and deleted files now show a minimap, so they scroll like any other. - Quick-changes rows open on the object they name — an A2L characteristic, a port, an RTE access point — instead of on the file's first change. diff --git a/CLAUDE.md b/CLAUDE.md index 6e3d120..b2376fe 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -151,7 +151,7 @@ The exit code is a contract with somebody's pipeline. Do not change it: |---|---| | 0 | No real change | | 1 | Real changes found (the CI gate) | -| 2 | Compare INCOMPLETE — a path could not be listed, read or compared | +| 2 | Compare INCOMPLETE — a path could not be listed, read or compared, or the report could not be written (no record == not a clean run) | `build.ps1` produces one `dist\compare-tool.exe` carrying the CLI and the viewer. It is a **console** build on purpose: a terminal run must keep stdout diff --git a/README.md b/README.md index 6a328d9..5d6cd7c 100644 --- a/README.md +++ b/README.md @@ -69,10 +69,15 @@ Exit codes: |---|---| | `0` | No real changes | | `1` | Real changes found (useful as a CI gate) | -| `2` | **Compare INCOMPLETE** — some path could not be listed, read or compared (permissions, file locked by another process, long paths, …) | +| `2` | **Compare INCOMPLETE** — some path could not be listed, read or compared (permissions, file locked by another process, long paths, …), or the report could not be written | Exit `2` always shows: `!!` in the terminal, a red banner in the report. `--exit-zero` does not suppress it. +A report path that cannot be written (missing folder, file open in a browser, +read-only) is exit `2` with a one-line reason — never a traceback, and never +exit `1`, which a pipeline reads as the ordinary "real changes found". What the +scan did find is still printed. + ## Command line | Flag | Meaning | @@ -100,6 +105,13 @@ python -m compare_tool --qt # or start loaded Two ways in: `Open folders…` for two folders you name yourself, and `Git compare…` for **one** folder in a git checkout — it lists the commits that touched that folder, checks the one you pick out to a temp folder (read-only — your working copy is never touched), and compares as usual. +Reading a scan: + +- The scan **opens on the first change** — the pane is never empty next to a tree full of results. +- `F8` / `F7` step through the changes in the open file and then **carry on into the next (previous) file** with something to review, wrapping at the end. `Ctrl+Home` / `Ctrl+End` stay inside the file. +- `Ctrl+F` **finds text in the open file** (either side, `F3` / `Shift+F3` to step, `Esc` to close). The query survives moving to another file, so an identifier can be chased across the compare. +- `Hide identical` leaves only the files with a difference in the tree. It is a view: verdicts, counts and the exported report are untouched. + `Review mode` adds the note box and a `Review` column in the tree — green when every change in a row is signed off, amber part way, grey when none is. Sign off one change (`Ctrl+R`) or a whole file (`Ctrl+Shift+R`); the notes travel into the exported report. Full walkthrough is built into the app — `Help` → `User guide` (`F1`), which works offline. Standalone `.exe` (no Python needed): see [Single-file build](#single-file-build). diff --git a/compare_tool/main.py b/compare_tool/main.py index 786d807..abd6f94 100644 --- a/compare_tool/main.py +++ b/compare_tool/main.py @@ -16,6 +16,32 @@ summarize_rte, summarize_swcs) +class ReportWriteError(Exception): + """The report path could not be written, so this run left no record. + + Carries the scan it could not write (``results`` / ``counts``, None when + the failure came before the scan) so the caller can still print what was + found instead of throwing the whole run away. + """ + + def __init__(self, message, results=None, counts=None): + super().__init__(message) + self.results = results + self.counts = counts + + +def _write_hint(out, err): + """Why a report path could not be written, in the reviewer's terms rather + than as a traceback: the two everyday causes are a folder that does not + exist and a report still open in a browser holding the file.""" + reason = err.strerror or str(err) + if not out.parent.is_dir(): + reason += ' -- the folder {} does not exist'.format(out.parent) + elif isinstance(err, PermissionError): + reason += ' -- the file may be open in a browser or editor, or read-only' + return '{}: {}'.format(out, reason) + + def default_report_name(arxml_only): return 'arxml_update.html' if arxml_only else 'compare_report.html' @@ -23,12 +49,17 @@ def default_report_name(arxml_only): def run_compare(old_root, new_root, out, arxml_only=False, exclude=(), progress=None, reviews=None): """Scan two trees and write the HTML report. - Returns (results, counts).""" + Returns (results, counts). Raises :class:`ReportWriteError` when the report + could not be written -- a run whose record does not exist is not a run that + may report success.""" out = Path(out) # delete a leftover report from an earlier run BEFORE scanning: if this # run dies, a stale report must not pass for this run's result - if out.exists(): - out.unlink() + try: + if out.exists(): + out.unlink() + except OSError as e: + raise ReportWriteError(_write_hint(out, e)) include = tuple('*' + ext for ext, rs in RULES.items() if rs in ('arxml', 'a2l')) if arxml_only else () results = scan(old_root, new_root, progress=progress, exclude=exclude, @@ -40,7 +71,10 @@ def run_compare(old_root, new_root, out, arxml_only=False, exclude=(), page = build_arxml_report(results, old_root, new_root) else: page = build_report(results, old_root, new_root, reviews) - out.write_text(page, encoding='utf-8') + try: + out.write_text(page, encoding='utf-8') + except OSError as e: + raise ReportWriteError(_write_hint(out, e), results, counts) return results, counts @@ -238,9 +272,23 @@ def progress(done, total, rel): if done % 50 == 0 or done == total: print(' {}/{} {}'.format(done, total, rel)) - results, counts = run_compare(old_root, new_root, out, args.arxml_only, - exclude=args.exclude, progress=progress, - reviews=reviews) + try: + results, counts = run_compare(old_root, new_root, out, args.arxml_only, + exclude=args.exclude, progress=progress, + reviews=reviews) + except ReportWriteError as e: + # what WAS scanned still goes to the terminal -- the compare itself may + # have been fine, it is only the record that is missing + if e.results is not None: + for line in summary_lines(e.results, e.counts): + print(line) + print('!! REPORT NOT WRITTEN -- {}'.format(e), file=sys.stderr) + print('!! This run left no record: treat it as INCOMPLETE.', + file=sys.stderr) + # exit 2, never 1: 1 is the gate's "real changes found", a perfectly + # normal outcome, and a run that produced no report must not be + # indistinguishable from it (--exit-zero cannot mask this either) + return 2 for line in summary_lines(results, counts): print(line) diff --git a/compare_tool/qtviewer/app.py b/compare_tool/qtviewer/app.py index 6b4f0f0..da350f6 100644 --- a/compare_tool/qtviewer/app.py +++ b/compare_tool/qtviewer/app.py @@ -104,6 +104,10 @@ def __init__(self, old=None, new=None, exclude=(), arxml_only=False): self._units = {} # rel -> reviewable units, read once per scan self._git_temp = None # where commits are checked out, this session self._old_label = None # what OLD is, when its folder does not say + # one scan, one automatic jump to the first change. Flipping a compare + # rule re-judges the same scan and must NOT drag the reviewer off the + # file they are reading. + self._autoselect = False self.setWindowTitle('AUTOSAR CodeGen Compare — viewer') self.setWindowIcon(app_icon()) @@ -160,12 +164,24 @@ def __init__(self, old=None, new=None, exclude=(), arxml_only=False): 'timestamps, renames, whitespace): each such file is then reported ' 'as Identical or Modified.') self.cb_unimportant.toggled.connect(self._apply_rules) + # a display filter, NOT a compare rule: it removes rows from the tree + # without touching a verdict, which is why it sits apart from the two + # above. A regenerated tree is mostly untouched files, and scrolling + # past hundreds of '=' rows to reach five changed ones is its own way + # of hiding them. + self.cb_hide_identical = QCheckBox('Hide identical') + self.cb_hide_identical.setToolTip( + 'Leave only the files with a difference in the tree. Nothing is ' + 're-judged: verdicts, counts and the exported report are unchanged. ' + 'Files folded to Identical by the two boxes on the left go too.') + self.cb_hide_identical.toggled.connect(self._refresh_tree_keep_selection) rules = QHBoxLayout() rules.setContentsMargins(0, 0, 0, 0) rules.addWidget(QLabel('Report:')) rules.addWidget(self.cb_comment) rules.addWidget(self.cb_unimportant) rules.addStretch(1) + rules.addWidget(self.cb_hide_identical) tree_box = QWidget() lv = QVBoxLayout(tree_box) @@ -272,18 +288,21 @@ def _make_actions(self): 'commits — no second folder to choose') self.act_git.triggered.connect(self._pick_commit) + # First/Last stay inside the open file -- they mean "this file's ends". + # Previous/Next run off them into the next file with something to + # review, so a whole compare can be walked on F7/F8 alone. nav = (('act_first', 'nav-first-change', 'First change', 'Ctrl+Home', - self.diff.first_change), + self.diff.first_change, 'in this file'), ('act_prev', 'nav-prev-change', 'Previous change', 'F7', - self.diff.prev_change), + self._prev_change, 'crosses into the previous changed file'), ('act_next', 'nav-next-change', 'Next change', 'F8', - self.diff.next_change), + self._next_change, 'crosses into the next changed file'), ('act_last', 'nav-last-change', 'Last change', 'Ctrl+End', - self.diff.last_change)) - for attr, glyph, text, key, slot in nav: + self.diff.last_change, 'in this file')) + for attr, glyph, text, key, slot, scope in nav: act = QAction(icon(glyph), text, self) act.setShortcut(key) - act.setToolTip('{} ({}) — noise is skipped'.format(text, key)) + act.setToolTip('{} ({}) — noise is skipped, {}'.format(text, key, scope)) act.triggered.connect(slot) setattr(self, attr, act) # these four live in the diff pane's own header, beside the file name @@ -796,6 +815,7 @@ def _on_progress(self, done, total, rel): def _on_done(self, results): self._raw_results = results + self._autoselect = True # consumed by _apply_rules, below # the rollup reports the scan itself, never the folded view: a hidden # category must not make the model look untouched self.summary.set_results(results) @@ -823,6 +843,9 @@ def _apply_rules(self): self.diff.set_fold_modes([self._FOLD_MODE[f] for f in fold]) self._refresh_tree() self._reselect(keep) # keep the reviewer on the file they were reading + if self._autoselect: + self._autoselect = False + self._select_first_change() counts = summarize(self.results) self.counts_label.setText( '{real-change} modified · {comment-only} comment-only · ' @@ -907,8 +930,17 @@ def _refresh_tree(self): self.tree.clear() if not self.results: return - self._fill_tree(filter_nodes(build_nodes(self.results), - text=self.filter_edit.text())) + self._fill_tree(filter_nodes( + build_nodes(self.results), text=self.filter_edit.text(), + hide_identical=self.cb_hide_identical.isChecked())) + + def _refresh_tree_keep_selection(self): + """Rebuild the tree and put the reviewer back on the file they were + reading. Toggling a display filter is not a reason to lose the diff on + screen -- unless that very file is what the filter just hid.""" + keep = self._selected_rel() + self._refresh_tree() + self._reselect(keep) def _fill_tree(self, nodes): def add(parent, node, prefix): @@ -1039,6 +1071,80 @@ def _selected_rel(self): items = self.tree.selectedItems() return items[0].data(0, REL_ROLE) if items else None + # --- walking the files, not just the changes inside one --- + # + # verdicts a review pass has to look at: everything that is not noise. A + # finished scan opens on the first of these, and F7/F8 step between them + # once the current file runs out of changes. + _NAV_STATUS = ('error', 'real-change', 'added', 'deleted') + + def _tree_rels(self): + """Every file row, in the order the tree shows it. + + Read from the TREE and not from the results dict on purpose: rows the + path filter or `Hide identical` took off screen must not be what F8 + jumps to, and the tree is the only thing that knows what is visible. + """ + out = [] + + def walk(item): + rel = item.data(0, REL_ROLE) + if rel is not None: + out.append(rel) + for i in range(item.childCount()): + walk(item.child(i)) + + for i in range(self.tree.topLevelItemCount()): + walk(self.tree.topLevelItem(i)) + return out + + def _is_nav(self, rel): + r = self.results.get(rel) + return bool(r) and r['status'] in self._NAV_STATUS + + def _select_first_change(self): + """Open the first file a reviewer would have to read. Called once per + scan: landing on an empty pane next to a tree full of results makes the + reviewer's first act a hunt for where the changes are, which the tool + already knows.""" + rel = next((r for r in self._tree_rels() if self._is_nav(r)), None) + if rel: + self._reselect(rel) + + def _step_file(self, delta, to_last=False): + """Move to the next (delta +1) or previous (-1) file with something to + review, wrapping round the whole compare. `to_last` parks on that + file's LAST change, so stepping backwards continues where the eye is.""" + order = self._tree_rels() + nav = [r for r in order if self._is_nav(r)] + if not nav: + return + cur = self._selected_rel() + nxt = None + if cur in order: + i = order.index(cur) + after = order[i + 1:] if delta > 0 else list(reversed(order[:i])) + nxt = next((r for r in after if self._is_nav(r)), None) + wrapped = nxt is None + if wrapped: + nxt = nav[0] if delta > 0 else nav[-1] + self._reselect(nxt) # loads the file and parks on its first change + if to_last: + self.diff.last_change() + self.statusBar().showMessage( + '{}{}'.format('Wrapped to ' if wrapped else '', nxt), 4000) + + def _next_change(self): + """F8: the next change in this file, or the first change of the next + file with any. One key walks the whole compare instead of dead-ending + at the bottom of whichever file happens to be open.""" + if not self.diff.next_change(): + self._step_file(1) + + def _prev_change(self): + if not self.diff.prev_change(): + self._step_file(-1, to_last=True) + def _jump_to_name(self, rel, key): """Open `rel` from the quick-changes panel, on the hunk about `key`. @@ -1112,6 +1218,12 @@ def _on_select(self): text-align:center; color:#d0d0d0; } QProgressBar::chunk { background:#4F46E5; border-radius:5px; } QCheckBox { spacing:6px; } +/* find strip: a band of its own between the header and the code, so it reads + as a tool over the diff rather than as part of the file being read */ +QWidget#findbar { background:#212226; border-top:1px solid #34363c; + border-bottom:1px solid #34363c; } +QWidget#findbar QToolButton { color:#9aa1ad; padding:2px 6px; border-radius:4px; } +QWidget#findbar QToolButton:hover { background:#34363c; color:#e8e8e8; } """ diff --git a/compare_tool/qtviewer/dialogs.py b/compare_tool/qtviewer/dialogs.py index 78850d4..0053c81 100644 --- a/compare_tool/qtviewer/dialogs.py +++ b/compare_tool/qtviewer/dialogs.py @@ -36,9 +36,11 @@ ## 3. Read the tree -- Every file always listed -- a verdict never removes a row +- Every file listed -- a verdict never removes a row on its own - A folder shows its heaviest child verdict - Box above the tree filters by path +- `Hide identical` leaves only the files with a difference. It is a view, not a + rule: verdicts, counts and the exported report do not change - Right-click a row: show it in Explorer, or copy its full path | Mark | Verdict | Meaning | @@ -70,19 +72,29 @@ | dim red / green | noise: comment, UUID, rename, whitespace | | blue | moved block | -## 6. Navigate and export +## 6. Navigate, find and export +- The scan opens on the first change it found -- no hunting for it - Header shows `change 3 of 7` +- `F8` past the last change of a file goes on to the **next file** with + something to review; `F7` goes back the same way. One key walks the whole + compare, and it wraps round at the end +- `Ctrl+Home` / `Ctrl+End` stay inside the open file +- `Ctrl+F` finds text in the open file -- either side counts, the match is + marked amber, and the search is kept when you move to another file - Export writes the full HTML report -- folded categories still included - **Quick changes** panel: AUTOSAR / A2L rollup, click a row to jump straight to the line that names that object | Shortcut | Action | |---|---| -| `Ctrl+Home` | First change | -| `F7` | Previous change | -| `F8` | Next change | -| `Ctrl+End` | Last change | +| `Ctrl+Home` | First change (this file) | +| `F7` | Previous change, crossing into the previous changed file | +| `F8` | Next change, crossing into the next changed file | +| `Ctrl+End` | Last change (this file) | +| `Ctrl+F` | Find in this file | +| `F3` / `Shift+F3` | Next / previous match | +| `Esc` | Close the find bar | | `Ctrl+R` | Mark this change reviewed | | `Ctrl+Shift+R` | Mark the whole file reviewed | | `Ctrl+E` | Export report | @@ -112,6 +124,8 @@ ## Fail-safe - Unreadable or uncompared path -> red banner, `‼` mark, exit code `2` +- A report that could not be written is exit `2` too: a run with no record must + not look like a clean one - Never silently skipped """ diff --git a/compare_tool/qtviewer/diffpane.py b/compare_tool/qtviewer/diffpane.py index 38a05bf..7782a18 100644 --- a/compare_tool/qtviewer/diffpane.py +++ b/compare_tool/qtviewer/diffpane.py @@ -17,11 +17,13 @@ from pathlib import Path -from PySide6.QtCore import QRect, QSize, Qt, Signal -from PySide6.QtGui import (QColor, QFont, QPainter, QTextBlockFormat, - QTextCharFormat, QTextCursor, QTextFormat) -from PySide6.QtWidgets import (QHBoxLayout, QLabel, QPlainTextEdit, QSplitter, - QStackedWidget, QTextEdit, QVBoxLayout, QWidget) +from PySide6.QtCore import QEvent, QRect, QSize, Qt, Signal +from PySide6.QtGui import (QColor, QFont, QKeySequence, QPainter, QShortcut, + QTextBlockFormat, QTextCharFormat, QTextCursor, + QTextFormat) +from PySide6.QtWidgets import (QHBoxLayout, QLabel, QLineEdit, QPlainTextEdit, + QSplitter, QStackedWidget, QTextEdit, + QToolButton, QVBoxLayout, QWidget) from .. import review from ..scanner import looks_binary, read_text @@ -102,6 +104,13 @@ def _semantic_summary(result): # F7/F8 are visibly doing something even when the file fits on screen and # there is nothing to scroll _CUR_BG = QColor(255, 255, 255, 34) +# the find hits. Amber on purpose: red, green and blue already mean removed, +# added and moved, so a fourth hue is the only way a search result can be told +# apart from a verdict about the code. Every occurrence is marked, the one the +# counter is pointing at brighter -- "3 of 8" is only useful if the other seven +# are visible too. +_FIND_BG = QColor('#5a4715') +_FIND_CUR_BG = QColor('#8f7220') # OLD/NEW pane-banner accents: one source, used for both the tag text and the # underline so the two can never drift apart _OLD_ACCENT = '#c98b8b' @@ -298,6 +307,9 @@ def __init__(self): bl.setSpacing(0) bl.addWidget(self._split, 1) bl.addWidget(self.minimap) + self._find_bar = self._build_find_bar() + self._find_bar.setVisible(False) + diff_page = QWidget() dl = QVBoxLayout(diff_page) dl.setContentsMargins(0, 0, 0, 0) @@ -308,6 +320,7 @@ def __init__(self): # being pushed to the bottom by an oversized header gap dl.addLayout(head_row) dl.addWidget(self._sem) + dl.addWidget(self._find_bar) dl.addWidget(body, 1) self.addWidget(msg_page) # index 0 @@ -328,7 +341,23 @@ def __init__(self): self._head_base = '' # header without the "change k of N" suffix self._pos_text = '' # "change k of N", folded into the header text self._syncing = False + # which editor scrolling is driven from. The old pane normally carries + # both (the scrollbar mirror follows it), but a whole added/deleted file + # puts its text in ONE pane and leaves the other empty -- driving from + # an empty document scrolls nothing at all. + self._drive = self.old_edit + self._hits = [] # rows matching the find box, in file order + self._hit_idx = -1 + # the two extraSelections layers, per editor (old, new): see + # _paint_selections + self._sel_rows = [[], []] + self._sel_match = [[], []] self._link_scrolls() + # Ctrl+F is where every editor puts find; the pane owns the shortcut so + # it works wherever the focus sits inside the diff + QShortcut(QKeySequence.Find, self).activated.connect(self.open_find) + QShortcut(QKeySequence(Qt.Key_F3), self).activated.connect(self.find_next) + QShortcut(QKeySequence('Shift+F3'), self).activated.connect(self.find_prev) @staticmethod def _pane_banner(accent): @@ -351,6 +380,175 @@ def _pane(banner, editor): lay.addWidget(editor, 1) return w + # --- find in this file --- + + def _build_find_bar(self): + """One-line find strip under the header, hidden until Ctrl+F. + + It searches the ROWS, not the two documents: a row is one aligned pair, + so a hit is reported once whichever side carries it, and the jump can + reuse the same reveal-and-highlight the change navigation uses. Both + panes then land on the same line, which is the whole point of a + side-by-side view. + """ + bar = QWidget() + bar.setObjectName('findbar') + lay = QHBoxLayout(bar) + lay.setContentsMargins(10, 4, 6, 4) + lay.setSpacing(6) + self._find_edit = QLineEdit() + self._find_edit.setPlaceholderText('Find in this file… (Enter next, ' + 'Shift+Enter previous, Esc close)') + self._find_edit.setClearButtonEnabled(True) + self._find_edit.textChanged.connect(self._find_changed) + self._find_edit.returnPressed.connect(self.find_next) + self._find_edit.installEventFilter(self) + self._find_count = QLabel('') + self._find_count.setStyleSheet('color:#9aa1ad; font-size:12px;') + close = QToolButton() + close.setText('✕') + close.setToolTip('Close the find bar (Esc)') + close.setAutoRaise(True) + close.clicked.connect(self.close_find) + lay.addWidget(self._find_edit, 1) + lay.addWidget(self._find_count) + lay.addWidget(close) + return bar + + def eventFilter(self, obj, event): + # Esc closes, Shift+Enter steps back: QLineEdit has no signal for + # either, and both are what every find box in every editor does + if obj is self._find_edit and event.type() == QEvent.KeyPress: + if event.key() == Qt.Key_Escape: + self.close_find() + return True + if (event.key() in (Qt.Key_Return, Qt.Key_Enter) + and event.modifiers() & Qt.ShiftModifier): + self.find_prev() + return True + return super().eventFilter(obj, event) + + def open_find(self): + """Show the find bar and take the focus, selecting what is there so a + second Ctrl+F starts a new search instead of appending to the old one.""" + if self.currentIndex() != 1: + return # nothing to search: landing screen or a message page + self._find_bar.setVisible(True) + self._find_edit.setFocus() + self._find_edit.selectAll() + + def close_find(self): + self._find_bar.setVisible(False) + self._hits = [] + self._hit_idx = -1 + self._mark_matches() # a closed find bar leaves nothing lit behind it + # the focus goes back to the diff, or the next keystroke would vanish + # into a hidden box + self._drive.setFocus() + + def _refresh_find(self): + """Re-run the current query against the file just loaded. + + The query is KEPT across files on purpose -- "where else does this + identifier change" is the reason to search a codegen diff at all, and + retyping it per file is the whole cost of asking. The pane does not + jump to a hit by itself, though: opening a file parks on its first + change, and a search from a previous file must not quietly override + that. Enter or F3 moves. + """ + self._hits = [] + self._hit_idx = -1 + if not self._find_bar.isVisible(): + self._mark_matches() + return + text = self._find_edit.text() + if not text.strip(): + self._find_count.setText('') + self._mark_matches() + return + self._hits = self.find_matches(text) + self._find_count.setText('{} match{}'.format(len(self._hits), + '' if len(self._hits) == 1 else 'es') + if self._hits else 'no match') + self._mark_matches() + + def _find_changed(self, text): + self._hits = self.find_matches(text) + self._hit_idx = -1 + if not text.strip(): + self._find_count.setText('') + self._mark_matches() # every path repaints: see _mark_matches + return + if not self._hits: + self._find_count.setText('no match') + self._mark_matches() + return + self.find_next() + + def find_matches(self, text): + """Row indices containing `text` on either side, case-insensitive. + + Public so the behaviour can be tested without driving the widget: the + rows are the model, the bar is only a way to walk them. + """ + text = text.strip().lower() + if not text: + return [] + return [i for i, r in enumerate(self.rows) + if text in (r.old_txt or '').lower() + or text in (r.new_txt or '').lower()] + + def _goto_hit(self, idx): + self._hit_idx = idx % len(self._hits) + self._find_count.setText('{} of {}'.format(self._hit_idx + 1, + len(self._hits))) + self._reveal(self._hits[self._hit_idx]) + self._mark_matches() + + def _mark_matches(self): + """Repaint the search layer from the CURRENT hits: every occurrence in + amber, the one being stepped through brighter. + + Rebuilt from scratch on every call, and every path through the find box + calls it -- a query that stops matching (typing on past the last hit) + has to take its highlights with it, or the pane says 'no match' while + the old word is still lit, which reads as a wrong answer. + """ + self._sel_match = [[], []] + needle = self._find_edit.text().strip().lower() + cur = self._hits[self._hit_idx] if 0 <= self._hit_idx < len(self._hits) else None + if needle and self._hits: + for i, editor in enumerate((self.old_edit, self.new_edit)): + doc = editor.document() + for row in self._hits: + if doc.blockCount() <= row: + continue # the other side of a one-sided file + block = doc.findBlockByNumber(row) + line = block.text().lower() + colour = _FIND_CUR_BG if row == cur else _FIND_BG + at = line.find(needle) + while at >= 0: + sel = QTextEdit.ExtraSelection() + sel.format.setBackground(colour) + c = QTextCursor(block) + c.setPosition(block.position() + at) + c.setPosition(block.position() + at + len(needle), + QTextCursor.KeepAnchor) + sel.cursor = c + self._sel_match[i].append(sel) + at = line.find(needle, at + len(needle)) + self._paint_selections() + + def find_next(self): + if not self._hits: + return + self._goto_hit(self._hit_idx + 1) + + def find_prev(self): + if not self._hits: + return + self._goto_hit(self._hit_idx - 1) + def set_old_label(self, text=None, tip=None): """Name the BASELINE pane something other than its folder. @@ -421,6 +619,10 @@ def clear(self): self._logo.setVisible(False) self._msg.setText(_HINT) self._forget_units() + self._find_bar.setVisible(False) # no file: nothing to search + self._hits = [] + self._hit_idx = -1 + self._clear_selections() self.setCurrentIndex(0) def _forget_units(self): @@ -458,6 +660,7 @@ def show_file(self, rel, result, old_root, new_root): self._units = [] # nothing to sign off on a file we could not read self._message('{}\n\nCould not render — treat as potentially ' 'changed.\n{}: {}'.format(rel, type(e).__name__, e)) + self._refresh_find() self.unitChanged.emit() def file_units(self): @@ -488,6 +691,7 @@ def current_unit(self): def _message(self, text): self.rows = [] self._stops = [] + self._clear_selections() self.minimap.set_rows([]) self._pos_text = '' self._logo.setVisible(False) @@ -566,6 +770,7 @@ def _load_rows(self, rel, status, result=None, row_map=None): # back to the two-pane layout: the old editor drives again (its # scrollbar mirror carries the new pane), whatever a previous # one-sided file left the map pointing at + self._drive = self.old_edit self.minimap.set_editor(self.old_edit) self.minimap.set_rows(rows) @@ -612,14 +817,19 @@ def _load_rows(self, rel, status, result=None, row_map=None): self._reveal(self._stops[0]) else: self._pos_text = '' - self.old_edit.setExtraSelections([]) - self.new_edit.setExtraSelections([]) + self._clear_selections() self.old_edit.verticalScrollBar().setValue(0) def _load_one_side(self, rel, label, lines, side): - self.rows = [] + # rows are marked 'ctx': the pane is already one solid colour, so the + # map has nothing to add by repeating it -- but they ARE the file, and + # the find box searches rows, so a whole added file has to have them + self.rows = [Row(None, None, i + 1, line, 'ctx', 'ctx') + if side == 'new' else Row(i + 1, line, None, None, 'ctx', 'ctx') + for i, line in enumerate(lines)] self._stops = [] self._pos_text = '' + self._clear_selections() # nothing of the previous file may survive self._sem.setVisible(False) # keep _head_base in step with the shown header (an added/deleted file # has no change stops, but leaving a stale base from the previous file @@ -641,12 +851,10 @@ def _load_one_side(self, rel, label, lines, side): for i in range(len(lines)): self._block_bg(edit, i, bg) # the map still shows the file's shape, so a whole added or deleted - # file scrolls like any other. Rows are marked 'ctx' on purpose: the - # pane is already one solid colour, and repeating that on the map would - # be a red or green rectangle carrying no information. + # file scrolls like any other -- driven by the pane that holds the text + self._drive = edit self.minimap.set_editor(edit) - self.minimap.set_rows([Row(None, None, i + 1, line, 'ctx', 'ctx') - for i, line in enumerate(lines)]) + self.minimap.set_rows(self.rows) self.setCurrentIndex(1) @staticmethod @@ -700,15 +908,32 @@ def _reveal(self, row, context=3): The change block is also highlighted on both sides: without it, a file that fits on screen has nothing to scroll and the navigation looks dead even though it moved.""" - block = self.old_edit.document().findBlockByNumber(row) - self.old_edit.setTextCursor(QTextCursor(block)) + drive = self._drive + block = drive.document().findBlockByNumber(row) + drive.setTextCursor(QTextCursor(block)) # NoWrap: the vertical scrollbar is in lines, so its value is the top # visible line index - self.old_edit.verticalScrollBar().setValue(max(0, row - context)) + drive.verticalScrollBar().setValue(max(0, row - context)) self._highlight_block(row) self._update_position(row) self.unitChanged.emit() + # Two overlays share one extraSelections list per editor: the block the + # reviewer is on, and the search hits. They are kept apart so either can be + # rebuilt without wiping the other -- one list meant a cleared search took + # the current-change overlay with it, and a search that stopped matching + # left its old highlights behind. + + def _paint_selections(self): + for i, editor in enumerate((self.old_edit, self.new_edit)): + # matches last: they paint on top of the row overlay + editor.setExtraSelections(self._sel_rows[i] + self._sel_match[i]) + + def _clear_selections(self): + self._sel_rows = [[], []] + self._sel_match = [[], []] + self._paint_selections() + def _highlight_block(self, row): """Overlay the whole contiguous change block containing `row`.""" rows = self.rows @@ -719,8 +944,12 @@ def _highlight_block(self, row): start -= 1 while end + 1 < len(rows) and rows[end + 1].mode == rows[row].mode != 'ctx': end += 1 - for editor in (self.old_edit, self.new_edit): - sels = [] + self._sel_rows = [[], []] + for k, editor in enumerate((self.old_edit, self.new_edit)): + # a one-sided file leaves the other document empty: selecting a + # block it does not have would be a null cursor + if editor.document().blockCount() <= end: + continue for i in range(start, end + 1): sel = QTextEdit.ExtraSelection() sel.format.setBackground(_CUR_BG) @@ -728,8 +957,8 @@ def _highlight_block(self, row): cur = QTextCursor(editor.document().findBlockByNumber(i)) cur.clearSelection() sel.cursor = cur - sels.append(sel) - editor.setExtraSelections(sels) + self._sel_rows[k].append(sel) + self._paint_selections() def _update_position(self, row): if not self._stops: @@ -742,18 +971,33 @@ def _update_position(self, row): self._header.setText('{} · {}'.format(self._head_base, self._pos_text)) # --- change navigation (real/moved blocks; noise is skipped) --- + # + # next_change / prev_change stop at the end of THIS file and say so with + # False instead of wrapping round to the other end. Wrapping inside one + # file was silent and it dead-ended the review pass: the window catches the + # False and moves to the next changed file, so F8 walks the whole compare. def next_change(self): + """Step to the next change. False when this file has no next one.""" if not self._stops: - return - cur = self.old_edit.textCursor().blockNumber() - self._reveal(next((s for s in self._stops if s > cur), self._stops[0])) + return False + cur = self._drive.textCursor().blockNumber() + nxt = next((s for s in self._stops if s > cur), None) + if nxt is None: + return False + self._reveal(nxt) + return True def prev_change(self): + """Step to the previous change. False when this file has no earlier one.""" if not self._stops: - return - cur = self.old_edit.textCursor().blockNumber() - self._reveal(next((s for s in reversed(self._stops) if s < cur), self._stops[-1])) + return False + cur = self._drive.textCursor().blockNumber() + prv = next((s for s in reversed(self._stops) if s < cur), None) + if prv is None: + return False + self._reveal(prv) + return True def goto_name(self, key): """Jump to the first changed row naming `key`. True when it landed. diff --git a/compare_tool/qtviewer/tree.py b/compare_tool/qtviewer/tree.py index a6105e9..90d3c56 100644 --- a/compare_tool/qtviewer/tree.py +++ b/compare_tool/qtviewer/tree.py @@ -91,24 +91,35 @@ def walk(node): return walk(root) -def filter_nodes(nodes, text=''): +def filter_nodes(nodes, text='', hide_identical=False): """Narrow the tree to files whose path matches `text` (a directory survives only if a descendant matches, so empty folders collapse away). - Status is deliberately NOT a filter: the folder structure must stay stable - whatever the verdicts are, so a file never disappears from the tree just - because it is identical or noise-only. Hiding a change category folds it - into another verdict (see the compare rules) -- the row stays put and only - its label changes, so the tree never reshuffles under the reviewer.""" + `hide_identical` additionally drops every file the compare found no + difference in. It is the one status-driven filter, and it is OFF by + default and asked for explicitly: a verdict never removes a row on its + own, because the folder structure has to stay stable while the reviewer + works. A regenerated tree is mostly untouched files, though, so scrolling + past hundreds of `=` rows to reach five changed ones is its own kind of + hiding -- the reviewer gets to make that call with a button they can undo. + + Note this composes with the compare rules rather than fighting them: a + folded category re-judges its files as identical, so hiding identical + files hides those too. That is the point -- both say "this does not count". + """ text = text.strip().lower() - if not text: + if not text and not hide_identical: return list(nodes) out = [] for n in nodes: if n.is_dir: - kids = filter_nodes(n.children, text) + kids = filter_nodes(n.children, text, hide_identical) if kids: out.append(n._replace(children=kids)) - elif text in (n.rel or '').lower(): - out.append(n) + continue + if hide_identical and n.status == 'identical': + continue + if text and text not in (n.rel or '').lower(): + continue + out.append(n) return out diff --git a/tests/test_diffpane_qt.py b/tests/test_diffpane_qt.py index ccc1fd9..6742790 100644 --- a/tests/test_diffpane_qt.py +++ b/tests/test_diffpane_qt.py @@ -222,5 +222,288 @@ def test_every_row_lands_on_its_own_object_in_the_model_fixture(self): self._check(FIX / 'model_old', FIX / 'model_new') +@unittest.skipUnless(HAVE_QT, 'PySide6 not installed') +class TestFindInFile(unittest.TestCase): + """Ctrl+F searches the rows of the file on screen.""" + + def setUp(self): + from compare_tool.qtviewer.diffpane import DiffPane + self.app = _app() + self.results = scan(FIX / 'old', FIX / 'new') + self.pane = DiffPane() + self.pane.resize(1200, 600) + self.pane.setAttribute(Qt.WA_DontShowOnScreen, True) + self.pane.show() + self.addCleanup(self.pane.close) + + def _open(self, rel): + self.pane.show_file(rel, self.results[rel], + str(FIX / 'old'), str(FIX / 'new')) + for _ in range(5): + self.app.processEvents() + + def test_a_hit_on_either_side_counts_once(self): + self._open('src/rename_conflict.c') + hits = self.pane.find_matches('rtb_') + self.assertTrue(hits) + for row in hits: + r = self.pane.rows[row] + self.assertIn('rtb_', (r.old_txt or '') + (r.new_txt or '')) + # one row, not one per side, or "3 of 8" would count lines twice + self.assertEqual(len(hits), len(set(hits))) + + def test_search_is_case_insensitive_and_empty_finds_nothing(self): + self._open('src/rename_conflict.c') + self.assertEqual(self.pane.find_matches('RTB_'), + self.pane.find_matches('rtb_')) + self.assertEqual(self.pane.find_matches(' '), []) + + def test_a_whole_added_file_is_searchable(self): + # it renders through the one-sided path, which used to leave .rows + # empty -- a file with no rows is a file the find box cannot see + self._open('src/added.c') + self.assertTrue(self.pane.rows) + self.assertTrue(self.pane.find_matches('void')) + + def test_stepping_wraps_and_reports_position(self): + self._open('src/rename_conflict.c') + self.pane.open_find() + self.pane._find_edit.setText('rtb_') + for _ in range(5): + self.app.processEvents() + n = len(self.pane._hits) + self.assertGreater(n, 1) + self.assertEqual(self.pane._find_count.text(), '1 of {}'.format(n)) + for _ in range(n): # once round the whole file + self.pane.find_next() + self.assertEqual(self.pane._find_count.text(), '1 of {}'.format(n)) + self.pane.find_prev() + self.assertEqual(self.pane._find_count.text(), '{} of {}'.format(n, n)) + + def _match_marks(self): + return sum(len(s) for s in self.pane._sel_match) + + def _type(self, text): + self.pane._find_edit.setText(text) + for _ in range(5): + self.app.processEvents() + + def test_a_query_that_stops_matching_takes_its_highlights_with_it(self): + # typing on past the last hit ("begin" -> "beginal") said "no match" + # while the old word stayed lit, which reads as the wrong answer + self._open('a2l/comment_only.a2l') + self.pane.open_find() + self._type('begin') + self.assertGreater(self._match_marks(), 0) + self._type('beginal') + self.assertEqual(self.pane._find_count.text(), 'no match') + self.assertEqual(self._match_marks(), 0) + + def test_clearing_the_box_clears_the_marks(self): + self._open('a2l/comment_only.a2l') + self.pane.open_find() + self._type('begin') + self._type('') + self.assertEqual(self._match_marks(), 0) + self.assertEqual(self.pane._find_count.text(), '') + + def test_closing_the_bar_clears_the_marks(self): + self._open('a2l/comment_only.a2l') + self.pane.open_find() + self._type('begin') + self.pane.close_find() + for _ in range(5): + self.app.processEvents() + self.assertEqual(self._match_marks(), 0) + + def test_every_occurrence_is_marked_not_only_the_current_one(self): + self._open('a2l/comment_only.a2l') + self.pane.open_find() + self._type('begin') + # one mark per occurrence per pane that has the row + self.assertGreaterEqual(self._match_marks(), len(self.pane._hits)) + + def test_the_current_change_overlay_survives_a_cleared_search(self): + # the two overlays share one selection list; clearing the search used + # to wipe the block the reviewer is standing on as well + self._open('src/rename_conflict.c') + self.pane.open_find() + self._type('rtb_') + self._type('') + self.assertGreater(sum(len(s) for s in self.pane._sel_rows), 0) + + def test_leaving_a_file_takes_its_marks_with_it(self): + self._open('src/rename_conflict.c') + self.pane.open_find() + self._type('rtb_') + self._open('src/real_change.c') # no rtb_ in this one + self.assertEqual(self._match_marks(), 0) + + def test_the_query_survives_a_file_change_without_moving_the_pane(self): + self._open('src/rename_conflict.c') + self.pane.open_find() + self.pane._find_edit.setText('rtb_') + for _ in range(5): + self.app.processEvents() + self._open('src/real_change.c') + self.assertEqual(self.pane._find_edit.text(), 'rtb_') + # opening a file parks on its FIRST CHANGE; a query carried over from + # another file must not quietly scroll somewhere else + self.assertEqual(self.pane._hit_idx, -1) + self.assertIn(self.pane.old_edit.textCursor().blockNumber(), + self.pane._stops) + + +@unittest.skipUnless(HAVE_QT, 'PySide6 not installed') +class TestChangeNavigationStopsAtTheEnd(unittest.TestCase): + """next/prev report False at the ends instead of wrapping silently. + + The window uses that False to step into the next file, which is what makes + F8 walk the whole compare; wrapping inside one file dead-ended the pass. + """ + + def setUp(self): + from compare_tool.qtviewer.diffpane import DiffPane + self.app = _app() + self.results = scan(FIX / 'old', FIX / 'new') + self.pane = DiffPane() + self.pane.resize(1200, 600) + self.pane.setAttribute(Qt.WA_DontShowOnScreen, True) + self.pane.show() + self.addCleanup(self.pane.close) + self.pane.show_file('src/rename_conflict.c', + self.results['src/rename_conflict.c'], + str(FIX / 'old'), str(FIX / 'new')) + for _ in range(5): + self.app.processEvents() + + def test_next_walks_the_file_then_stops(self): + n = len(self.pane._stops) + self.assertGreater(n, 1) + self.pane.first_change() + for i in range(n - 1): + self.assertTrue(self.pane.next_change(), 'stopped at change {}'.format(i)) + self.assertFalse(self.pane.next_change()) + + def test_prev_stops_at_the_first_change(self): + self.pane.first_change() + self.assertFalse(self.pane.prev_change()) + + def test_a_file_with_no_change_stops_never_claims_to_move(self): + self.pane.show_file('src/rename_only.c', self.results['src/rename_only.c'], + str(FIX / 'old'), str(FIX / 'new')) + for _ in range(5): + self.app.processEvents() + self.assertFalse(self.pane.next_change()) + self.assertFalse(self.pane.prev_change()) + + +@unittest.skipUnless(HAVE_QT, 'PySide6 not installed') +class TestWindowLevelReviewFlow(unittest.TestCase): + """What the reviewer meets right after a scan, and how F7/F8 walk it.""" + + def setUp(self): + from compare_tool.qtviewer.app import MainWindow + self.app = _app() + self.win = MainWindow(str(FIX / 'old'), str(FIX / 'new')) + self.win.resize(1400, 800) + self.win.setAttribute(Qt.WA_DontShowOnScreen, True) + self.win.show() + self.addCleanup(self.win.close) + _settle(self.app, self.win) + + def _settle_ui(self): + for _ in range(5): + self.app.processEvents() + + def test_a_finished_scan_opens_on_the_first_change(self): + rel = self.win._selected_rel() + self.assertIsNotNone(rel, 'the scan left the diff pane empty') + self.assertIn(self.win.results[rel]['status'], self.win._NAV_STATUS) + # the first one in tree order, not just any + self.assertEqual(rel, next(r for r in self.win._tree_rels() + if self.win._is_nav(r))) + + def test_flipping_a_compare_rule_does_not_move_the_reviewer(self): + self.win._reselect('src/rename_conflict.c') + self._settle_ui() + self.win.cb_comment.setChecked(False) + self._settle_ui() + self.assertEqual(self.win._selected_rel(), 'src/rename_conflict.c') + + def test_next_change_crosses_into_the_following_file(self): + nav = [r for r in self.win._tree_rels() if self.win._is_nav(r)] + self.assertGreater(len(nav), 1) + self.win._reselect(nav[0]) + self._settle_ui() + seen = [self.win._selected_rel()] + for _ in range(60): # bounded: pressing F8 must terminate, not loop + self.win._next_change() + self._settle_ui() + rel = self.win._selected_rel() + if rel != seen[-1]: + seen.append(rel) + if len(seen) > len(nav): + break + # every file with something to review, in tree order, then round again + self.assertEqual(seen, nav + [nav[0]]) + + def test_the_walk_wraps_back_to_the_first_file(self): + nav = [r for r in self.win._tree_rels() if self.win._is_nav(r)] + self.win._reselect(nav[-1]) + self._settle_ui() + for _ in range(20): + self.win._next_change() + self._settle_ui() + if self.win._selected_rel() == nav[0]: + return + self.fail('F8 never came back round to {}'.format(nav[0])) + + def test_prev_change_steps_back_a_file_at_its_last_change(self): + nav = [r for r in self.win._tree_rels() if self.win._is_nav(r)] + target = 'src/rename_conflict.c' # the fixture file with two changes + self.assertIn(target, nav) + # the file after it, wrapping: rename_conflict.c is the last nav file + # in this fixture, so this also covers stepping back over the wrap + after = nav[(nav.index(target) + 1) % len(nav)] + self.win._reselect(after) + self._settle_ui() + self.win._prev_change() + self._settle_ui() + self.assertEqual(self.win._selected_rel(), target) + self.assertEqual(self.win.diff._cur_idx, len(self.win.diff._stops) - 1) + + def test_hide_identical_drops_only_identical_rows(self): + before = self.win._tree_rels() + identical = [r for r in before + if self.win.results[r]['status'] == 'identical'] + self.assertTrue(identical, 'fixture has no identical file to hide') + self.win.cb_hide_identical.setChecked(True) + self._settle_ui() + after = self.win._tree_rels() + self.assertEqual(after, [r for r in before if r not in identical]) + self.win.cb_hide_identical.setChecked(False) + self._settle_ui() + self.assertEqual(self.win._tree_rels(), before) + + def test_hide_identical_keeps_the_file_on_screen(self): + self.win._reselect('src/real_change.c') + self._settle_ui() + self.win.cb_hide_identical.setChecked(True) + self._settle_ui() + self.assertEqual(self.win._selected_rel(), 'src/real_change.c') + + def test_hiding_rows_never_changes_a_verdict_or_the_counts(self): + # the filter is display-only: the record the export is built from must + # not notice it at all + before = {rel: r['status'] for rel, r in self.win.results.items()} + raw = dict(self.win._raw_results) + self.win.cb_hide_identical.setChecked(True) + self._settle_ui() + self.assertEqual({rel: r['status'] for rel, r in self.win.results.items()}, + before) + self.assertEqual(set(self.win._raw_results), set(raw)) + + if __name__ == '__main__': unittest.main() diff --git a/tests/test_failsafe.py b/tests/test_failsafe.py index 326f40d..1b949e2 100644 --- a/tests/test_failsafe.py +++ b/tests/test_failsafe.py @@ -201,5 +201,54 @@ def test_stale_arxml_report_replaced_with_no_changes_page(self): self.assertEqual(rc, 0) +class TestReportNotWritten(_TreeCase): + """A run whose report could not be written left no record of itself. + + Exit 1 is the gate's ordinary "real changes found"; a crash that returned 1 + was indistinguishable from it, and the traceback that came with it told the + reviewer nothing they could act on. + """ + + def _run(self, out): + out_buf, err_buf = io.StringIO(), io.StringIO() + with contextlib.redirect_stdout(out_buf), contextlib.redirect_stderr(err_buf): + rc = main([str(self.old), str(self.new), '--report', str(out)]) + return rc, out_buf.getvalue(), err_buf.getvalue() + + def test_a_missing_output_folder_is_exit_2_not_a_traceback(self): + out = Path(self.tmp.name) / 'nodir' / 'r.html' + rc, stdout, stderr = self._run(out) + self.assertEqual(rc, 2) + self.assertIn('REPORT NOT WRITTEN', stderr) + self.assertIn(str(out), stderr) + self.assertIn('does not exist', stderr) + self.assertNotIn('Traceback', stderr) + + def test_the_scan_it_could_not_write_is_still_reported(self): + # the compare itself succeeded; throwing its result away as well would + # cost the reviewer the one thing the run did produce + rc, stdout, _err = self._run(Path(self.tmp.name) / 'nodir' / 'r.html') + self.assertEqual(rc, 2) + self.assertIn('Summary:', stdout) + self.assertIn('MODIFIED a.c', stdout) + + def test_an_unwritable_path_fails_before_the_scan_runs(self): + # a directory where the report should go: the leftover-report unlink + # hits it first, so this covers the pre-scan branch + out = Path(self.tmp.name) / 'busy' + out.mkdir() + rc, _stdout, stderr = self._run(out) + self.assertEqual(rc, 2) + self.assertIn('REPORT NOT WRITTEN', stderr) + self.assertIn('INCOMPLETE', stderr) + + def test_exit_zero_cannot_mask_a_missing_report(self): + out_buf, err_buf = io.StringIO(), io.StringIO() + with contextlib.redirect_stdout(out_buf), contextlib.redirect_stderr(err_buf): + rc = main([str(self.old), str(self.new), '--exit-zero', + '--report', str(Path(self.tmp.name) / 'nodir' / 'r.html')]) + self.assertEqual(rc, 2) + + if __name__ == '__main__': unittest.main() diff --git a/tests/test_qtviewer.py b/tests/test_qtviewer.py index 185453b..473e7ef 100644 --- a/tests/test_qtviewer.py +++ b/tests/test_qtviewer.py @@ -96,6 +96,32 @@ def test_text_filter_drops_folders_without_a_match(self): kept = filter_nodes(nodes, text='x.c') self.assertEqual([n.name for n in kept], ['a']) + def test_hide_identical_drops_only_identical_files(self): + mapping = {'a.c': 'identical', 'b.c': 'real-change', + 'noise/c.c': 'ignorable-only', 'noise/d.c': 'comment-only', + 'x/added.c': 'added', 'x/gone.c': 'deleted', 'bad.c': 'error'} + kept = filter_nodes(self._nodes(mapping), hide_identical=True) + self.assertEqual(sorted(self._rels(kept)), + sorted(r for r in mapping if mapping[r] != 'identical')) + + def test_hide_identical_collapses_a_folder_with_nothing_left(self): + nodes = self._nodes({'quiet/a.c': 'identical', 'quiet/b.c': 'identical', + 'src/c.c': 'real-change'}) + kept = filter_nodes(nodes, hide_identical=True) + self.assertEqual([n.name for n in kept], ['src']) + + def test_hide_identical_and_the_text_filter_compose(self): + nodes = self._nodes({'src/ctrl.c': 'identical', 'src/ctrl_b.c': 'real-change', + 'other/ctrl.c': 'real-change'}) + kept = filter_nodes(nodes, text='src/', hide_identical=True) + self.assertEqual(self._rels(kept), ['src/ctrl_b.c']) + + def test_hide_identical_is_off_by_default(self): + # a verdict removing a row is opt-in: the folder structure has to stay + # stable unless the reviewer asked for it + nodes = self._nodes({'a.c': 'identical'}) + self.assertEqual(self._rels(filter_nodes(nodes)), ['a.c']) + class TestReviewState(unittest.TestCase): """The colour behind the tree's Review column. 'done' is a claim that