From d707df349e6f0d008af7124c87f427017ff4b587 Mon Sep 17 00:00:00 2001 From: longvo920 Date: Thu, 30 Jul 2026 21:18:42 +0700 Subject: [PATCH 1/7] feat: light/dark themes, A2L colouring, muted noise instead of hidden noise Three changes that all landed on the same seam problem: colour was written inline on every surface, so a second scheme meant answering "what does a removed line look like" five times. theme.py is that seam. Every colour is a named role with one value per theme; the report emits the whole palette as CSS custom properties and uses var(--role), the Qt widgets look the same role up with theme.c. Both palettes go into the report -- it is read on machines with no internet, so the reader's switch has to be an attribute flip with nothing left to fetch. --theme picks what either front end opens with; the viewer's toolbar and the report's own button switch afterwards. Switching a noise category off now greys its rows rather than collapsing them. A regenerated file is mostly banner churn, so folding it removed most of the file and left the surviving hunks with no context to be read in. The rows keep their place, their line numbers and their text, and only stop counting on the surfaces that answer "where do I look next" -- the minimap, and F7/F8, which already skipped them. Row indices no longer move, so navigation stops and find hits need no translation across the fold. A2L is highlighted from a curated keyword list plus the block name's position, never from shape: an [A-Z_]+ rule lights up the calibration object names too, and those are the one thing a reviewer scans an a2l diff for. The stripper's A2L string rules come with it -- a doubled quote continues the literal and a backslash is a literal character. --- CHANGELOG.md | 20 ++ CLAUDE.md | 19 +- README.md | 10 +- compare_tool/main.py | 22 +- compare_tool/qtviewer/app.py | 244 +++++++++++++------ compare_tool/qtviewer/dialogs.py | 13 +- compare_tool/qtviewer/diffpane.py | 251 +++++++++++--------- compare_tool/qtviewer/highlight.py | 90 ++++--- compare_tool/qtviewer/icons.py | 37 +-- compare_tool/qtviewer/minimap.py | 40 ++-- compare_tool/qtviewer/summary.py | 19 +- compare_tool/qtviewer/tree.py | 37 ++- compare_tool/report.py | 299 +++++++++++++++--------- compare_tool/syntax.py | 121 ++++++++-- compare_tool/theme.py | 361 +++++++++++++++++++++++++++++ compare_tool/view_model.py | 65 +++--- docs/architecture.md | 36 ++- docs/vi/README.md | 20 +- docs/vi/architecture.md | 36 ++- tests/test_cli_modes.py | 24 ++ tests/test_diffpane_qt.py | 153 ++++++++++++ tests/test_qtviewer.py | 14 +- tests/test_report.py | 103 ++++++-- tests/test_syntax.py | 76 +++++- tests/test_theme.py | 96 ++++++++ tests/test_view_model.py | 80 +++---- 26 files changed, 1759 insertions(+), 527 deletions(-) create mode 100644 compare_tool/theme.py create mode 100644 tests/test_theme.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e764ea..c4a0ee0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,26 @@ All notable changes to this project are documented here. Versions follow [semantic versioning](https://semver.org/). +## [Unreleased] + +### Added + +- **Light and dark colour schemes.** Both the viewer and the report can be read + either way. `--theme dark|light` sets which one they open with (dark stays the + default), the viewer has a switch in its toolbar, and the report carries both + schemes inside the file — so its own switch works with no internet, and your + choice is remembered for the next report you open. +- **A2L files are syntax-coloured** in the viewer, keywords and block types + apart from the calibration object names — so a name still stands out in a + page of ASAM keywords. + +### Changed + +- **Hiding comment or unimportant differences now greys those lines out instead + of removing them.** They keep their place and their line numbers, so the code + around a real change is still there to read it in, and they no longer count + as changes on the minimap or when stepping through changes. + ## [1.2.0] — 2026-07-29 Compare a folder against its own git history, sign off a whole file at once, diff --git a/CLAUDE.md b/CLAUDE.md index 9a350c5..e49f279 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -50,12 +50,25 @@ place, `diff_engine._status_of`. Only noise verdicts are foldable (`scanner.FOLDABLE`). `real-change`, `added`, `deleted` and `error` can **never** be folded away by a UI toggle. +Folding a category in the viewer changes the file's verdict and **greys** its +rows (`view_model.mute_rows`) — it does not remove them. The lines stay +readable, and only the "where should I look next" surfaces (minimap, F7/F8) +stop counting them. Collapsing them to a `⋯ N lines hidden` placeholder was +tried and reverted: a regenerated file is mostly banner churn, so it took the +context the surviving hunks have to be read in. + ## 3. One seam per shared decision Any fact two renderers need lives in **one** module they both import. -`compare_tool/view_model.py` holds `mode_of`, `char_span` and `aligned_rows`; -the HTML report and the Qt viewer both consume them, so they cannot disagree -about what changed or how it is coloured. +`compare_tool/view_model.py` holds `mode_of`, `char_span`, `aligned_rows` and +`mute_rows`; `compare_tool/theme.py` holds every colour as a named role, one +value per theme. The HTML report and the Qt viewer both consume them, so they +cannot disagree about what changed or how it is coloured. + +A colour literal outside `theme.py` is a bug: it paints one theme correctly and +the other by accident. Add a role to **both** palettes (an import-time assert +enforces it), then use `var(--role)` in the report's CSS or `theme.c(role)` in +Qt. Re-implementing a mapping inline "because it is only four lines" is the bug: the copies drift the moment a new kind is added. If you find a duplicated diff --git a/README.md b/README.md index 03ea1a9..bc1851f 100644 --- a/README.md +++ b/README.md @@ -89,6 +89,7 @@ scan did find is still printed. | `--exit-zero` | Always exit 0 even when real changes exist (report-only mode for pipelines). Compare errors still exit 2 | | `--arxml-only` | Scan only `.arxml`/`.xml`/`.a2l` and write a compact per-type report (default `arxml_update.html`) — always written, even when nothing changed | | `--review FILE` | Render notes and sign-offs from a review file (`codegen-review.json`, written by the viewer) next to the changes they belong to, plus a `Reviewed` badge that hides the changes already signed off. Must be named explicitly — a report must not pick up someone else's sign-off by accident; no effect with `--arxml-only` | +| `--theme dark\|light` | Colour scheme the report and the viewer open with (default `dark`). The report carries **both** and has its own switch, so this only sets what the reader sees first | | `--qt`, `--viewer` | Open the side-by-side viewer on folders named on the command line, instead of comparing them in the terminal. Needs the `viewer` extra (see below) | Omitting `old_dir`/`new_dir` opens the viewer. `--gui` (the tkinter panel) was removed in 1.1.0. @@ -113,6 +114,8 @@ Reading a scan: - `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. +- Unticking `Comment` / `Unimportant` **greys those lines out** rather than removing them: they stay where they are, keep their line numbers, lose their red/green, and drop off the minimap and out of `F7`/`F8`. The code around a change is what makes it readable, and a regenerated file is mostly banner churn — folding it away took most of the file with it. +- `☀ Light` / `☾ Dark` in the toolbar switches the colour scheme; `--theme` picks the one it starts in. C, ARXML and A2L are syntax-coloured in both. `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. @@ -172,7 +175,7 @@ Files are grouped by **Simulink model** using the Embedded Coder AUTOSAR naming ## HTML report -Self-contained file, one per compare: badge toggles, folder tree, filter box, collapsible diffs per file. Opens `Unimportant` hidden and `Modified` expanded, so it opens on what matters. +Self-contained file, one per compare: badge toggles, folder tree, filter box, collapsible diffs per file. Opens `Unimportant` hidden and `Modified` expanded, so it opens on what matters. A `☀ Light` / `☾ Dark` button sits in the top right — both palettes are embedded in the file, so switching fetches nothing and works on a machine with no internet. ![Report viewer](resources/pic/report_page.png) @@ -230,13 +233,14 @@ compare_tool/ ├── arxml_rules.py # ARXML rules: UUID, ADMIN-DATA, DATE, comments + extract port interfaces, SWCs (ports/runnables/events) ├── a2l_rules.py # A2L rules: strip C-style comments + extract CHARACTERISTIC/MEASUREMENT ├── view_model.py # renderer-agnostic view model (paint mode, intra-line span, row alignment) shared by the report and the viewer -├── syntax.py # line-at-a-time C / XML token spans, Qt-free so it ships in the .pyz +├── theme.py # the dark and light palettes as named roles, shared by the report's CSS and every Qt surface +├── syntax.py # line-at-a-time C / XML / A2L token spans, Qt-free so it ships in the .pyz ├── review.py # reviewer notes and sign-offs, keyed by change content so they survive a rescan ├── gitsource.py # read-only `git archive` of a commit into a temp folder, so a commit can be the OLD side └── report.py # self-contained HTML report (badge toggles, model overview, grouping, filter, collapsible diffs) ``` -[docs/architecture.md](docs/architecture.md) covers how these fit together and why: the two diff passes, where a verdict is decided, the shared seams and the result-dict contract. Anything both renderers need lives in `view_model.py` — reimplementing a mapping inline lets the HTML report and the viewer drift apart about what changed. +[docs/architecture.md](docs/architecture.md) covers how these fit together and why: the two diff passes, where a verdict is decided, the shared seams and the result-dict contract. Anything both renderers need lives in `view_model.py` (what changed) or `theme.py` (what colour it gets) — reimplementing a mapping inline lets the HTML report and the viewer drift apart. To add a rule: write the strip function in `c_rules.py` / `arxml_rules.py` / `a2l_rules.py`, join it into that ruleset's shadow, register one labelled variant in `_build_variants` in `diff_engine.py`, and add both tests — the pattern alone is noise, and the same pattern *beside* a real change still reports the real change. diff --git a/compare_tool/main.py b/compare_tool/main.py index abd6f94..45fc32b 100644 --- a/compare_tool/main.py +++ b/compare_tool/main.py @@ -2,14 +2,14 @@ Usage: python -m compare_tool [--report out.html] [--arxml-only] - python -m compare_tool # side-by-side viewer + python -m compare_tool [--theme light] # side-by-side viewer """ import argparse import sys from pathlib import Path -from . import review +from . import review, theme from .diff_engine import RULES from .report import build_arxml_report, build_report from .scanner import (scan, summarize, summarize_a2l, summarize_ifaces, @@ -47,7 +47,7 @@ def default_report_name(arxml_only): def run_compare(old_root, new_root, out, arxml_only=False, exclude=(), - progress=None, reviews=None): + progress=None, reviews=None, theme_name=theme.DEFAULT): """Scan two trees and write the HTML report. 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 @@ -68,9 +68,11 @@ def run_compare(old_root, new_root, out, arxml_only=False, exclude=(), if arxml_only: # ALWAYS written: "no changes" must be an explicit statement, never # a silently absent file (indistinguishable from a run that died) - page = build_arxml_report(results, old_root, new_root) + page = build_arxml_report(results, old_root, new_root, + theme_name=theme_name) else: - page = build_report(results, old_root, new_root, reviews) + page = build_report(results, old_root, new_root, reviews, + theme_name=theme_name) try: out.write_text(page, encoding='utf-8') except OSError as e: @@ -184,6 +186,12 @@ def _parser(): 'diff report; the report is ALWAYS written -- when ' 'nothing real changed it states "no changes" ' 'explicitly per file type') + ap.add_argument('--theme', choices=theme.THEMES, default=theme.DEFAULT, + help='colour scheme the viewer opens with, and the one the ' + 'HTML report opens with (default: dark). The report ' + 'always carries both, so its own button switches with ' + 'nothing to download; the viewer has the same button ' + 'in its toolbar') ap.add_argument('--exclude', metavar='PATTERN', action='append', default=[], help='skip files matching this glob (relative path or bare ' 'file name); repeatable. Example: --exclude compare_report.html') @@ -230,7 +238,7 @@ def main(argv=None): from .qtviewer import run_viewer # deferred: PySide6 may be absent try: return run_viewer(args.old_dir, args.new_dir, exclude=args.exclude, - arxml_only=args.arxml_only) + arxml_only=args.arxml_only, theme_name=args.theme) except ImportError as e: # a stdlib-only install (the .pyz, a locked-down box) has no Qt. # Say so plainly instead of dumping a traceback. @@ -275,7 +283,7 @@ def progress(done, total, rel): try: results, counts = run_compare(old_root, new_root, out, args.arxml_only, exclude=args.exclude, progress=progress, - reviews=reviews) + reviews=reviews, theme_name=args.theme) 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 diff --git a/compare_tool/qtviewer/app.py b/compare_tool/qtviewer/app.py index da350f6..90ec0cb 100644 --- a/compare_tool/qtviewer/app.py +++ b/compare_tool/qtviewer/app.py @@ -21,7 +21,7 @@ QToolButton, QTreeWidget, QTreeWidgetItem, QVBoxLayout, QWidget) -from .. import gitsource, review +from .. import gitsource, review, theme from ..diff_engine import RULES from ..main import default_report_name from ..report import build_arxml_report, build_report @@ -31,7 +31,8 @@ from .icons import ACCENT, app_icon, icon, std_icon from .pickers import pick_commit, pick_folders from .summary import SummaryPanel -from .tree import REVIEW_COLOR, STATUS, build_nodes, filter_nodes, review_state +from .tree import (STATUS, build_nodes, filter_nodes, review_color, + review_state, status_color) from .worker import ScanWorker REL_ROLE = Qt.UserRole # a FILE row's relative path (folders: None) @@ -89,8 +90,11 @@ def focusOutEvent(self, event): class MainWindow(QMainWindow): - def __init__(self, old=None, new=None, exclude=(), arxml_only=False): + def __init__(self, old=None, new=None, exclude=(), arxml_only=False, + theme_name=theme.DEFAULT): super().__init__() + self._theme = theme.set_current(theme_name) + self._state = ('idle', 'Ready') self.old = old self.new = new self.exclude = tuple(exclude) @@ -117,8 +121,6 @@ def __init__(self, old=None, new=None, exclude=(), arxml_only=False): self.banner = QLabel() self.banner.setVisible(False) self.banner.setWordWrap(True) - self.banner.setStyleSheet('background:#4a1d1d; color:#ffd6d6; padding:6px 10px;' - 'font-weight:bold; border-bottom:1px solid #b04a4a;') self.tree = QTreeWidget() # the Review column exists at all times but is hidden until review mode @@ -236,7 +238,6 @@ def __init__(self, old=None, new=None, exclude=(), arxml_only=False): self.state_label.setStyleSheet('padding:0 8px;') self.statusBar().addWidget(self.state_label) self.counts_label = QLabel('') - self.counts_label.setStyleSheet('color:#9aa1ad; padding:0 8px;') self.statusBar().addPermanentWidget(self.counts_label) self.progress = QProgressBar() self.progress.setMaximumWidth(240) @@ -245,6 +246,7 @@ def __init__(self, old=None, new=None, exclude=(), arxml_only=False): self._set_state('idle', 'Ready') self._build_toolbar() + self._style_widgets() if self.old and self.new: self._start_scan() @@ -259,21 +261,77 @@ def __init__(self, old=None, new=None, exclude=(), arxml_only=False): # tool-state chip on the status bar: a coloured dot plus a word, so the # reviewer can tell at a glance whether a result is final or still coming - _STATE_DOT = {'idle': '#8a8f98', 'busy': '#e2c16b', - 'ready': '#7bd88a', 'error': '#ff7b7b'} + _STATE_DOT = {'idle': 'state-idle', 'busy': 'state-busy', + 'ready': 'state-ready', 'error': 'state-error'} def _set_state(self, kind, text): - dot = self._STATE_DOT.get(kind, '#8a8f98') + dot = theme.c(self._STATE_DOT.get(kind, 'state-idle')) + self._state = (kind, text) self.state_label.setText( '●  {}' .format(dot, text)) + # --- theme --- + + def _style_widgets(self): + """The stylesheets this window sets by hand (the rest is the app-wide + QSS). One place, so a theme switch is one call.""" + self.banner.setStyleSheet( + 'background:{}; color:{}; padding:6px 10px; font-weight:bold; ' + 'border-bottom:1px solid {};'.format( + theme.c('err-bg'), theme.c('err-fg'), theme.c('err-border'))) + self.counts_label.setStyleSheet('color:{}; padding:0 8px;' + .format(theme.c('st-ign'))) + self.review_where.setStyleSheet('color:{}; font-size:11px;' + .format(theme.c('state-idle'))) + self._show_review_file() # owns review_file's colour: normal or warning + + def _toggle_theme(self): + self._set_theme(theme.other()) + + def _set_theme(self, name): + """Repaint the whole window in `name`. + + Every surface that stamps a colour into a widget rather than reading it + from the stylesheet has to be told: the tree's verdict colours, the + quick-changes rows, the diff pane's block formats, the tinted icons. + Missing one leaves half the window in the old theme, which is why they + are listed here and not discovered by walking children. + """ + if theme.set_current(name) == self._theme: + return + self._theme = theme.current() + apply_theme(QApplication.instance()) + self._style_widgets() + self._set_state(*self._state) + self._apply_icons() + self.summary.apply_theme() + self.diff.apply_theme() + self._refresh_tree_keep_selection() # verdict colours are per item + self.act_theme.setText(self._theme_label()) + self.act_theme.setChecked(self._theme == theme.LIGHT) + + @staticmethod + def _theme_label(): + return '☀ Light' if theme.current() == theme.DARK else '☾ Dark' + + def _apply_icons(self): + """Re-tint every shipped glyph for the current chrome.""" + self.act_open.setIcon(std_icon(self, QStyle.SP_DirOpenIcon)) + for act, glyph, role in self._icon_actions: + act.setIcon(icon(glyph, role) if role else icon(glyph)) + self.help_button.setIcon(icon('report')) + # --- actions, toolbar, bottom action bar --- def _make_actions(self): """Every command the window offers, in one place. The bottom bar and the toolbar are just two views of these actions, so a button and its shortcut can never drift apart.""" + # (action, glyph, role): what _apply_icons re-tints after a theme + # switch. A QIcon carries baked pixels, so it cannot follow a palette + # by itself. + self._icon_actions = [] self.act_open = QAction(std_icon(self, QStyle.SP_DirOpenIcon), 'Open folders…', self) self.act_open.setToolTip('Choose the BASELINE and CURRENT folders — or ' @@ -287,6 +345,7 @@ def _make_actions(self): self.act_git.setToolTip('Compare a folder against one of its own ' 'commits — no second folder to choose') self.act_git.triggered.connect(self._pick_commit) + self._icon_actions.append((self.act_git, 'git-commit', None)) # 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 @@ -305,6 +364,7 @@ def _make_actions(self): act.setToolTip('{} ({}) — noise is skipped, {}'.format(text, key, scope)) act.triggered.connect(slot) setattr(self, attr, act) + self._icon_actions.append((act, glyph, None)) # these four live in the diff pane's own header, beside the file name # they step through -- a bar of their own at the bottom repeated the # same "change k of N" the header already shows @@ -317,6 +377,7 @@ def _make_actions(self): self.act_export.setToolTip('Write the full HTML report (Ctrl+E) — always ' 'the complete scan, never the folded view') self.act_export.triggered.connect(self._export_report) + self._icon_actions.append((self.act_export, 'export', ACCENT)) # signing off is a second pass, not part of reading a diff, so the note # box stays out of the way until it is asked for -- it was taking a @@ -326,6 +387,17 @@ def _make_actions(self): self.act_review_mode.setToolTip('Show the note box and sign-off for the ' 'current change') self.act_review_mode.toggled.connect(self._set_review_mode) + self._icon_actions.append((self.act_review_mode, 'review-comment', None)) + + # text and a sun/moon glyph, no shipped icon: the label names where the + # click GOES, and reusing another button's glyph for it would make two + # different commands look like the same one + self.act_theme = QAction(self._theme_label(), self) + self.act_theme.setCheckable(True) + self.act_theme.setChecked(self._theme == theme.LIGHT) + self.act_theme.setToolTip('Switch the viewer between the dark and the ' + 'light colour scheme') + self.act_theme.triggered.connect(self._toggle_theme) # no button of its own: the tick in the review bar IS the button. The # shortcut exists so a review pass can stay on the keyboard -- F8, tick, @@ -344,10 +416,12 @@ def _make_actions(self): self.act_guide.setShortcut('F1') self.act_guide.setToolTip('How to use the viewer (F1)') self.act_guide.triggered.connect(lambda: show_user_guide(self)) + self._icon_actions.append((self.act_guide, 'report', None)) self.act_notes = QAction(icon('review-resolved'), 'Release notes', self) self.act_notes.setToolTip("What changed in this and earlier versions") self.act_notes.triggered.connect(lambda: show_release_notes(self)) + self._icon_actions.append((self.act_notes, 'review-resolved', None)) self.act_about = QAction(app_icon(), 'About', self) self.act_about.setToolTip('Version, author and license') @@ -375,6 +449,9 @@ def _build_toolbar(self): spacer = QWidget() spacer.setSizePolicy(QSizePolicy.Expanding, QSizePolicy.Preferred) tb.addWidget(spacer) + # over on the right with Help: both are about the tool, not about the + # compare, and the left half of the bar is for the latter + tb.addAction(self.act_theme) # the three help pages behind one menu on the right: they are read once # and then never again, so three permanent buttons were spending the # top bar on the rarest thing in the window @@ -445,9 +522,7 @@ def _review_bar(self): self.btn_file_reviewed.setCursor(Qt.PointingHandCursor) self.btn_file_reviewed.clicked.connect(self._toggle_file_reviewed) self.review_where = QLabel('') - self.review_where.setStyleSheet('color:#8a8f98; font-size:11px;') self.review_file = QLabel('') - self.review_file.setStyleSheet('color:#6f757e; font-size:11px;') side = QVBoxLayout() side.setContentsMargins(0, 0, 0, 0) @@ -584,14 +659,16 @@ def _show_review_file(self): return if self._reviews.error: self.review_file.setText('⚠ {}'.format(path.name)) - self.review_file.setStyleSheet('color:#ff9d9d; font-size:11px;') + self.review_file.setStyleSheet('color:{}; font-size:11px;' + .format(theme.c('st-err'))) self.review_file.setToolTip('{}\n\n{}\n\nNothing is loaded from it ' 'and nothing will be written over it. ' 'Fix or remove the file, then rescan.' .format(path, self._reviews.error)) else: self.review_file.setText(path.name) - self.review_file.setStyleSheet('color:#6f757e; font-size:11px;') + self.review_file.setStyleSheet('color:{}; font-size:11px;' + .format(theme.c('fg-muted'))) self.review_file.setToolTip('Notes and sign-offs are saved to\n{}' .format(path)) @@ -759,10 +836,12 @@ def _checkout(self, root, sub, commit): # --- scan lifecycle --- - # a folded category disappears from BOTH places it shows: the file's verdict - # (status -> Identical/Modified) and the lines in the diff panes. Leaving a - # wall of coloured rows in the code after saying those do not count was - # the worst of both. + # switching a category off changes BOTH places it shows: the file's verdict + # (status -> Identical/Modified) and how its lines are painted in the diff + # panes -- greyed out, and dropped from the minimap and from F7/F8. Leaving + # a wall of red and green in the code after saying those do not count was + # the worst of both; taking the lines away instead cost the context the + # remaining changes have to be read in. _FOLD_MODE = {'comment-only': 'comment', 'ignorable-only': 'minor'} def _fold(self): @@ -840,7 +919,7 @@ def _apply_rules(self): keep = self._selected_rel() fold = self._fold() self.results = apply_fold(self._raw_results, fold) - self.diff.set_fold_modes([self._FOLD_MODE[f] for f in fold]) + self.diff.set_muted_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: @@ -898,15 +977,19 @@ def _export_report(self): # the ARXML/A2L report lists files, not individual changes, so # there is nothing for a per-change note to attach to page = build_arxml_report(self._raw_results, self.old, self.new, - old_label=self._old_label) + old_label=self._old_label, + theme_name=self._theme) else: # only pass the store when it holds something: an untouched # review would otherwise add a "0 of N Reviewed" badge to every # report, for a feature that run never used store = self._reviews if (self._reviews.any_entries() or self._reviews.error) else None + # the report opens in whatever the viewer is showing, and + # carries both palettes so the reader can still switch page = build_report(self._raw_results, self.old, self.new, store, - old_label=self._old_label) + old_label=self._old_label, + theme_name=self._theme) Path(out).write_text(page, encoding='utf-8') except Exception as e: QMessageBox.critical(self, 'Export failed', @@ -944,9 +1027,9 @@ def _refresh_tree_keep_selection(self): def _fill_tree(self, nodes): def add(parent, node, prefix): - marker, label, color = STATUS[node.status] + marker, label, _role = STATUS[node.status] item = QTreeWidgetItem(['{} {}'.format(marker, node.name), label]) - brush = QBrush(QColor(color)) + brush = QBrush(QColor(status_color(node.status))) item.setForeground(0, brush) item.setForeground(1, brush) rel = node.rel or (prefix + node.name) @@ -1016,11 +1099,11 @@ def _paint_review(self, item, done, total): # or NOT-compared file has nothing anyone could have read, and a # free "done" on it is exactly the false all-clear to avoid. item.setText(REVIEW_COL, '—') - item.setForeground(REVIEW_COL, QBrush(QColor('#5a5d63'))) + item.setForeground(REVIEW_COL, QBrush(QColor(theme.c('fg-muted')))) item.setToolTip(REVIEW_COL, 'Nothing here can be signed off.') return item.setText(REVIEW_COL, '{}/{}'.format(done, total)) - item.setForeground(REVIEW_COL, QBrush(QColor(REVIEW_COLOR[state]))) + item.setForeground(REVIEW_COL, QBrush(QColor(review_color(state)))) item.setToolTip(REVIEW_COL, '{} of {} change(s) reviewed'.format(done, total)) # --- right-click: open the file where it really lives --- @@ -1178,60 +1261,69 @@ def _on_select(self): # chrome styling. Deliberately narrow: the diff editors, the minimap and the # per-file tree colours are painted in code, and a stylesheet rule on their -# items would override those verdict colours. +# items would override those verdict colours. Colours are named as theme roles +# and filled in by apply_theme, so there is one QSS for both schemes. _QSS = """ -QToolBar#main { background:#25262a; border:0; border-bottom:1px solid #34363c; - padding:4px 12px 4px 6px; spacing:2px; } +QToolBar#main {{ background:{chrome-bg}; border:0; border-bottom:1px solid {border}; + padding:4px 12px 4px 6px; spacing:2px; }} /* the Help button carries a menu: without room for it the arrow is clipped against the window edge */ -QToolBar#main QToolButton::menu-indicator { subcontrol-position: right center; - subcontrol-origin: padding; right:-2px; } -QToolBar#main QToolButton { padding:5px 10px; border-radius:6px; color:#d7d7d7; } -QToolBar#main QToolButton:hover { background:#34363c; } -QToolBar#main QToolButton:pressed { background:#3d404a; } +QToolBar#main QToolButton::menu-indicator {{ subcontrol-position: right center; + subcontrol-origin: padding; right:-2px; }} +QToolBar#main QToolButton {{ padding:5px 10px; border-radius:6px; color:{icon-tint}; }} +QToolBar#main QToolButton:hover {{ background:{chrome-hover}; }} +QToolBar#main QToolButton:pressed {{ background:{chrome-pressed}; }} /* Review mode is a MODE: without a lit checked state the button looks the same on as off, and the note box appearing is the only clue it worked */ -QToolBar#main QToolButton:checked { background:#343a63; color:#e8e8ff; } -QToolBar#main QToolButton:checked:hover { background:#454c80; } -QFrame#reviewbar { background:#212226; border-top:1px solid #34363c; } -QFrame#reviewbar QPlainTextEdit { background:#232427; border:1px solid #3a3c42; - border-radius:6px; padding:4px 6px; color:#d4d4d4; } -QFrame#reviewbar QPlainTextEdit:focus { border:1px solid #7c8cf8; } -QFrame#reviewbar QPlainTextEdit:disabled { background:#1f2023; color:#6a6a6a; - border:1px solid #2f3136; } -QToolButton#primary { background:#343a63; } -QToolButton#primary:hover { background:#454c80; } -QToolButton#primary:disabled { background:#2b2d33; } -QTreeWidget { border:1px solid #34363c; border-radius:6px; } -QTreeWidget::item { padding:2px 0; } -QTreeWidget::item:selected { background:#3a4a7a; } -QHeaderView::section { background:#2a2c31; color:#b9b9b9; border:0; - border-right:1px solid #34363c; padding:4px 6px; } -QLineEdit { background:#232427; border:1px solid #3a3c42; border-radius:6px; - padding:5px 8px; } -QLineEdit:focus { border:1px solid #7c8cf8; } -QSplitter::handle { background:#34363c; } -QSplitter::handle:horizontal { width:3px; } -QSplitter::handle:vertical { height:3px; } -QStatusBar { background:#25262a; color:#b0b0b0; border-top:1px solid #34363c; } -QProgressBar { background:#232427; border:1px solid #3a3c42; border-radius:6px; - text-align:center; color:#d0d0d0; } -QProgressBar::chunk { background:#4F46E5; border-radius:5px; } -QCheckBox { spacing:6px; } +QToolBar#main QToolButton:checked {{ background:{chrome-checked-bg}; + color:{chrome-checked-fg}; }} +QToolBar#main QToolButton:checked:hover {{ background:{chrome-checked-hover}; }} +QFrame#reviewbar {{ background:{chrome-bar-bg}; border-top:1px solid {border}; }} +QFrame#reviewbar QPlainTextEdit {{ background:{code-bg}; border:1px solid {border}; + border-radius:6px; padding:4px 6px; color:{fg}; }} +QFrame#reviewbar QPlainTextEdit:focus {{ border:1px solid {accent-2}; }} +QFrame#reviewbar QPlainTextEdit:disabled {{ background:{chrome-disabled-bg}; + color:{chrome-disabled-fg}; border:1px solid {border}; }} +QToolButton#primary {{ background:{chrome-checked-bg}; color:{chrome-checked-fg}; }} +QToolButton#primary:hover {{ background:{chrome-checked-hover}; }} +QToolButton#primary:disabled {{ background:{chrome-disabled-bg}; + color:{chrome-disabled-fg}; }} +QTreeWidget {{ border:1px solid {border}; border-radius:6px; }} +QTreeWidget::item {{ padding:2px 0; }} +QTreeWidget::item:selected {{ background:{tree-selected}; }} +QHeaderView::section {{ background:{header-bg}; color:{header-fg}; border:0; + border-right:1px solid {border}; padding:4px 6px; }} +QLineEdit {{ background:{code-bg}; border:1px solid {border}; border-radius:6px; + padding:5px 8px; }} +QLineEdit:focus {{ border:1px solid {accent-2}; }} +QSplitter::handle {{ background:{border}; }} +QSplitter::handle:horizontal {{ width:3px; }} +QSplitter::handle:vertical {{ height:3px; }} +QStatusBar {{ background:{chrome-bg}; color:{status-fg}; border-top:1px solid {border}; }} +QProgressBar {{ background:{code-bg}; border:1px solid {border}; border-radius:6px; + text-align:center; color:{fg}; }} +QProgressBar::chunk {{ background:{progress-chunk}; 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; } +QWidget#findbar {{ background:{chrome-bar-bg}; border-top:1px solid {border}; + border-bottom:1px solid {border}; }} +QWidget#findbar QToolButton {{ color:{st-ign}; padding:2px 6px; border-radius:4px; }} +QWidget#findbar QToolButton:hover {{ background:{chrome-hover}; color:{fg-strong}; }} """ -def _apply_dark(app): - """Fusion dark palette so the viewer matches the report's dark identity.""" +def apply_theme(app): + """Palette and stylesheet for the whole application, in the current theme. + + Fusion in both directions: the native Windows style ignores most of a + palette, so a light run under it would come out as a half-themed window + rather than a light one. + """ app.setStyle('Fusion') p = QPalette() - bg, base, text = QColor('#1e1f22'), QColor('#232427'), QColor('#d4d4d4') + bg, base, text = (QColor(theme.c('bg')), QColor(theme.c('code-bg')), + QColor(theme.c('fg'))) p.setColor(QPalette.Window, bg) p.setColor(QPalette.Base, base) p.setColor(QPalette.AlternateBase, bg) @@ -1239,13 +1331,15 @@ def _apply_dark(app): p.setColor(QPalette.WindowText, text) p.setColor(QPalette.Button, base) p.setColor(QPalette.ButtonText, text) - p.setColor(QPalette.Highlight, QColor('#3a5a7a')) - p.setColor(QPalette.HighlightedText, QColor('#ffffff')) + p.setColor(QPalette.ToolTipBase, base) + p.setColor(QPalette.ToolTipText, text) + p.setColor(QPalette.Highlight, QColor(theme.c('tree-selected'))) + p.setColor(QPalette.HighlightedText, QColor(theme.c('fg-strong'))) # the filter box's "Filter by path…" placeholder: Fusion fades it so far it - # is barely legible on the dark field, so set an explicit, readable grey - p.setColor(QPalette.PlaceholderText, QColor('#9aa1ad')) + # is barely legible, so set an explicit, readable grey + p.setColor(QPalette.PlaceholderText, QColor(theme.c('st-ign'))) app.setPalette(p) - app.setStyleSheet(_QSS) + app.setStyleSheet(_QSS.format(**theme.palette())) def _taskbar_identity(): @@ -1259,16 +1353,18 @@ def _taskbar_identity(): pass # not Windows, or the call is unavailable: cosmetic either way -def run_viewer(old=None, new=None, exclude=(), arxml_only=False): +def run_viewer(old=None, new=None, exclude=(), arxml_only=False, + theme_name=theme.DEFAULT): app = QApplication.instance() owns = app is None if owns: _taskbar_identity() app = QApplication(sys.argv[:1]) - _apply_dark(app) + theme.set_current(theme_name) + apply_theme(app) app.setApplicationName('CodeGen Compare') app.setWindowIcon(app_icon()) - win = MainWindow(old, new, exclude, arxml_only) + win = MainWindow(old, new, exclude, arxml_only, theme_name) win.show() return app.exec() if owns else 0 diff --git a/compare_tool/qtviewer/dialogs.py b/compare_tool/qtviewer/dialogs.py index 0053c81..fa84f24 100644 --- a/compare_tool/qtviewer/dialogs.py +++ b/compare_tool/qtviewer/dialogs.py @@ -56,20 +56,27 @@ ## 4. Fold the noise - Untick `Comment` / `Unimportant` -- affected files re-judge instantly -- Folded lines collapse to `⋯ N lines hidden` -- Tick back on to bring them back +- Those lines are **greyed out, not removed**: they stay where they are, keep + their line numbers, and lose their red/green +- They also drop off the minimap and out of `F7` / `F8`, so nothing sends you + back to them +- Tick back on to bring the colour back - Real changes can never be folded away ## 5. Read the diff - Baseline on the left, Current on the right, scrolled in lockstep - Minimap on the right edge -- click or drag to jump -- C and ARXML are syntax-coloured; the code colours never use red or green +- C, ARXML and A2L are syntax-coloured; the code colours never use red or green +- `☀ Light` / `☾ Dark` in the toolbar switches the colour scheme. Start in one + with `--theme dark|light`; an exported report opens in the same one and + carries its own switch | Row colour | Meaning | |---|---| | red / green | removed / added | | dim red / green | noise: comment, UUID, rename, whitespace | +| flat grey | a category you switched off -- shown, but not a change | | blue | moved block | ## 6. Navigate, find and export diff --git a/compare_tool/qtviewer/diffpane.py b/compare_tool/qtviewer/diffpane.py index 7782a18..3a6859a 100644 --- a/compare_tool/qtviewer/diffpane.py +++ b/compare_tool/qtviewer/diffpane.py @@ -9,7 +9,12 @@ noise = the same pair dimmed, moved = blue, absent side = dim filler); the changed characters inside a line are highlighted at the exact offsets :func:`view_model.char_span` reports, so the pane and the HTML report mark -identical spans. +identical spans. Every colour comes from :mod:`compare_tool.theme`, looked up +when it is painted, so the dark/light switch is a repaint. + +A category the reviewer switches off is *muted*, not removed: the lines keep +their place and stay readable in one flat grey, and only the surfaces that +answer "where are the changes" -- the minimap and F7/F8 -- stop counting them. Foreground is the other channel: :mod:`compare_tool.syntax` colours the code itself, and the two never touch -- the diff owns background, syntax owns text. @@ -25,11 +30,11 @@ QSplitter, QStackedWidget, QTextEdit, QToolButton, QVBoxLayout, QWidget) -from .. import review +from .. import review, theme from ..scanner import looks_binary, read_text from ..syntax import language_for -from ..view_model import (Row, aligned_rows, char_span, collapse_rows, - hunk_row_starts, row_with) +from ..view_model import (MUTED, Row, aligned_rows, char_span, hunk_row_starts, + mute_rows, row_with) from .highlight import CodeHighlighter from .icons import logo_pixmap from .minimap import Minimap @@ -73,7 +78,9 @@ def _semantic_summary(result): chips = [c for c in chips if c] return 'AUTOSAR / A2L: ' + ' · '.join(chips) if chips else '' -# per-side row background by mode; None = context (editor base colour). +# per-side row background by mode, as theme roles; None = context (editor base +# colour). Looked up at paint time, so a theme switch is a repaint and never a +# second copy of this table. # # One colour language: removed is red, added is green, on every category. Noise # (comment banners, UUID churn, renames) used to get purple and yellow of its @@ -82,44 +89,39 @@ def _semantic_summary(result): # at a glance. Noise is the SAME red/green, one notch dimmer: the reviewer still # has to see which hunks inside a Modified file are the ones that count. _ROW_BG = { - ('real', 'old'): '#3a2222', ('real', 'new'): '#1f3a24', - ('comment', 'old'): '#2f2020', ('comment', 'new'): '#1e2f21', - ('minor', 'old'): '#2f2020', ('minor', 'new'): '#1e2f21', - ('moved', 'old'): '#1d2f3e', ('moved', 'new'): '#1d2f3e', - # a folded run of noise: a flat strip, no diff colour -- it stands for - # lines the current compare rules say are not a difference at all - ('folded', 'old'): '#26272b', ('folded', 'new'): '#26272b', + ('real', 'old'): 'del-bg', ('real', 'new'): 'add-bg', + ('comment', 'old'): 'del-bg-dim', ('comment', 'new'): 'add-bg-dim', + ('minor', 'old'): 'del-bg-dim', ('minor', 'new'): 'add-bg-dim', + ('moved', 'old'): 'mv-bg', ('moved', 'new'): 'mv-bg', + # a muted row -- a category the reviewer switched off -- keeps its code but + # loses its diff colour: one flat grey, same on both sides, so the eye + # passes over it on the way to the change that still counts + (MUTED, 'old'): 'muted-bg', (MUTED, 'new'): 'muted-bg', } -# a folded placeholder is not code and gets no diff colour; its text says what -# was folded, so the colour does not have to -_FOLD_FG = {'comment': '#8f96a2', 'other': '#8f96a2'} -# inline changed-span background by mode/side +# inline changed-span background by mode/side. No entry for MUTED on purpose: +# marking the changed characters inside a line the rules no longer report would +# undo the whole point of playing it down. _SEG_BG = { - ('real', 'old'): '#7a2f2f', ('real', 'new'): '#2f6e3d', - ('comment', 'old'): '#5e2a2a', ('comment', 'new'): '#2c5738', - ('minor', 'old'): '#5e2a2a', ('minor', 'new'): '#2c5738', - ('moved', 'old'): '#2f5a7a', ('moved', 'new'): '#2f5a7a', + ('real', 'old'): 'seg-del-bg', ('real', 'new'): 'seg-add-bg', + ('comment', 'old'): 'seg-del-dim-bg', ('comment', 'new'): 'seg-add-dim-bg', + ('minor', 'old'): 'seg-del-dim-bg', ('minor', 'new'): 'seg-add-dim-bg', + ('moved', 'old'): 'seg-mv-bg', ('moved', 'new'): 'seg-mv-bg', } -# translucent overlay marking the change the reviewer is currently on, so -# 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, +_ZOOM_MIN, _ZOOM_MAX = 6, 24 # point size clamp for Ctrl+wheel zoom + +# The find hits are 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' -_NEW_ACCENT = '#8ec69a' -_FILLER_BG = '#26272b' # the absent side of an insert/delete -_ADD_BG = '#1f3a24' -_DEL_BG = '#3a2222' -_ZOOM_MIN, _ZOOM_MAX = 6, 24 # point size clamp for Ctrl+wheel zoom -_BASE_BG = '#232427' +# are visible too. (Roles: find-bg / find-cur-bg.) +# +# 'cur-row' is the translucent wash over the change the reviewer is on, so +# F7/F8 are visibly doing something even when the file fits on screen and there +# is nothing to scroll. + + +def _qc(role): + return QColor(theme.c(role)) class _Gutter(QWidget): @@ -162,14 +164,18 @@ def __init__(self): f = QFont('Consolas', 10) f.setStyleHint(QFont.Monospace) self.setFont(f) - self.setStyleSheet('QPlainTextEdit{{background:{};color:#d4d4d4;' - 'border:none;}}'.format(_BASE_BG)) + self.apply_theme() self._nos = [] # per block: line-number string ('' for padding) self._gutter = _Gutter(self) self.blockCountChanged.connect(lambda _n: self._update_gutter_width()) self.updateRequest.connect(self._on_update_request) self._update_gutter_width() + def apply_theme(self): + self.setStyleSheet('QPlainTextEdit{{background:{};color:{};' + 'border:none;}}'.format(theme.c('code-bg'), + theme.c('code-fg'))) + def wheelEvent(self, event): if event.modifiers() & Qt.ControlModifier: self.zoomStep.emit(1 if event.angleDelta().y() > 0 else -1) @@ -213,11 +219,11 @@ def resizeEvent(self, event): def paint_gutter(self, event): painter = QPainter(self._gutter) - painter.fillRect(event.rect(), QColor('#1e1f22')) + painter.fillRect(event.rect(), _qc('gutter-bg')) block = self.firstVisibleBlock() top = self.blockBoundingGeometry(block).translated(self.contentOffset()).top() bottom = top + self.blockBoundingRect(block).height() - painter.setPen(QColor('#6a6a6a')) + painter.setPen(_qc('gutter-fg')) h = self.fontMetrics().height() while block.isValid() and top <= event.rect().bottom(): if block.isVisible() and bottom >= event.rect().top(): @@ -254,7 +260,6 @@ def __init__(self): self._msg = QLabel(_HINT) self._msg.setAlignment(Qt.AlignCenter) self._msg.setWordWrap(True) - self._msg.setStyleSheet('color:#b9b9b9; font-size:13px;') msg_page = QWidget() ml = QVBoxLayout(msg_page) ml.setSpacing(18) @@ -264,7 +269,6 @@ def __init__(self): ml.addStretch(1) self._header = QLabel('') - self._header.setStyleSheet('color:#e8e8e8; font-weight:bold;') # navigation lives in THIS row, beside the file name it steps through -- # not in a bar of its own at the bottom, which repeated the same # "change k of N" this header already carries for four small buttons' @@ -280,7 +284,6 @@ def __init__(self): head_row.addLayout(self.nav_actions) self._sem = QLabel('') self._sem.setWordWrap(True) - self._sem.setStyleSheet('color:#9a9a9a; padding:0 10px 6px; font-size:12px;') self._sem.setVisible(False) self.old_edit = DiffEditor() self.new_edit = DiffEditor() @@ -294,8 +297,10 @@ def __init__(self): # the right, so which side is which is unmistakable at a glance. Each # banner is wrapped INTO the splitter pane, so it tracks the split when # the reviewer drags the divider. - self._old_name = self._pane_banner(_OLD_ACCENT) - self._new_name = self._pane_banner(_NEW_ACCENT) + self._old_name = QLabel('') + self._new_name = QLabel('') + for lbl in (self._old_name, self._new_name): + lbl.setTextInteractionFlags(Qt.TextSelectableByMouse) self._split = QSplitter(Qt.Horizontal) self._split.addWidget(self._pane(self._old_name, self.old_edit)) self._split.addWidget(self._pane(self._new_name, self.new_edit)) @@ -333,9 +338,12 @@ def __init__(self): self.rows = [] self._stops = [] # first row of each reviewable change block - self._fold = () # row modes folded out of the panes + self._muted = () # row modes played down in the panes self._units = [] # review.Unit per change, same order as _stops self._rel = None # file currently shown + # what show_file was last called with, so a theme switch can re-render + # the same file from the same arguments instead of half-repainting it + self._last = None self._old_label = None # (text, tooltip) when OLD is not a folder self._cur_idx = 0 # which change (index into _stops / _units) self._head_base = '' # header without the "change k of N" suffix @@ -358,17 +366,50 @@ def __init__(self): 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): - # neutral dark strip, coloured only in the OLD/NEW tag text and a thin + self._style_widgets() + + # --- theme --- + + def _style_widgets(self): + """Every stylesheet this pane sets by hand, in one place, so a theme + switch is one call rather than a hunt through the constructor.""" + self._msg.setStyleSheet('color:{}; font-size:13px;' + .format(theme.c('fg-dim'))) + self._header.setStyleSheet('color:{}; font-weight:bold;' + .format(theme.c('fg-strong'))) + self._sem.setStyleSheet('color:{}; padding:0 10px 6px; font-size:12px;' + .format(theme.c('fg-dim'))) + self._find_count.setStyleSheet('color:{}; font-size:12px;' + .format(theme.c('st-ign'))) + # neutral strip, coloured only in the OLD/NEW tag text and a thin # underline -- a full red/green band would read as a changed diff row - lbl = QLabel('') - lbl.setStyleSheet( - 'background:#2a2c31; color:{}; padding:5px 10px; font-weight:bold; ' - 'font-size:13px; border-bottom:2px solid {};'.format(accent, accent)) - lbl.setTextInteractionFlags(Qt.TextSelectableByMouse) - return lbl + for lbl, role in ((self._old_name, 'pane-old-accent'), + (self._new_name, 'pane-new-accent')): + lbl.setStyleSheet( + 'background:{}; color:{}; padding:5px 10px; font-weight:bold; ' + 'font-size:13px; border-bottom:2px solid {};' + .format(theme.c('pane-banner-bg'), theme.c(role), + theme.c(role))) + + def apply_theme(self): + """Repaint everything this pane owns in the current theme. + + The file on screen is re-rendered from ``show_file``'s own arguments: + the row backgrounds are stamped into the document as block formats, so + a stylesheet swap alone would leave the previous theme's red and green + sitting in the code.""" + self._style_widgets() + for editor, hl in ((self.old_edit, self._hl_old), + (self.new_edit, self._hl_new)): + editor.apply_theme() + hl.apply_theme() + self.minimap.update() + if self._last is not None and self.currentIndex() == 1: + # re-rendering parks on change 1 again; put the reviewer back where + # they were reading -- a colour switch is not a navigation command + at = self._drive.verticalScrollBar().value() + self.show_file(*self._last) + self._drive.verticalScrollBar().setValue(at) @staticmethod def _pane(banner, editor): @@ -404,7 +445,6 @@ def _build_find_bar(self): 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)') @@ -525,7 +565,7 @@ def _mark_matches(self): 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 + colour = _qc('find-cur-bg' if row == cur else 'find-bg') at = line.find(needle) while at >= 0: sel = QTextEdit.ExtraSelection() @@ -563,16 +603,16 @@ def _set_pane_names(self, old_root, new_root): """Name each pane by its folder: a coloured BASELINE/CURRENT tag then the folder name, bright, with the full path as a tooltip. Called wherever a file is shown, so the two roots are in hand.""" - for lbl, root, tag, accent in ( - (self._old_name, old_root, 'BASELINE', _OLD_ACCENT), - (self._new_name, new_root, 'CURRENT', _NEW_ACCENT)): + for lbl, root, tag, role in ( + (self._old_name, old_root, 'BASELINE', 'pane-old-accent'), + (self._new_name, new_root, 'CURRENT', 'pane-new-accent')): p = Path(root) name, tip = p.name or str(p), str(p) if tag == 'BASELINE' and self._old_label: name, tip = self._old_label[0], self._old_label[1] or str(p) lbl.setText('{}' - '  ·  {}' - .format(accent, tag, name)) + '  ·  {}' + .format(theme.c(role), tag, theme.c('fg-strong'), name)) lbl.setToolTip(tip) # --- scroll sync: equal block counts make it a straight mirror --- @@ -608,12 +648,18 @@ def _zoom_by(self, step): # --- public seam --- - def set_fold_modes(self, modes): - """Row modes to fold out of the panes -- the same categories the - compare rules stop reporting. Unticking `Unimportant` should not leave - a wall of yellow lines in the code: if those differences do not count, - showing them is noise. Takes effect on the next ``show_file``.""" - self._fold = tuple(modes) + def set_muted_modes(self, modes): + """Row modes to play down in the panes -- the same categories the + compare rules stop reporting. + + Unticking `Unimportant` should not leave a wall of red and green in the + code claiming to be changes; but taking those lines away costs the + reviewer the context the surviving hunks are read in, and a regenerated + file is mostly banner churn. So they are greyed, not removed: still + readable, no diff colour, and gone from the minimap and from F7/F8. + + Takes effect on the next ``show_file``.""" + self._muted = tuple(modes) def clear(self): self._logo.setVisible(False) @@ -627,6 +673,7 @@ def clear(self): def _forget_units(self): self._rel = None + self._last = None self._units = [] self._cur_idx = 0 self.unitChanged.emit() @@ -652,6 +699,7 @@ def show_file(self, rel, result, old_root, new_root): self._rel = rel self._units = [] self._cur_idx = 0 + self._last = (rel, result, old_root, new_root) try: self._show_file(rel, result, old_root, new_root) except Exception as e: @@ -739,10 +787,10 @@ def _show_file(self, rel, result, old_root, new_root): # come from the hunks, so what a note is attached to never depends on # which categories happen to be folded on screen self._units = review.units_of(result, old_p, new_p, old_lines, new_lines) - self.rows, row_map = collapse_rows(rows, self._fold) - self._load_rows(rel, status, result, row_map) + self.rows = mute_rows(rows, self._muted) + self._load_rows(rel, status, result) - def _load_rows(self, rel, status, result=None, row_map=None): + def _load_rows(self, rel, status, result=None): rows = self.rows n_moved = sum(1 for r in rows if r.mode == 'moved') # header names the file only -- the verdict (real-change / identical / @@ -777,37 +825,30 @@ def _load_rows(self, rel, status, result=None, row_map=None): for i, r in enumerate(rows): if r.mode == 'ctx': continue - if r.mode == 'folded': - fg = _FOLD_FG['comment' if r.kind == 'comment' else 'other'] - for editor in (self.old_edit, self.new_edit): - self._block_bg(editor, i, _ROW_BG[('folded', 'old')]) - self._block_fg(editor, i, fg) - continue # old side if r.old_txt is None: - self._block_bg(self.old_edit, i, _FILLER_BG) + self._block_bg(self.old_edit, i, theme.c('filler-bg')) else: - self._block_bg(self.old_edit, i, _ROW_BG.get((r.mode, 'old'))) + self._block_bg(self.old_edit, i, self._bg(r.mode, 'old')) # new side if r.new_txt is None: - self._block_bg(self.new_edit, i, _FILLER_BG) + self._block_bg(self.new_edit, i, theme.c('filler-bg')) else: - self._block_bg(self.new_edit, i, _ROW_BG.get((r.mode, 'new'))) - # inline highlight only when both sides present - if r.old_txt is not None and r.new_txt is not None: + self._block_bg(self.new_edit, i, self._bg(r.mode, 'new')) + # inline highlight only when both sides present, and never on a + # muted row: there is no _SEG_BG entry for it, so this stays quiet + if r.mode != MUTED and r.old_txt is not None and r.new_txt is not None: (o_lo, o_hi), (n_lo, n_hi) = char_span(r.old_txt, r.new_txt) - self._seg_bg(self.old_edit, i, o_lo, o_hi, _SEG_BG.get((r.mode, 'old'))) - self._seg_bg(self.new_edit, i, n_lo, n_hi, _SEG_BG.get((r.mode, 'new'))) + self._seg_bg(self.old_edit, i, o_lo, o_hi, self._seg(r.mode, 'old')) + self._seg_bg(self.new_edit, i, n_lo, n_hi, self._seg(r.mode, 'new')) # navigation stops: the first row of each reviewable change, one per # hunk. Deriving them from the hunk list rather than from runs of # coloured rows is what makes "change 3 of 7", the hunk count the CLI # prints and the units a review note attaches to all the same thing. + # Muting never moves a row, so these indices need no translation -- + # and a muted category is absent from _units anyway, so F7/F8 skip it. starts = hunk_row_starts((result or {}).get('hunks') or []) - # the starts are positions in the UNFOLDED layout; row_map carries them - # over. Real and moved rows are never folded, so a stop always lands on - # the change itself and never inside a placeholder. - self._stops = [row_map[starts[u.index]] if row_map else starts[u.index] - for u in self._units + self._stops = [starts[u.index] for u in self._units if u.index is not None and u.index < len(starts)] self._cur_idx = 0 self.setCurrentIndex(1) @@ -838,9 +879,9 @@ def _load_one_side(self, rel, label, lines, side): self._header.setText(self._head_base) edit = self.old_edit if side == 'old' else self.new_edit other = self.new_edit if side == 'old' else self.old_edit - bg = _DEL_BG if side == 'old' else _ADD_BG - # a whole added/deleted file is all one mode, so there are no folded - # placeholders to skip -- the language is the only thing to pass on + bg = theme.c('del-bg' if side == 'old' else 'add-bg') + # a whole added/deleted file is all one mode, so there are no muted + # rows to grey out -- the language is the only thing to pass on lang = language_for(rel) self._hl_old.configure(lang, (), repaint=False) self._hl_new.configure(lang, (), repaint=False) @@ -857,6 +898,16 @@ def _load_one_side(self, rel, label, lines, side): self.minimap.set_rows(self.rows) self.setCurrentIndex(1) + @staticmethod + def _bg(mode, side): + role = _ROW_BG.get((mode, side)) + return theme.c(role) if role else None + + @staticmethod + def _seg(mode, side): + role = _SEG_BG.get((mode, side)) + return theme.c(role) if role else None + @staticmethod def _set_text(editor, text): """Replace an editor's contents, caret formatting reset first. @@ -880,14 +931,6 @@ def _block_bg(self, editor, block_no, color): fmt.setBackground(QColor(color)) cursor.setBlockFormat(fmt) - def _block_fg(self, editor, block_no, color): - block = editor.document().findBlockByNumber(block_no) - cursor = QTextCursor(block) - cursor.select(QTextCursor.BlockUnderCursor) - fmt = QTextCharFormat() - fmt.setForeground(QColor(color)) - cursor.mergeCharFormat(fmt) - def _seg_bg(self, editor, block_no, lo, hi, color): if not color or lo >= hi: return @@ -952,7 +995,7 @@ def _highlight_block(self, row): continue for i in range(start, end + 1): sel = QTextEdit.ExtraSelection() - sel.format.setBackground(_CUR_BG) + sel.format.setBackground(_qc('cur-row')) sel.format.setProperty(QTextFormat.FullWidthSelection, True) cur = QTextCursor(editor.document().findBlockByNumber(i)) cur.clearSelection() diff --git a/compare_tool/qtviewer/highlight.py b/compare_tool/qtviewer/highlight.py index 4d7049a..cdf0058 100644 --- a/compare_tool/qtviewer/highlight.py +++ b/compare_tool/qtviewer/highlight.py @@ -10,25 +10,27 @@ **No red, no green.** Those two mean removed and added here. The palette is blue / teal / amber / lilac, muted enough to stay legible on the red and green -row fills rather than competing with them. +row fills rather than competing with them. The actual values live in +:mod:`compare_tool.theme` -- one per role per theme -- so switching to the light +page does not need a second copy of this mapping. """ from PySide6.QtGui import QColor, QSyntaxHighlighter, QTextCharFormat -from .. import syntax +from .. import syntax, theme -# token kind -> (colour, italic). Deliberately low-saturation: these sit on top -# of diff fills, and a bright token colour there reads as another change. +# token kind -> (theme role, italic). Deliberately low-saturation: these sit on +# top of diff fills, and a bright token colour there reads as another change. _PALETTE = { - syntax.COMMENT: ('#8f96a2', True), - syntax.STRING: ('#e0a860', False), - syntax.NUMBER: ('#c5a3e8', False), - syntax.KEYWORD: ('#7aa2e3', False), - syntax.TYPE: ('#57b6a9', False), - syntax.PREPROC: ('#b58ac4', False), - syntax.CALL: ('#d8c99a', False), - syntax.TAG: ('#7aa2e3', False), - syntax.ATTR: ('#57b6a9', False), + syntax.COMMENT: ('syn-comment', True), + syntax.STRING: ('syn-string', False), + syntax.NUMBER: ('syn-number', False), + syntax.KEYWORD: ('syn-keyword', False), + syntax.TYPE: ('syn-type', False), + syntax.PREPROC: ('syn-preproc', False), + syntax.CALL: ('syn-call', False), + syntax.TAG: ('syn-tag', False), + syntax.ATTR: ('syn-attr', False), } # above this the pane stays plain: rehighlighting runs on the GUI thread, and a @@ -38,30 +40,43 @@ def _formats(): out = {} - for kind, (colour, italic) in _PALETTE.items(): + for kind, (role, italic) in _PALETTE.items(): fmt = QTextCharFormat() - fmt.setForeground(QColor(colour)) + fmt.setForeground(QColor(theme.c(role))) if italic: fmt.setFontItalic(True) out[kind] = fmt return out +def _muted_format(): + """One flat grey for a whole muted line. + + A row the compare rules no longer report is still on screen, and syntax + colour on it would undo the point of playing it down -- so the code channel + goes quiet too, not just the diff background.""" + fmt = QTextCharFormat() + fmt.setForeground(QColor(theme.c('muted-fg'))) + return fmt + + class CodeHighlighter(QSyntaxHighlighter): - """Colours one editor's document, skipping rows that are not code. - - ``modes`` is the row-mode list the pane is showing, in block order. Its - only job is to spot ``folded`` placeholders (`⋯ 12 uuid lines hidden`): - those are not code, and -- more importantly -- the lines they stand for are - gone, so a `/*` whose `*/` was folded away would otherwise turn every line - below into a comment. A fold therefore resets the state: worst case a real - comment loses its colour, which is cosmetic, where the other direction - looks like a finding. + """Colours one editor's document, playing down rows that do not count. + + ``modes`` is the row-mode list the pane is showing, in block order. Its job + is to spot ``muted`` rows -- the noise categories the reviewer has switched + off -- and paint those in one flat grey instead of syntax colours. The line + is still there and still readable; it just stops competing with the change + beside it. + + The block state is still computed from the real text of a muted row, so a + `/* ... */` running across one keeps the lines below it correctly coloured. """ def __init__(self, document): super().__init__(document) self._fmt = _formats() + self._muted = _muted_format() self._language = None self._modes = () @@ -77,20 +92,27 @@ def configure(self, language, modes=(), repaint=True): if repaint: self.rehighlight() + def apply_theme(self): + """Re-read the palette after a theme switch and repaint.""" + self._fmt = _formats() + self._muted = _muted_format() + self.rehighlight() + def highlightBlock(self, text): - if self._language is None: - return n = self.currentBlock().blockNumber() - if n >= MAX_LINES: - return - if n < len(self._modes) and self._modes[n] == 'folded': - self.setCurrentBlockState(syntax.PLAIN) + muted = n < len(self._modes) and self._modes[n] == 'muted' + if muted and text: + # the grey has to win even with no language: a muted plain-text row + # must not keep the editor's normal foreground + self.setFormat(0, len(text), self._muted) + if self._language is None or n >= MAX_LINES: return prev = self.previousBlockState() state = prev if prev in (syntax.PLAIN, syntax.IN_BLOCK_COMMENT) else syntax.PLAIN spans, state = syntax.spans(text, self._language, state) - for start, end, kind in spans: - fmt = self._fmt.get(kind) - if fmt is not None: - self.setFormat(start, end - start, fmt) + if not muted: + for start, end, kind in spans: + fmt = self._fmt.get(kind) + if fmt is not None: + self.setFormat(start, end - start, fmt) self.setCurrentBlockState(state) diff --git a/compare_tool/qtviewer/icons.py b/compare_tool/qtviewer/icons.py index a8e4331..48ab1b0 100644 --- a/compare_tool/qtviewer/icons.py +++ b/compare_tool/qtviewer/icons.py @@ -13,11 +13,14 @@ from PySide6.QtCore import QByteArray, Qt from PySide6.QtGui import QColor, QIcon, QPainter, QPixmap +from .. import theme from ..resources import icon_file, logo_file -# icon tint on the dark chrome, and the accent used for the primary action -TINT = '#d7d7d7' -ACCENT = '#7c8cf8' +# theme roles: the icon tint, and the accent the primary action wears. Roles, +# not literals, because the tint has to flip with the chrome -- a light grey +# glyph is invisible on the light theme's toolbar. +TINT = 'icon-tint' +ACCENT = 'accent-2' _cache = {} @@ -31,8 +34,12 @@ def _tinted(pm, color): return out -def icon(name, color=TINT, size=20): - """Tinted :class:`QIcon` for a shipped glyph; empty when it is missing.""" +def icon(name, role=TINT, size=20): + """Tinted :class:`QIcon` for a shipped glyph; empty when it is missing. + + ``role`` is a theme role, resolved now -- so the same call made again after + a theme switch returns a glyph tinted for the new chrome.""" + color = theme.c(role) key = (name, color, size) if key in _cache: return _cache[key] @@ -46,14 +53,14 @@ def icon(name, color=TINT, size=20): return ico -def std_icon(widget, standard_pixmap, color=TINT, size=20): +def std_icon(widget, standard_pixmap, role=TINT, size=20): """A Qt built-in icon, tinted to match the shipped set. Windows' own icons are full colour (a blue folder, a red help ring) and would read as decorations dropped into an otherwise monochrome toolbar. """ pm = widget.style().standardIcon(standard_pixmap).pixmap(size, size) - return QIcon(_tinted(pm, color)) if not pm.isNull() else QIcon() + return QIcon(_tinted(pm, theme.c(role))) if not pm.isNull() else QIcon() def app_icon(): @@ -68,22 +75,24 @@ def app_icon(): # The shipped lockup's wordmark is near-black on transparent -- correct on a -# white page, all but invisible on this app's dark chrome. So the SVG twin of -# logo-full.png is re-rendered with a light wordmark; the gradient mark itself -# is untouched. The PNG stays the fallback when Qt's SVG module is absent. +# white page, all but invisible on the dark chrome. So on the dark theme the +# SVG twin of logo-full.png is re-rendered with a light wordmark; the gradient +# mark itself is untouched, and the light theme wants the asset as shipped. The +# PNG stays the fallback when Qt's SVG module is absent. _DARK_WORDMARK = (('fill="#0f172a"', 'fill="#e9edf5"'), ('fill="#475569"', 'fill="#9fb0c6"')) -def _dark_lockup(width): +def _svg_lockup(width): path = logo_file('logo-full.svg') if path is None: return None try: from PySide6.QtSvg import QSvgRenderer svg = path.read_text(encoding='utf-8') - for old, new in _DARK_WORDMARK: - svg = svg.replace(old, new) + if theme.current() == theme.DARK: + for old, new in _DARK_WORDMARK: + svg = svg.replace(old, new) r = QSvgRenderer(QByteArray(svg.encode('utf-8'))) if not r.isValid(): return None @@ -103,7 +112,7 @@ def _dark_lockup(width): def logo_pixmap(width=320, name='logo-full.png'): """Horizontal lockup for the landing page and the About box; None if the asset is not shipped with this install.""" - pm = _dark_lockup(width) + pm = _svg_lockup(width) if pm is not None: return pm path = logo_file(name) diff --git a/compare_tool/qtviewer/minimap.py b/compare_tool/qtviewer/minimap.py index bcb1023..326c9f4 100644 --- a/compare_tool/qtviewer/minimap.py +++ b/compare_tool/qtviewer/minimap.py @@ -14,28 +14,28 @@ from PySide6.QtGui import QColor, QPainter from PySide6.QtWidgets import QWidget +from .. import theme + _WIDTH = 92 -_BG = '#202124' _MAX_LINE_H = 4.0 # px per line at most; short files render small, VS Code-like _MAX_CHAR_W = 3.0 # px per char at most, so short lines don't stretch full width # dim token colour per mode (ctx = plain code grey); changed rows also get a # translucent full-width strip so diffs pop on the map. Noise shares the change # colour, dimmer -- same one-colour-language rule as the panes (see _ROW_BG). -_TOKEN = {'ctx': '#565b62', 'real': '#e8908d', 'comment': '#a4706e', - 'minor': '#a4706e', 'moved': '#7fb0d9', 'folded': '#4a4e55'} -# a folded placeholder is not a change on the map either: it stands for lines -# the current compare rules say are not a difference, so it gets no strip and -# collapses away with the context rows when the map is compressed -_NOT_A_CHANGE = ('ctx', 'folded') +_TOKEN = {'ctx': 'map-ctx', 'real': 'map-real', 'comment': 'map-noise', + 'minor': 'map-noise', 'moved': 'map-moved', 'muted': 'map-muted'} +# a muted row is not a change on the map either. That is the point of switching +# a category off: the lines stay readable in the panes, but the map -- the +# "where are the changes" surface -- stops pointing at them, exactly like +# F7/F8 already do. +_NOT_A_CHANGE = ('ctx', 'muted') _STRIP = { - 'real': QColor(217, 82, 79, 70), - 'comment': QColor(217, 82, 79, 40), - 'minor': QColor(217, 82, 79, 40), - 'moved': QColor(63, 127, 176, 70), + 'real': 'map-strip-real', + 'comment': 'map-strip-noise', + 'minor': 'map-strip-noise', + 'moved': 'map-strip-moved', } -_VIEW_FILL = QColor(255, 255, 255, 26) -_VIEW_BORDER = QColor(190, 190, 190, 120) def _row_text(r): @@ -92,7 +92,9 @@ def _visible_rows(self): def paintEvent(self, event): p = QPainter(self) - p.fillRect(self.rect(), QColor(_BG)) + # colours are read on every paint, not cached: a theme switch is then + # just an update() away, with no state of its own to keep in step + p.fillRect(self.rect(), QColor(theme.c('map-bg'))) rows = self._rows n = len(rows) if not n: @@ -111,8 +113,9 @@ def paintEvent(self, event): continue prev_y = y if is_change: - p.fillRect(0, y, self.width(), bh, _STRIP.get(r.mode, _STRIP['real'])) - token = QColor(_TOKEN.get(r.mode, _TOKEN['ctx'])) + p.fillRect(0, y, self.width(), bh, + QColor(theme.c(_STRIP.get(r.mode, _STRIP['real'])))) + token = QColor(theme.c(_TOKEN.get(r.mode, _TOKEN['ctx']))) self._paint_tokens(p, _row_text(r), y, cw, bh, token) self._paint_viewport(p, n, h, lh) @@ -135,8 +138,9 @@ def _paint_viewport(self, p, n, h, lh): count = self._visible_rows() y0 = int(first * lh) y1 = int(min((first + count) * lh, n * lh)) - p.fillRect(0, y0, self.width(), max(y1 - y0, 2), _VIEW_FILL) - p.setPen(_VIEW_BORDER) + p.fillRect(0, y0, self.width(), max(y1 - y0, 2), + QColor(theme.c('map-view-fill'))) + p.setPen(QColor(theme.c('map-view-border'))) p.drawRect(0, y0, self.width() - 1, max(y1 - y0 - 1, 2)) # --- interaction: click / drag scrolls the driven editor --- diff --git a/compare_tool/qtviewer/summary.py b/compare_tool/qtviewer/summary.py index 4b3e896..16ef26e 100644 --- a/compare_tool/qtviewer/summary.py +++ b/compare_tool/qtviewer/summary.py @@ -8,12 +8,14 @@ from PySide6.QtGui import QBrush, QColor from PySide6.QtWidgets import QHeaderView, QTreeWidget, QTreeWidgetItem +from .. import theme from .summary_model import summary_sections REL_ROLE = Qt.UserRole KEY_ROLE = Qt.UserRole + 1 -_SIGN_COLOR = {'+': '#7bd88a', '−': '#ff7b7b', '~': '#7fb3d9'} +# added / removed / changed, in the same colours the tree and the report use +_SIGN_COLOR = {'+': 'st-add', '−': 'st-real', '~': 'mv-fg'} _EMPTY = 'No AUTOSAR / A2L changes' @@ -34,8 +36,15 @@ def __init__(self): header.setSectionResizeMode(0, QHeaderView.ResizeToContents) header.setStretchLastSection(True) self.itemClicked.connect(self._on_click) + self._results = {} + + def apply_theme(self): + """Item colours are set per row, so a theme switch has to rebuild them + -- from the results the panel was last given, never from the widget.""" + self.set_results(self._results) def set_results(self, results): + self._results = results self.clear() sections = summary_sections(results) if results else [] if not sections: @@ -46,14 +55,14 @@ def set_results(self, results): font = head.font(0) font.setBold(True) head.setFont(0, font) - head.setForeground(0, QBrush(QColor('#dcdcaa'))) + head.setForeground(0, QBrush(QColor(theme.c('accent')))) self.addTopLevelItem(head) for row in rows: item = QTreeWidgetItem(['{} {}'.format(row.sign, row.name), row.detail]) - item.setForeground(0, QBrush(QColor(_SIGN_COLOR.get(row.sign, - '#d4d4d4')))) - item.setForeground(1, QBrush(QColor('#9a9a9a'))) + item.setForeground(0, QBrush(QColor(theme.c( + _SIGN_COLOR.get(row.sign, 'fg'))))) + item.setForeground(1, QBrush(QColor(theme.c('fg-dim')))) item.setToolTip(0, row.rel) item.setData(0, REL_ROLE, row.rel) item.setData(0, KEY_ROLE, row.key) diff --git a/compare_tool/qtviewer/tree.py b/compare_tool/qtviewer/tree.py index 90d3c56..84cf293 100644 --- a/compare_tool/qtviewer/tree.py +++ b/compare_tool/qtviewer/tree.py @@ -7,30 +7,45 @@ from collections import namedtuple -# status -> (tree marker, display label, hex colour). Mirrors the HTML +from .. import theme + +# status -> (tree marker, display label, theme role). Mirrors the HTML # report's verdict vocabulary (Modified / Unimportant / Added / Deleted / -# Identical) and its colours so the viewer and the report read the same. +# Identical) and reads its colours from the SAME roles the report's CSS does, +# so the viewer and the report cannot disagree about what Modified looks like +# -- in either theme. STATUS = { - 'real-change': ('≠', 'Modified', '#ff7b7b'), # not-equal sign + 'real-change': ('≠', 'Modified', 'st-real'), # not-equal sign # the two noise verdicts are grey on purpose: grey is what "this does not # count" looks like, and it keeps red/green meaning removed/added only - 'comment-only': ('≉', 'Comment', '#8f96a2'), # comments only - 'ignorable-only': ('≈', 'Unimportant', '#9aa1ad'), # almost-equal - 'added': ('+', 'Added', '#7bd88a'), - 'deleted': ('−', 'Deleted', '#c88ad8'), # minus sign - 'identical': ('=', 'Identical', '#8a8a8a'), - 'error': ('!', 'NOT compared', '#ff5c5c'), + 'comment-only': ('≉', 'Comment', 'st-cmt'), # comments only + 'ignorable-only': ('≈', 'Unimportant', 'st-ign'), # almost-equal + 'added': ('+', 'Added', 'st-add'), + 'deleted': ('−', 'Deleted', 'st-del'), # minus sign + 'identical': ('=', 'Identical', 'st-id'), + 'error': ('!', 'NOT compared', 'st-err'), } + +def status_color(status): + """The current theme's colour for a verdict.""" + return theme.c(STATUS[status][2]) + # folder verdict = most significant child verdict; an uncompared 'error' path # outranks everything so a folder hiding one can never look clean PRIO = {'error': 6, 'real-change': 5, 'ignorable-only': 4, 'comment-only': 3, 'added': 2, 'deleted': 2, 'identical': 1} -# review progress -> colour, for the tree column that only exists in review +# review progress -> theme role, for the tree column that only exists in review # mode. Same three colours as the status chip on the status bar: green is a # finished state, amber is in flight, grey is nothing yet. -REVIEW_COLOR = {'done': '#7bd88a', 'partial': '#e2c16b', 'none': '#8a8f98'} +REVIEW_COLOR = {'done': 'review-done', 'partial': 'review-partial', + 'none': 'review-none'} + + +def review_color(state): + """The current theme's colour for a review-progress state.""" + return theme.c(REVIEW_COLOR[state]) def review_state(reviewed, total): diff --git a/compare_tool/report.py b/compare_tool/report.py index cad9d13..7af55b0 100644 --- a/compare_tool/report.py +++ b/compare_tool/report.py @@ -1,11 +1,17 @@ -"""Self-contained HTML report. Summary badges toggle each change category.""" +"""Self-contained HTML report. Summary badges toggle each change category. + +Every colour comes from :mod:`compare_tool.theme` as a CSS custom property, and +BOTH palettes are written into the page -- so the reader's dark/light button is +an attribute flip with nothing to fetch, on a machine with no internet, which is +where these reports are usually opened. +""" import datetime import html import re from pathlib import Path -from . import review +from . import review, theme from .diff_engine import ruleset_for from .scanner import (looks_binary, read_text, summarize, summarize_a2l, summarize_ifaces, summarize_rte, summarize_swcs) @@ -15,82 +21,96 @@ MAX_CONTENT = 400 # max lines shown for added/deleted file content _CSS = """ -body { font-family: Segoe UI, Arial, sans-serif; background: #1e1f22; color: #d4d4d4; +body { font-family: Segoe UI, Arial, sans-serif; background: var(--bg); color: var(--fg); margin: 0; padding: 24px; } -h1 { font-size: 20px; } h2 { font-size: 15px; margin: 28px 0 6px; color: #e8e8e8; } -.meta { color: #9a9a9a; font-size: 13px; margin-bottom: 4px; } +h1 { font-size: 20px; } h2 { font-size: 15px; margin: 28px 0 6px; color: var(--fg-strong); } +.meta { color: var(--fg-dim); font-size: 13px; margin-bottom: 4px; } .summary { margin: 14px 0 22px; } .badge { display: inline-block; padding: 2px 10px; border-radius: 10px; font-size: 12px; margin-right: 8px; cursor: pointer; user-select: none; border: 1px solid transparent; } -.badge:hover { border-color: #888; } +.badge:hover { border-color: var(--border-strong); } .badge.off { opacity: .35; text-decoration: line-through; } -.b-real { background: #6e2b2b; color: #ffb3b3; } .b-ign { background: #3a3b40; color: #c3c7cd; } -.b-id { background: #333; color: #aaa; } +.b-real { background: var(--tag-real-bg); color: var(--tag-real-fg); } +.b-ign { background: var(--tag-ign-bg); color: var(--tag-ign-fg); } +.b-id { background: var(--tag-id-bg); color: var(--tag-id-fg); } /* added and deleted share one control: both are "a whole file appeared or vanished", and a reviewer flips them together */ -.b-adddel { background: #33404a; color: #cfe0ec; } -.bgroup + .bgroup { border-left: 1px solid #43454c; margin-left: 4px; padding-left: 18px; } -.b-err { background: #7a1f1f; color: #ffc2c2; border-color: #b04a4a; cursor: default; } -.b-ok { background: #2b5232; color: #a8e6b0; cursor: default; } -.errbox { background: #4a1d1d; border: 1px solid #b04a4a; border-radius: 6px; - padding: 10px 14px; margin: 14px 0 20px; color: #ffd6d6; font-size: 13px; } +.b-adddel { background: var(--tag-adddel-bg); color: var(--tag-adddel-fg); } +.bgroup + .bgroup { border-left: 1px solid var(--border-strong); margin-left: 4px; + padding-left: 18px; } +.b-err { background: var(--tag-err-bg); color: var(--tag-err-fg); + border-color: var(--err-border); cursor: default; } +.b-ok { background: var(--tag-add-bg); color: var(--tag-add-fg); cursor: default; } +.errbox { background: var(--err-bg); border: 1px solid var(--err-border); border-radius: 6px; + padding: 10px 14px; margin: 14px 0 20px; color: var(--err-fg); font-size: 13px; } .errbox .errtitle { font-weight: 700; font-size: 14px; margin-bottom: 6px; } .errbox div { padding: 1px 0; } -.errbox code { background: #5c2626; } -.hint { color: #7a7a7a; font-size: 11px; margin: -14px 0 18px; } +.errbox code { background: var(--err-code-bg); color: var(--err-fg); } +.hint { color: var(--fg-faint); font-size: 11px; margin: -14px 0 18px; } body.hide-real .sec-real, body.hide-ign .sec-ign, body.hide-add .sec-add, body.hide-del .sec-del { display: none; } ul.files { margin: 4px 0 14px; padding-left: 22px; font-size: 13px; } ul.files li { margin: 2px 0; } -.kinds { color: #8a8a8a; font-size: 12px; } -.tree { font-family: Consolas, monospace; font-size: 13px; background: #232427; - border: 1px solid #333; border-radius: 6px; padding: 10px 14px; margin: 0 0 20px; } +.kinds { color: var(--fg-muted); font-size: 12px; } +.tree { font-family: Consolas, monospace; font-size: 13px; background: var(--panel); + border: 1px solid var(--border); border-radius: 6px; padding: 10px 14px; margin: 0 0 20px; } .tree details.dir > summary { cursor: pointer; list-style: none; padding: 1px 0; - user-select: none; color: #dcdcaa; } + user-select: none; color: var(--accent); } .tree details.dir > summary::-webkit-details-marker { display: none; } -.tree details.dir > summary::before { content: '▸ '; color: #8a8a8a; } +.tree details.dir > summary::before { content: '▸ '; color: var(--fg-muted); } .tree details.dir[open] > summary::before { content: '▾ '; } .tree details.dir > *:not(summary) { margin-left: 18px; } .tf { padding: 1px 0; } -.tf a { color: inherit; text-decoration: none; border-bottom: 1px dotted #666; cursor: pointer; } -.tf a:hover { color: #fff; } +.tf a { color: inherit; text-decoration: none; border-bottom: 1px dotted var(--link-underline); + cursor: pointer; } +.tf a:hover { color: var(--link-hover); } .tmark { display: inline-block; width: 14px; font-weight: bold; } -.t-real { color: #ff7b7b; } .t-ign { color: #9aa1ad; } .t-add { color: #7bd88a; } -.t-del { color: #c88ad8; } .t-id { color: #777; } .t-err { color: #ff5c5c; } -.t-cmt { color: #8f96a2; } -.tf.tc-cmt { color: #b9bec6; } -.tf.tc-real { color: #ffb3b3; } .tf.tc-ign { color: #c3c7cd; } .tf.tc-add { color: #a8e6b0; } -.tf.tc-del { color: #d9a8e6; text-decoration: line-through; } .tf.tc-id { color: #8a8a8a; } -.tf.tc-err { color: #ffb3b3; font-weight: 700; } -.legend { color: #8a8a8a; font-size: 12px; margin: 2px 0 8px; } +.t-real { color: var(--st-real); } .t-ign { color: var(--st-ign); } +.t-add { color: var(--st-add); } +.t-del { color: var(--st-del); } .t-id { color: var(--st-id); } +.t-err { color: var(--st-err); } +.t-cmt { color: var(--st-cmt); } +.tf.tc-cmt { color: var(--st-cmt-text); } +.tf.tc-real { color: var(--tag-real-fg); } .tf.tc-ign { color: var(--tag-ign-fg); } +.tf.tc-add { color: var(--tag-add-fg); } +.tf.tc-del { color: var(--tag-del-fg); text-decoration: line-through; } +.tf.tc-id { color: var(--st-id); } +.tf.tc-err { color: var(--tag-real-fg); font-weight: 700; } +.legend { color: var(--fg-muted); font-size: 12px; margin: 2px 0 8px; } table.diff { border-collapse: collapse; width: 100%; table-layout: fixed; font-family: Consolas, monospace; font-size: 12px; margin: 6px 0 14px; } table.diff td { padding: 1px 6px; vertical-align: top; white-space: pre-wrap; word-break: break-all; border: none; } -td.ln { width: 44px; color: #6a6a6a; text-align: right; user-select: none; } -td.del { background: #3a2222; } td.add { background: #1f3a24; } +td.ln { width: 44px; color: var(--ln-fg); text-align: right; user-select: none; } +td.del { background: var(--del-bg); } td.add { background: var(--add-bg); } /* Noise (comment, uuid, rename, whitespace) uses the SAME red/green as a real change, one notch dimmer -- one colour language instead of three. Yellow and purple were a third and fourth hue competing with the syntax colours for the reader's attention, and a diff that needs a legend to be read is too loud. Dimmer, not identical: inside a Modified file the reviewer still has to see which hunks are the ones that count. */ -td.delm, td.delc { background: #2f2020; } -td.addm, td.addc { background: #1e2f21; } -td.mvd, td.mva { background: #1d2f3e; } -td.ctx { color: #9a9a9a; } -td.del .chg-seg { background: #7a2f2f; color: #ffc2c2; font-weight: 700; border-radius: 2px; } -td.add .chg-seg { background: #2f6e3d; color: #c9f7d1; font-weight: 700; border-radius: 2px; } -td.delm .chg-seg, td.delc .chg-seg { background: #5e2a2a; color: #f0c4c4; font-weight: 700; +td.delm, td.delc { background: var(--del-bg-dim); } +td.addm, td.addc { background: var(--add-bg-dim); } +td.mvd, td.mva { background: var(--mv-bg); } +td.ctx { color: var(--fg-dim); } +td.del .chg-seg { background: var(--seg-del-bg); color: var(--seg-del-fg); font-weight: 700; + border-radius: 2px; } +td.add .chg-seg { background: var(--seg-add-bg); color: var(--seg-add-fg); font-weight: 700; + border-radius: 2px; } +td.delm .chg-seg, td.delc .chg-seg { background: var(--seg-del-dim-bg); + color: var(--seg-del-dim-fg); font-weight: 700; border-radius: 2px; } -td.addm .chg-seg, td.addc .chg-seg { background: #2c5738; color: #bfe8c8; font-weight: 700; +td.addm .chg-seg, td.addc .chg-seg { background: var(--seg-add-dim-bg); + color: var(--seg-add-dim-fg); font-weight: 700; border-radius: 2px; } .sw { display: inline-block; width: 10px; height: 10px; border-radius: 2px; margin: 0 4px 0 2px; vertical-align: -1px; } -.sw-del { background: #7a2f2f; } .sw-add { background: #2f6e3d; } -.sw-mv { background: #2f5a7a; } -tr.gap td { text-align: center; color: #666; background: #26272b; font-size: 11px; } -tr.mvnote td { text-align: center; color: #7fb3d9; background: #26272b; font-size: 11px; } +.sw-del { background: var(--seg-del-bg); } .sw-add { background: var(--seg-add-bg); } +.sw-mv { background: var(--seg-mv-bg); } +tr.gap td { text-align: center; color: var(--gap-fg); background: var(--panel-2); + font-size: 11px; } +tr.mvnote td { text-align: center; color: var(--mv-fg); background: var(--panel-2); + font-size: 11px; } body.hide-ign tr.minor, body.hide-ign .grp-min { display: none; } tr.minorph { display: none; } body.hide-ign tr.minorph { display: table-row; } @@ -101,86 +121,141 @@ silently dropped. (The viewer still shows them in full -- it is the reading surface, this is the record to send.) */ tr.comment, .grp-cmt { display: none; } -tr.commentph { display: table-row; color: #8f96a2; } -tr.minorph td { color: #8f96a2; } -.filenote { color: #8a8a8a; font-size: 12px; margin: 2px 0 10px; } -.renames { font-size: 12px; color: #9aa1ad; margin: 2px 0 8px; } -.iflist { font-family: Consolas, monospace; font-size: 13px; background: #232427; - border: 1px solid #333; border-radius: 6px; padding: 10px 14px; margin: 0 0 20px; } +tr.commentph { display: table-row; color: var(--muted-fg); } +tr.minorph td { color: var(--muted-fg); } +.filenote { color: var(--fg-muted); font-size: 12px; margin: 2px 0 10px; } +.renames { font-size: 12px; color: var(--st-ign); margin: 2px 0 8px; } +.iflist { font-family: Consolas, monospace; font-size: 13px; background: var(--panel); + border: 1px solid var(--border); border-radius: 6px; padding: 10px 14px; + margin: 0 0 20px; } .iflist div { padding: 1px 0; } -.if-add { color: #7bd88a; } .if-del { color: #ff7b7b; } -.iflist a { color: #9a9a9a; text-decoration: none; border-bottom: 1px dotted #666; - cursor: pointer; } -.iflist a:hover { color: #fff; } -.ifnote { font-size: 12px; color: #7fb3d9; margin: 2px 0 8px; } -code { background: #2b2c30; padding: 1px 5px; border-radius: 4px; } -details.file { margin: 10px 0; border: 1px solid #333; border-radius: 6px; background: #232427; } +.if-add { color: var(--st-add); } .if-del { color: var(--st-real); } +.iflist a { color: var(--fg-dim); text-decoration: none; + border-bottom: 1px dotted var(--link-underline); cursor: pointer; } +.iflist a:hover { color: var(--link-hover); } +.ifnote { font-size: 12px; color: var(--mv-fg); margin: 2px 0 8px; } +code { background: var(--panel-3); padding: 1px 5px; border-radius: 4px; } +details.file { margin: 10px 0; border: 1px solid var(--border); border-radius: 6px; + background: var(--panel); } details.file > summary { list-style: none; cursor: pointer; padding: 10px 14px; - font-size: 15px; color: #e8e8e8; display: flex; align-items: center; gap: 10px; user-select: none; } + font-size: 15px; color: var(--fg-strong); display: flex; align-items: center; gap: 10px; + user-select: none; } details.file > summary::-webkit-details-marker { display: none; } -details.file > summary::before { content: '▶'; font-size: 10px; color: #8a8a8a; transition: transform .15s; } +details.file > summary::before { content: '▶'; font-size: 10px; color: var(--fg-muted); + transition: transform .15s; } details.file[open] > summary::before { transform: rotate(90deg); } -details.file > summary:hover { background: #2a2b2f; } +details.file > summary:hover { background: var(--panel-hover); } details.file > .body { padding: 0 14px 12px; } -summary .hcount { color: #8a8a8a; font-size: 12px; font-weight: normal; } +summary .hcount { color: var(--fg-muted); font-size: 12px; font-weight: normal; } summary .tag { display: inline-block; padding: 1px 8px; border-radius: 8px; font-size: 11px; } -.tag-real { background: #6e2b2b; color: #ffb3b3; } .tag-ign { background: #3a3b40; color: #c3c7cd; } -.tag-cmt { background: #33353a; color: #b9bec6; } -.tag-add { background: #2b5232; color: #a8e6b0; } .tag-del { background: #4a2b52; color: #d9a8e6; } -.tag-err { background: #7a1f1f; color: #ffc2c2; } -.hunklabel { color: #9aa1ad; font-size: 11px; margin: 10px 0 0; text-transform: uppercase; +.tag-real { background: var(--tag-real-bg); color: var(--tag-real-fg); } +.tag-ign { background: var(--tag-ign-bg); color: var(--tag-ign-fg); } +.tag-cmt { background: var(--tag-cmt-bg); color: var(--tag-cmt-fg); } +.tag-add { background: var(--tag-add-bg); color: var(--tag-add-fg); } +.tag-del { background: var(--tag-del-bg); color: var(--tag-del-fg); } +.tag-err { background: var(--tag-err-bg); color: var(--tag-err-fg); } +.hunklabel { color: var(--st-ign); font-size: 11px; margin: 10px 0 0; text-transform: uppercase; letter-spacing: .5px; } .toolbar { margin: 4px 0 16px; } -.toolbar button { background: #2b2c30; color: #d4d4d4; border: 1px solid #444; border-radius: 4px; +.toolbar button { background: var(--btn-bg); color: var(--btn-fg); + border: 1px solid var(--btn-border); border-radius: 4px; padding: 4px 10px; font-size: 12px; cursor: pointer; margin-right: 6px; } -.toolbar button:hover { background: #35363b; } -#flt { background: #2b2c30; color: #d4d4d4; border: 1px solid #444; border-radius: 4px; - padding: 4px 10px; font-size: 12px; width: 280px; margin-left: 10px; } -#flt:focus { outline: none; border-color: #6a6a6a; } +.toolbar button:hover { background: var(--btn-hover); } +#flt { background: var(--btn-bg); color: var(--btn-fg); border: 1px solid var(--btn-border); + border-radius: 4px; padding: 4px 10px; font-size: 12px; width: 280px; margin-left: 10px; } +#flt:focus { outline: none; border-color: var(--btn-focus); } +/* the dark/light switch. Fixed, top right, out of the reading column: it is a + preference about the page, not a fact about the compare, so it must not sit + among the verdict badges where it would read as one. */ +#thm { position: fixed; top: 14px; right: 18px; z-index: 9; background: var(--btn-bg); + color: var(--btn-fg); border: 1px solid var(--btn-border); border-radius: 6px; + padding: 4px 10px; font-size: 12px; cursor: pointer; + font-family: Segoe UI, Arial, sans-serif; } +#thm:hover { background: var(--btn-hover); } table.ov { border-collapse: collapse; font-size: 13px; margin: 4px 0 22px; } -table.ov th { text-align: left; color: #8a8a8a; font-weight: normal; font-size: 12px; - padding: 3px 18px 4px 0; border-bottom: 1px solid #3a3b40; } -table.ov td { padding: 5px 18px 5px 0; border-bottom: 1px solid #2c2d31; vertical-align: top; } -table.ov a { color: #dcdcaa; text-decoration: none; border-bottom: 1px dotted #666; - cursor: pointer; } -table.ov a:hover { color: #fff; } +table.ov th { text-align: left; color: var(--fg-muted); font-weight: normal; font-size: 12px; + padding: 3px 18px 4px 0; border-bottom: 1px solid var(--border-strong); } +table.ov td { padding: 5px 18px 5px 0; border-bottom: 1px solid var(--border-soft); + vertical-align: top; } +table.ov a { color: var(--accent); text-decoration: none; + border-bottom: 1px dotted var(--link-underline); cursor: pointer; } +table.ov a:hover { color: var(--link-hover); } .cnt { margin-right: 10px; white-space: nowrap; } -.cnt-real { color: #ffb3b3; } .cnt-add { color: #a8e6b0; } .cnt-del { color: #d9a8e6; } -.cnt-ign { color: #c3c7cd; } .cnt-id { color: #8a8a8a; } .cnt-err { color: #ff9d9d; font-weight: 700; } -.cnt-cmt { color: #b9bec6; } -.aut { color: #9a9a9a; } -.aut .a-add { color: #7bd88a; } .aut .a-del { color: #ff7b7b; } .aut .a-chg { color: #7fb3d9; } -.ifgroup { color: #8a8a8a; font-size: 11px; text-transform: uppercase; letter-spacing: .5px; - margin: 8px 0 2px; } +.cnt-real { color: var(--tag-real-fg); } .cnt-add { color: var(--tag-add-fg); } +.cnt-del { color: var(--tag-del-fg); } +.cnt-ign { color: var(--tag-ign-fg); } .cnt-id { color: var(--st-id); } +.cnt-err { color: var(--st-err); font-weight: 700; } +.cnt-cmt { color: var(--st-cmt-text); } +.aut { color: var(--fg-dim); } +.aut .a-add { color: var(--st-add); } .aut .a-del { color: var(--st-real); } +.aut .a-chg { color: var(--mv-fg); } +.ifgroup { color: var(--fg-muted); font-size: 11px; text-transform: uppercase; + letter-spacing: .5px; margin: 8px 0 2px; } .iflist .ifgroup:first-child { margin-top: 0; } -.if-chg { color: #7fb3d9; } -details.model { margin: 16px 0; border: 1px solid #3a3b40; border-radius: 8px; - background: #202124; } +.if-chg { color: var(--mv-fg); } +details.model { margin: 16px 0; border: 1px solid var(--border-strong); border-radius: 8px; + background: var(--panel-alt); } details.model > summary { list-style: none; cursor: pointer; padding: 9px 14px; - font-size: 15px; color: #dcdcaa; user-select: none; display: flex; + font-size: 15px; color: var(--accent); user-select: none; display: flex; align-items: center; gap: 10px; } details.model > summary::-webkit-details-marker { display: none; } -details.model > summary::before { content: '▶'; font-size: 10px; color: #8a8a8a; +details.model > summary::before { content: '▶'; font-size: 10px; color: var(--fg-muted); transition: transform .15s; } details.model[open] > summary::before { transform: rotate(90deg); } -details.model > summary:hover { background: #26272b; } +details.model > summary:hover { background: var(--panel-2); } details.model > .mbody { padding: 0 12px 10px; } summary .mcounts { font-size: 12px; font-weight: normal; } -.b-rev { background: #274a45; color: #9fe0cf; } -.rvnote { background: #22302e; border-left: 3px solid #3f8f7a; border-radius: 4px; - padding: 6px 10px; margin: 8px 0 2px; font-size: 12px; color: #cfe6df; - white-space: pre-wrap; } -.rvnote .rvtag { color: #7fd3ba; font-weight: 700; margin-right: 8px; } -.rvnote .rvwhere { color: #7d8f8b; margin-right: 8px; font-family: Consolas, monospace; } -.rvnote.pending { background: #2d2b21; border-left-color: #8a7a3f; color: #e6dcc0; } -.rvnote.pending .rvtag { color: #d8c07a; } -.rvcheck { color: #5f9e8b; } +.b-rev { background: var(--tag-rev-bg); color: var(--tag-rev-fg); } +.rvnote { background: var(--note-bg); border-left: 3px solid var(--note-border); + border-radius: 4px; padding: 6px 10px; margin: 8px 0 2px; font-size: 12px; + color: var(--note-fg); white-space: pre-wrap; } +.rvnote .rvtag { color: var(--note-tag); font-weight: 700; margin-right: 8px; } +.rvnote .rvwhere { color: var(--note-where); margin-right: 8px; + font-family: Consolas, monospace; } +.rvnote.pending { background: var(--note-pending-bg); + border-left-color: var(--note-pending-border); color: var(--note-pending-fg); } +.rvnote.pending .rvtag { color: var(--note-pending-tag); } +.rvcheck { color: var(--note-check); } /* the Reviewed badge hides what has already been signed off, so the next pass shows only what is left. It starts SHOWN: the report is the record, and a record that opens with real changes already hidden is not one. */ body.hide-rev .grp-rev, body.hide-rev details.file.file-rev { display: none; } """ +# the switch itself, plus the script behind it. A saved preference wins over +# the flag the report was built with: the flag is the author's default for +# somebody who has never expressed one, and after that it is the reader's eyes. +_THEME_BUTTON = ('') +_THEME_JS = ( + 'function sttheme(t){document.documentElement.setAttribute("data-theme",t);' + 'var b=document.getElementById("thm");' + 'if(b)b.innerHTML=(t==="dark"?"\\u2600 Light":"\\u263e Dark");' + 'try{localStorage.setItem("cgc-theme",t);}catch(e){}}' + 'function tgtheme(){sttheme(document.documentElement.getAttribute("data-theme")' + '==="dark"?"light":"dark");}' + '(function(){var t=null;try{t=localStorage.getItem("cgc-theme");}catch(e){}' + 'sttheme(t==="dark"||t==="light"?t' + ':document.documentElement.getAttribute("data-theme"));})();') + + +def _head(title, initial, body_class=''): + """Everything up to and including the opening ````. + + BOTH palettes go into the page: a report is mailed around and opened on a + machine that may have no network, so the dark/light switch has to be an + attribute flip with nothing left to fetch. ``initial`` only decides which + one the page opens with. + """ + cls = ' class="{}"'.format(body_class) if body_class else '' + return ('' + '{}{}' + .format(theme.normalize(initial), title, + theme.css_vars(theme.DARK), theme.css_vars(theme.LIGHT), + _CSS, cls, _THEME_BUTTON)) + def _esc(s): return html.escape(s, quote=False) @@ -949,12 +1024,14 @@ def _safe_file_section(rel, results, old_root, new_root, anchors, rv): ''.format(_esc(type(e).__name__), _esc(str(e)))) -def build_arxml_report(results, old_root, new_root, old_label=None): +def build_arxml_report(results, old_root, new_root, old_label=None, + theme_name=theme.DEFAULT): """Compact ARXML / A2L update report: did the AUTOSAR model or the calibration surface change, and how. ``old_label`` names the OLD side when its folder does not -- see - :func:`build_report`. + :func:`build_report`. ``theme_name`` is which palette the page opens with; + both are always embedded, and the reader can switch. Only .arxml/.xml/.a2l files are considered; other files in `results` are ignored ('error' entries of any extension always count -- a failed @@ -974,9 +1051,7 @@ def build_arxml_report(results, old_root, new_root, old_label=None): counts = summarize(ax) now = datetime.datetime.now().strftime('%Y-%m-%d %H:%M:%S') parts = [] - parts.append('' - 'ARXML / A2L Update Report' - ''.format(_CSS)) + parts.append(_head('ARXML / A2L Update Report', theme_name)) parts.append('

ARXML / A2L Update Report

') parts.append('
{} → {} · {}
'.format( _root_html('BASELINE', old_root, old_label), _root_html('CURRENT', new_root), now)) @@ -1048,17 +1123,23 @@ def build_arxml_report(results, old_root, new_root, old_label=None): 'timestamps / comments / whitespace).

') parts.append(_autosar_section(ax, {})) + parts.append(''.format(_THEME_JS)) parts.append('') return ''.join(parts) -def build_report(results, old_root, new_root, reviews=None, old_label=None): +def build_report(results, old_root, new_root, reviews=None, old_label=None, + theme_name=theme.DEFAULT): """Full self-contained HTML report. ``old_label`` names the OLD side when its folder does not: comparing against a commit checks it out to a temp folder, and the record has to say which commit that was. + ``theme_name`` is which palette the page opens with (``--theme`` on the + CLI). Both are embedded either way, so the reader's own button -- and their + saved preference -- can override it without the file changing. + ``reviews`` is a :class:`compare_tool.review.ReviewStore` or None. When given, every change the reviewer signed off carries its note, and a Reviewed badge folds those changes away so a second pass sees only what is @@ -1110,9 +1191,8 @@ def build_report(results, old_root, new_root, reviews=None, old_label=None): anchors, rv)) parts = [] - parts.append('' - 'AUTOSAR Code Generation Report' - ''.format(_CSS)) + parts.append(_head('AUTOSAR Code Generation Report', theme_name, + body_class='hide-ign')) parts.append('

AUTOSAR Code Generation Report

') parts.append('
{} → {} · {}
'.format( _root_html('BASELINE', old_root, old_label), _root_html('CURRENT', new_root), now)) @@ -1205,6 +1285,7 @@ def build_report(results, old_root, new_root, reviews=None, old_label=None): 'var any=!q;if(!any)mo.querySelectorAll("details.file").forEach(function(d){' 'if(d.style.display!=="none")any=true;});' 'mo.style.display=any?"":"none";});}' + + _THEME_JS + '') parts.append('') return ''.join(parts) diff --git a/compare_tool/syntax.py b/compare_tool/syntax.py index 1d9cd6f..b63521f 100644 --- a/compare_tool/syntax.py +++ b/compare_tool/syntax.py @@ -1,4 +1,4 @@ -"""Syntax token spans for one line of C or ARXML/XML. +"""Syntax token spans for one line of C, ARXML/XML or A2L. Line-at-a-time on purpose: the viewer paints a QTextDocument block by block, and a whole-file lexer would have to be re-run and re-mapped every time a @@ -11,10 +11,15 @@ No Qt, stdlib only: it ships in the zipapp and its tests run headless. -**Only C and XML are covered.** A2L is deliberately not highlighted -- the -format is a flat keyword soup where nearly every line would light up, which is -decoration, not information. An unknown file comes back with no spans and -renders as plain text. +**A2L is coloured from a keyword list, never by shape.** The format is a flat +soup of ALL-CAPS words, so a lazy `[A-Z_]+` rule lights up every line -- +including the calibration object names, which are the one thing a reviewer is +scanning for. Only `/begin` / `/end`, the block name that follows one of them, +and the ASAM keywords and enum literals below get a colour; an identifier stays +plain and therefore stands out. That is how ASAP2 editors show it, and it is +the reason the format is worth highlighting at all. + +An unknown file comes back with no spans and renders as plain text. Strings and comments are found by walking the line, not by regex alternation: a `/*` inside a string literal must not open a comment. Getting that wrong @@ -53,32 +58,89 @@ 'size_t|ptrdiff_t|u?int(?:8|16|32|64)_t' ) -# rules for the stretches that are neither string nor comment; first match wins +_NUMBER_RE = re.compile( + r'\b(?:0[xX][0-9a-fA-F]+|\d+\.?\d*(?:[eE][-+]?\d+)?)' + r'(?:[uUlL]{1,3}|[fF])?\b') + +# rules for the stretches that are neither string nor comment; first match +# wins. Each rule is (kind, regex, group): group 0 is the whole match, a +# higher group colours only part of it -- how a block name is picked out of +# '/begin CHARACTERISTIC' without a lookbehind that would have to guess how +# much whitespace sits between the two. _C_PLAIN = [ - (NUMBER, re.compile( - r'\b(?:0[xX][0-9a-fA-F]+|\d+\.?\d*(?:[eE][-+]?\d+)?)' - r'(?:[uUlL]{1,3}|[fF])?\b')), - (KEYWORD, re.compile(r'\b(?:{})\b'.format(_C_KEYWORDS))), - (TYPE, re.compile(r'\b(?:{}|[A-Za-z_]\w*_T)\b'.format(_C_TYPES))), - (CALL, re.compile(r'\b[A-Za-z_]\w*(?=\s*\()')), + (NUMBER, _NUMBER_RE, 0), + (KEYWORD, re.compile(r'\b(?:{})\b'.format(_C_KEYWORDS)), 0), + (TYPE, re.compile(r'\b(?:{}|[A-Za-z_]\w*_T)\b'.format(_C_TYPES)), 0), + (CALL, re.compile(r'\b[A-Za-z_]\w*(?=\s*\()'), 0), ] _XML_PLAIN = [ # the element name only, without its bracket: , - (TAG, re.compile(r'(?<=<)/?[A-Za-z_][\w.:-]*')), - (ATTR, re.compile(r'\b[A-Za-z_][\w.:-]*(?=\s*=)')), + (TAG, re.compile(r'(?<=<)/?[A-Za-z_][\w.:-]*'), 0), + (ATTR, re.compile(r'\b[A-Za-z_][\w.:-]*(?=\s*=)'), 0), +] + +# ASAM MCD-2 MC attribute keywords: the words that describe an object rather +# than name one. Curated, not '[A-Z_]+' -- see the module docstring. +_A2L_KEYWORDS = ( + 'A2ML_VERSION|ADDR_EPK|ALIGNMENT_BYTE|ALIGNMENT_FLOAT16_IEEE|' + 'ALIGNMENT_FLOAT32_IEEE|ALIGNMENT_FLOAT64_IEEE|ALIGNMENT_INT64|' + 'ALIGNMENT_LONG|ALIGNMENT_WORD|ANNOTATION_LABEL|ANNOTATION_ORIGIN|' + 'ARRAY_SIZE|ASAP2_VERSION|AXIS_PTS_REF|AXIS_PTS_[XYZ45]|' + 'AXIS_RESCALE_[XYZ45]|BIT_MASK|BYTE_ORDER|CALIBRATION_ACCESS|COEFFS|' + 'COEFFS_LINEAR|COMPARISON_QUANTITY|COMPU_TAB_REF|CPU_TYPE|CURVE_AXIS_REF|' + 'CUSTOMER_NO|CUSTOMER|DATA_SIZE|DEFAULT_VALUE_NUMERIC|DEFAULT_VALUE|' + 'DEPOSIT|DISCRETE|DISPLAY_IDENTIFIER|DIST_OP_[XYZ45]|' + 'ECU_ADDRESS_EXTENSION|ECU_ADDRESS|ECU_CALIBRATION_OFFSET|ECU|EPK|' + 'ERROR_MASK|EXTENDED_LIMITS|FIX_AXIS_PAR_DIST|FIX_AXIS_PAR|' + 'FIX_NO_AXIS_PTS_[XYZ45]|FNC_VALUES|FORMAT|FORMULA_INV|FORMULA|' + 'GUARD_RAILS|IDENTIFICATION|LEFT_SHIFT|MATRIX_DIM|MAX_DIFF|MAX_GRAD|' + 'MAX_REFRESH|MODEL_LINK|MONOTONY|NO_AXIS_PTS_[XYZ45]|NO_OF_INTERVALS|' + 'NO_RESCALE_[XYZ45]|NUMBER|OFFSET_[XYZ45]|PHONE_NO|PHYS_UNIT|PROJECT_NO|' + 'READ_ONLY|READ_WRITE|REF_MEMORY_SEGMENT|REF_UNIT|RIGHT_SHIFT|' + 'RIP_ADDR_[WXYZ45]|SHIFT_OP_[XYZ45]|SI_EXPONENTS|SRC_ADDR_[XYZ45]|' + 'STATIC_RECORD_LAYOUT|STATUS_STRING_REF|STEP_SIZE|SUPPLIER|' + 'SYMBOL_TYPE_LINK|SYMBOL_LINK|SYSTEM_CONSTANT|UNIT_CONVERSION|USER|VERSION' +) +# the closed vocabularies: data types, byte orders, conversion and layout +# kinds, access rights. They share the block names' colour because that is what +# they are -- the type of the thing, not its name. +_A2L_LITERALS = ( + 'A_INT64|A_UINT64|ASCII|BIG_ENDIAN|BYTE|CALIBRATION_VARIABLES|CALIBRATION|' + 'CODE|COLUMN_DIR|COM_AXIS|CUBOID|CUB4|CUB5|CURVE_AXIS|CURVE|DATA|DERIVED|' + 'DIRECT|EXCLUDE_FROM_FLASH|EXTERN|FIX_AXIS|FLOAT16_IEEE|FLOAT32_IEEE|' + 'FLOAT64_IEEE|FORM|IDENTICAL|INDEX_DECR|INDEX_INCR|INTERN|LINEAR|' + 'LITTLE_ENDIAN|LONG|MAP|MON_DECREASE|MON_INCREASE|MONOTONOUS|MSB_FIRST|' + 'MSB_LAST|NOT_IN_ECU|NOT_IN_MCD_SYSTEM|NOT_MON|NO_CALIBRATION|' + 'OFFLINE_CALIBRATION|OFFLINE_DATA|PBYTE|PLONG|PWORD|RAT_FUNC|RES_AXIS|' + 'RESERVED|ROW_DIR|SBYTE|SERAM|SLONG|STD_AXIS|STRICT_DECREASE|' + 'STRICT_INCREASE|STRICT_MON|SWORD|TAB_INTP|TAB_NOINTP|TAB_VERB|UBYTE|' + 'ULONG|UWORD|VAL_BLK|VALUE|VARIABLE|WORD|WORM|RO|RW|WO' +) + +_A2L_PLAIN = [ + (KEYWORD, re.compile(r'/(?:begin|end)\b', re.IGNORECASE), 0), + # whatever a block opens or closes IS its type, whether or not this module + # has heard of it -- so a vendor block reads like every other one + (TYPE, re.compile(r'/(?:begin|end)\s+([A-Za-z_]\w*)', re.IGNORECASE), 1), + (NUMBER, _NUMBER_RE, 0), + (KEYWORD, re.compile(r'\b(?:{})\b'.format(_A2L_KEYWORDS)), 0), + (TYPE, re.compile(r'\b(?:{})\b'.format(_A2L_LITERALS)), 0), ] class _Lang: - """Everything that differs between the two languages, in one place.""" + """Everything that differs between the languages, in one place.""" - def __init__(self, plain, block, line_comment=None, quotes='"', escape=False): + def __init__(self, plain, block, line_comment=None, quotes='"', + escape=False, doubled_quote=False, preproc=False): self.plain = plain self.block_open, self.block_close = block self.line_comment = line_comment self.quotes = quotes self.escape = escape + self.doubled_quote = doubled_quote + self.preproc = preproc parts = [re.escape(self.block_open)] if line_comment: parts.insert(0, re.escape(line_comment)) # '//' before '/*' @@ -88,8 +150,14 @@ def __init__(self, plain, block, line_comment=None, quotes='"', escape=False): _LANGS = { 'c': _Lang(_C_PLAIN, ('/*', '*/'), line_comment='//', quotes='"\'', - escape=True), + escape=True, preproc=True), 'arxml': _Lang(_XML_PLAIN, (''), quotes='"\''), + # A2L strings are not C strings: a backslash is a literal character + # (Windows paths appear verbatim) and a quote is escaped by doubling it. + # Treating '\' as an escape would swallow the code after a path ending in + # one -- the same trap a2l_rules.strip_a2l_comments exists to avoid. + 'a2l': _Lang(_A2L_PLAIN, ('/*', '*/'), line_comment='//', quotes='"', + doubled_quote=True), } _PREPROC = re.compile(r'^\s*#\s*\w+') @@ -108,22 +176,25 @@ def language_for(rel): def _plain_spans(text, rules, start, end): """Non-overlapping spans in ``text[start:end]``; earlier rules win.""" taken, out = [], [] - for kind, rx in rules: + for kind, rx, group in rules: for m in rx.finditer(text, start, end): - a, b = m.span() - if a == b or any(a < tb and ta < b for ta, tb in taken): + a, b = m.span(group) + if a < 0 or a == b or any(a < tb and ta < b for ta, tb in taken): continue taken.append((a, b)) out.append((a, b, kind)) return out -def _string_end(text, start, quote, escape): +def _string_end(text, start, quote, escape, doubled=False): """Index just past the closing quote, or len(text) when it never closes. An unterminated string ends at the newline rather than leaking into the next line: C has no multi-line string literals worth the state, and a diff row is shown one line at a time anyway. + + ``doubled`` is A2L's escape rule -- a quote inside a string is written + twice -- so `""` continues the literal instead of ending it. """ i = start + 1 while i < len(text): @@ -131,6 +202,9 @@ def _string_end(text, start, quote, escape): i += 2 continue if text[i] == quote: + if doubled and text[i + 1:i + 2] == quote: + i += 2 + continue return i + 1 i += 1 return len(text) @@ -157,7 +231,7 @@ def spans(text, language, state=PLAIN): return [(0, len(text), COMMENT)], IN_BLOCK_COMMENT pos = close + len(lang.block_close) out.append((0, pos, COMMENT)) - elif lang.line_comment: + elif lang.preproc: # anchored, so it has to be handled before the scan rather than as one # more alternative inside it m = _PREPROC.match(text) @@ -185,7 +259,8 @@ def spans(text, language, state=PLAIN): pos = close + len(lang.block_close) out.append((m.start(), pos, COMMENT)) else: - end = _string_end(text, m.start(), tok, lang.escape) + end = _string_end(text, m.start(), tok, lang.escape, + lang.doubled_quote) out.append((m.start(), end, STRING)) pos = end out.sort() diff --git a/compare_tool/theme.py b/compare_tool/theme.py new file mode 100644 index 0000000..90bc62b --- /dev/null +++ b/compare_tool/theme.py @@ -0,0 +1,361 @@ +"""The colour palettes, in one place, for every surface that paints something. + +Rule 3 of this repo -- one seam per shared decision -- applied to colour. The +HTML report, the Qt panes, the minimap, the folder tree and the syntax +highlighter all used to carry their own hex literals, which is fine until a +second theme exists: then "dark red for a removed line" has to be answered five +times and the five answers drift. + +So every colour is a **role** here, named once, with one value per theme. A role +name is a valid CSS custom-property name on purpose: the report emits the whole +palette as ``--role: value`` pairs and uses ``var(--role)``, while the Qt layer +looks the same role up with :func:`c`. Neither side can invent a colour the +other does not have. + +stdlib only, no Qt, no HTML: this ships in the zipapp and its tests run +headless. + +Two conventions worth knowing before adding a role: + +* Values are ``#rrggbb``, except a handful of overlay roles which are + ``#aarrggbb`` -- Qt's own notation, used for things painted *over* the code + (the current-change wash, the minimap strips). :func:`css_vars` leaves those + out, since CSS spells alpha the other way round and the report does not use + them. +* Red means removed and green means added, in both themes, on every surface. + Noise is the same pair one notch closer to the background, never a fourth + hue -- see the note in ``qtviewer/diffpane.py``. +""" + +DARK = 'dark' +LIGHT = 'light' +THEMES = (DARK, LIGHT) +DEFAULT = DARK + +_DARK = { + # --- page and chrome --- + 'bg': '#1e1f22', + 'fg': '#d4d4d4', + 'fg-strong': '#e8e8e8', + 'fg-dim': '#9a9a9a', + 'fg-muted': '#8a8a8a', + 'fg-faint': '#7a7a7a', + 'panel': '#232427', + 'panel-alt': '#202124', + 'panel-2': '#26272b', + 'panel-3': '#2b2c30', + 'panel-hover': '#2a2b2f', + 'border': '#34363c', + 'border-soft': '#2c2d31', + 'border-strong': '#43454c', + 'accent': '#dcdcaa', + 'accent-2': '#7c8cf8', + 'link-underline': '#666666', + 'link-hover': '#ffffff', + + # --- verdict colours (tree marks, counts, status text) --- + 'st-real': '#ff7b7b', + 'st-cmt': '#8f96a2', + 'st-cmt-text': '#b9bec6', + 'st-ign': '#9aa1ad', + 'st-add': '#7bd88a', + 'st-del': '#c88ad8', + 'st-id': '#8a8a8a', + 'st-err': '#ff5c5c', + + # --- verdict chips / badges --- + 'tag-real-bg': '#6e2b2b', 'tag-real-fg': '#ffb3b3', + 'tag-cmt-bg': '#33353a', 'tag-cmt-fg': '#b9bec6', + 'tag-ign-bg': '#3a3b40', 'tag-ign-fg': '#c3c7cd', + 'tag-add-bg': '#2b5232', 'tag-add-fg': '#a8e6b0', + 'tag-del-bg': '#4a2b52', 'tag-del-fg': '#d9a8e6', + 'tag-err-bg': '#7a1f1f', 'tag-err-fg': '#ffc2c2', + 'tag-id-bg': '#333333', 'tag-id-fg': '#aaaaaa', + 'tag-adddel-bg': '#33404a', 'tag-adddel-fg': '#cfe0ec', + 'tag-rev-bg': '#274a45', 'tag-rev-fg': '#9fe0cf', + + # --- the incomplete-compare banner --- + 'err-bg': '#4a1d1d', + 'err-border': '#b04a4a', + 'err-fg': '#ffd6d6', + 'err-code-bg': '#5c2626', + + # --- diff rows --- + 'del-bg': '#3a2222', 'add-bg': '#1f3a24', + 'del-bg-dim': '#2f2020', 'add-bg-dim': '#1e2f21', + 'mv-bg': '#1d2f3e', + 'seg-del-bg': '#7a2f2f', 'seg-del-fg': '#ffc2c2', + 'seg-add-bg': '#2f6e3d', 'seg-add-fg': '#c9f7d1', + 'seg-del-dim-bg': '#5e2a2a', 'seg-del-dim-fg': '#f0c4c4', + 'seg-add-dim-bg': '#2c5738', 'seg-add-dim-fg': '#bfe8c8', + 'seg-mv-bg': '#2f5a7a', + 'mv-fg': '#7fb3d9', + 'ln-fg': '#6a6a6a', + 'gap-fg': '#666666', + # a row the current compare rules do not report: still on screen, still + # readable, painted so the eye slides off it. The band has to be visibly + # OFF the editor background -- a wash one shade away just reads as ordinary + # context, and then nothing on screen says the category was switched off. + 'muted-bg': '#2c2e33', + 'muted-fg': '#6c7178', + + # --- review notes --- + 'note-bg': '#22302e', 'note-border': '#3f8f7a', 'note-fg': '#cfe6df', + 'note-tag': '#7fd3ba', 'note-where': '#7d8f8b', + 'note-pending-bg': '#2d2b21', 'note-pending-border': '#8a7a3f', + 'note-pending-fg': '#e6dcc0', 'note-pending-tag': '#d8c07a', + 'note-check': '#5f9e8b', + + # --- buttons and inputs (report toolbar; the viewer's QSS uses these too) --- + 'btn-bg': '#2b2c30', 'btn-fg': '#d4d4d4', 'btn-border': '#444444', + 'btn-hover': '#35363b', 'btn-focus': '#6a6a6a', + + # --- viewer: editors, gutter, overlays --- + 'code-bg': '#232427', + 'code-fg': '#d4d4d4', + 'gutter-bg': '#1e1f22', + 'gutter-fg': '#6a6a6a', + 'filler-bg': '#26272b', + 'cur-row': '#22ffffff', + 'find-bg': '#5a4715', + 'find-cur-bg': '#8f7220', + 'pane-banner-bg': '#2a2c31', + 'pane-old-accent': '#c98b8b', + 'pane-new-accent': '#8ec69a', + + # --- viewer: minimap --- + 'map-bg': '#202124', + 'map-ctx': '#565b62', + 'map-real': '#e8908d', + 'map-noise': '#a4706e', + 'map-moved': '#7fb0d9', + 'map-muted': '#3f4348', + 'map-strip-real': '#46d9524f', + 'map-strip-noise': '#28d9524f', + 'map-strip-moved': '#463f7fb0', + 'map-view-fill': '#1affffff', + 'map-view-border': '#78bebebe', + + # --- viewer: window chrome --- + 'chrome-bg': '#25262a', + 'chrome-hover': '#34363c', + 'chrome-pressed': '#3d404a', + 'chrome-checked-bg': '#343a63', + 'chrome-checked-fg': '#e8e8ff', + 'chrome-checked-hover': '#454c80', + 'chrome-bar-bg': '#212226', + 'chrome-disabled-bg': '#2b2d33', + 'chrome-disabled-fg': '#6a6a6a', + 'tree-selected': '#3a4a7a', + 'header-bg': '#2a2c31', + 'header-fg': '#b9b9b9', + 'progress-chunk': '#4f46e5', + 'status-fg': '#b0b0b0', + 'icon-tint': '#d7d7d7', + 'state-idle': '#8a8f98', + 'state-busy': '#e2c16b', + 'state-ready': '#7bd88a', + 'state-error': '#ff7b7b', + 'review-done': '#7bd88a', + 'review-partial': '#e2c16b', + 'review-none': '#8a8f98', + + # --- syntax tokens (foreground only; never red or green -- those two mean + # removed and added on the very same line) --- + 'syn-comment': '#8f96a2', + 'syn-string': '#e0a860', + 'syn-number': '#c5a3e8', + 'syn-keyword': '#7aa2e3', + 'syn-type': '#57b6a9', + 'syn-preproc': '#b58ac4', + 'syn-call': '#d8c99a', + 'syn-tag': '#7aa2e3', + 'syn-attr': '#57b6a9', +} + +_LIGHT = { + 'bg': '#ffffff', + 'fg': '#1f2328', + 'fg-strong': '#0d1117', + 'fg-dim': '#57606a', + 'fg-muted': '#6e7781', + 'fg-faint': '#8c959f', + 'panel': '#f6f8fa', + 'panel-alt': '#f0f3f6', + 'panel-2': '#eef1f4', + 'panel-3': '#eff2f5', + 'panel-hover': '#eaeef2', + 'border': '#d0d7de', + 'border-soft': '#e4e8ed', + 'border-strong': '#c2c8d0', + 'accent': '#6f42c1', + 'accent-2': '#4f46e5', + 'link-underline': '#b8c0c8', + 'link-hover': '#0550ae', + + 'st-real': '#cf222e', + 'st-cmt': '#7d848d', + 'st-cmt-text': '#57606a', + 'st-ign': '#6e7781', + 'st-add': '#1a7f37', + 'st-del': '#8250df', + 'st-id': '#8c959f', + 'st-err': '#d1242f', + + 'tag-real-bg': '#ffe1e1', 'tag-real-fg': '#a40e26', + 'tag-cmt-bg': '#eef1f4', 'tag-cmt-fg': '#57606a', + 'tag-ign-bg': '#eaeef2', 'tag-ign-fg': '#4a525c', + 'tag-add-bg': '#dafbe1', 'tag-add-fg': '#0f6626', + 'tag-del-bg': '#f5eafd', 'tag-del-fg': '#6639ba', + 'tag-err-bg': '#ffdcdc', 'tag-err-fg': '#a40e26', + 'tag-id-bg': '#eaeef2', 'tag-id-fg': '#6e7781', + 'tag-adddel-bg': '#ddf4ff', 'tag-adddel-fg': '#0a3069', + 'tag-rev-bg': '#d7f5ec', 'tag-rev-fg': '#0f5d4e', + + 'err-bg': '#fff5f5', + 'err-border': '#e0989b', + 'err-fg': '#a40e26', + 'err-code-bg': '#ffe0e0', + + 'del-bg': '#ffebe9', 'add-bg': '#e6ffec', + 'del-bg-dim': '#fff5f4', 'add-bg-dim': '#f1fff5', + 'mv-bg': '#ddf4ff', + 'seg-del-bg': '#ffc9c4', 'seg-del-fg': '#6e0a17', + 'seg-add-bg': '#abefc0', 'seg-add-fg': '#03502a', + 'seg-del-dim-bg': '#ffdedb', 'seg-del-dim-fg': '#8a2b2b', + 'seg-add-dim-bg': '#cdf3d8', 'seg-add-dim-fg': '#17512c', + 'seg-mv-bg': '#b6e3ff', + 'mv-fg': '#0969da', + 'ln-fg': '#8c959f', + 'gap-fg': '#8c959f', + 'muted-bg': '#eaecef', + 'muted-fg': '#9aa1a9', + + 'note-bg': '#eafaf4', 'note-border': '#3f8f7a', 'note-fg': '#12463c', + 'note-tag': '#0f6d5b', 'note-where': '#6a7d78', + 'note-pending-bg': '#fdf6e3', 'note-pending-border': '#b8a04f', + 'note-pending-fg': '#5c4c10', 'note-pending-tag': '#8a6d0a', + 'note-check': '#1b7f6a', + + 'btn-bg': '#f6f8fa', 'btn-fg': '#24292f', 'btn-border': '#d0d7de', + 'btn-hover': '#eaeef2', 'btn-focus': '#8c959f', + + 'code-bg': '#ffffff', + 'code-fg': '#1f2328', + 'gutter-bg': '#f6f8fa', + 'gutter-fg': '#8c959f', + 'filler-bg': '#f1f2f4', + 'cur-row': '#1e000000', + 'find-bg': '#fff0b3', + 'find-cur-bg': '#ffd633', + 'pane-banner-bg': '#eef1f4', + 'pane-old-accent': '#a4343a', + 'pane-new-accent': '#1a7f37', + + 'map-bg': '#f0f2f5', + 'map-ctx': '#c2c8d0', + 'map-real': '#e5484d', + 'map-noise': '#efa8a8', + 'map-moved': '#5b9bd5', + 'map-muted': '#dfe3e8', + 'map-strip-real': '#38d9524f', + 'map-strip-noise': '#1cd9524f', + 'map-strip-moved': '#383f7fb0', + 'map-view-fill': '#14000000', + 'map-view-border': '#78606a76', + + 'chrome-bg': '#f0f2f5', + 'chrome-hover': '#e2e6eb', + 'chrome-pressed': '#d5dae0', + 'chrome-checked-bg': '#dde3ff', + 'chrome-checked-fg': '#24306b', + 'chrome-checked-hover': '#ccd5ff', + 'chrome-bar-bg': '#f2f4f7', + 'chrome-disabled-bg': '#e6e9ee', + 'chrome-disabled-fg': '#a0a6ae', + 'tree-selected': '#cfe3ff', + 'header-bg': '#eaeef2', + 'header-fg': '#4a525c', + 'progress-chunk': '#4f46e5', + 'status-fg': '#57606a', + 'icon-tint': '#3d414a', + 'state-idle': '#8c959f', + 'state-busy': '#b58900', + 'state-ready': '#1a7f37', + 'state-error': '#cf222e', + 'review-done': '#1a7f37', + 'review-partial': '#b58900', + 'review-none': '#8c959f', + + 'syn-comment': '#6a737d', + 'syn-string': '#a04a00', + 'syn-number': '#6f42c1', + 'syn-keyword': '#1c39bb', + 'syn-type': '#0d7a86', + 'syn-preproc': '#8250df', + 'syn-call': '#7a5c00', + 'syn-tag': '#1c39bb', + 'syn-attr': '#0d7a86', +} + +PALETTES = {DARK: _DARK, LIGHT: _LIGHT} + +# a palette missing a role the other one has is a bug that only shows up on the +# surface nobody looked at, so it is caught at import instead +assert set(_DARK) == set(_LIGHT), sorted(set(_DARK) ^ set(_LIGHT)) + +_current = DEFAULT + + +def normalize(name): + """A theme name from anywhere (CLI flag, saved setting) mapped onto one we + actually have. Unknown names fall back to the default rather than raising: + a colour scheme is never worth refusing to open the tool over.""" + return name if name in PALETTES else DEFAULT + + +def palette(name=None): + """The whole role -> colour mapping for one theme (the current one when + `name` is None).""" + return PALETTES[normalize(name or _current)] + + +def color(role, name=None): + """One role's colour. Raises on an unknown role -- a typo must not become a + silently missing colour on one surface only.""" + return palette(name)[role] + + +def c(role): + """:func:`color` in the current theme -- what the Qt widgets call.""" + return PALETTES[_current][role] + + +def current(): + return _current + + +def set_current(name): + """Switch the theme every Qt surface paints from. Returns the name that + actually took effect.""" + global _current + _current = normalize(name) + return _current + + +def other(name=None): + """The theme the toggle would switch to.""" + return LIGHT if normalize(name or _current) == DARK else DARK + + +def css_vars(name): + """One theme's palette as CSS custom properties, ready for a rule body. + + Roles carrying an alpha channel are skipped: they are written in Qt's + ``#aarrggbb`` order, which CSS reads as ``#rrggbbaa`` -- so emitting them + would define a wrong colour rather than an unused one. Nothing in the + report uses them. + """ + return ''.join('--{}:{};'.format(k, v) + for k, v in sorted(palette(name).items()) + if len(v) == 7) diff --git a/compare_tool/view_model.py b/compare_tool/view_model.py index 87a3493..4904f57 100644 --- a/compare_tool/view_model.py +++ b/compare_tool/view_model.py @@ -22,17 +22,22 @@ # mode: how a row is painted. 'ctx' = equal line (context), 'real' = real # change (red/green), 'comment' = comment-only noise, 'minor' = the other # ignorable noise (both painted in the same red/green, dimmer), 'moved' = -# moved block (blue). Comments get +# moved block (blue), 'muted' = a noise row the current compare rules do not +# report (see mute_rows). Comments get # their own mode for the same reason they get their own file verdict: banner # churn reads very differently from a renamed identifier. kind = the # underlying hunk kind ('equal' for ctx rows, otherwise straight from the hunk). Row = namedtuple('Row', 'old_no old_txt new_no new_txt mode kind') -# the only row modes a caller may collapse out of sight. 'real' and 'moved' are -# absent by construction: a UI toggle must never be able to fold away a change -# the reviewer has not seen. +# the only row modes a caller may mute. 'real' and 'moved' are absent by +# construction: a UI toggle must never be able to play down a change the +# reviewer has not seen. FOLDABLE_MODES = ('comment', 'minor') +# what a muted row's mode becomes. Its `kind` is left alone, so the row still +# says WHY it was played down (uuid, comment, rename, …). +MUTED = 'muted' + def char_span(old_txt, new_txt): """Character offsets of the single changed span on each side of one line @@ -64,46 +69,30 @@ def mode_of(kind): return 'minor' -def collapse_rows(rows, modes): - """Fold every run of noise rows into ONE placeholder row. +def mute_rows(rows, modes): + """Play noise rows DOWN instead of taking them away. - ``modes`` names the paint modes to fold (see :data:`FOLDABLE_MODES`; any - other mode passed in is ignored, so `real` and `moved` can never be folded - away by a caller's mistake). Each run becomes a single row carrying the same - ``⋯ N uuid lines hidden`` text on BOTH sides -- identical text, so it reads - as context rather than as a difference, and the two panes keep the same - block count and stay in scroll lockstep. + ``modes`` names the paint modes to mute (see :data:`FOLDABLE_MODES`; any + other mode passed in is ignored, so `real` and `moved` can never be muted + by a caller's mistake). Each matching row keeps its line numbers and its + text and comes back with mode :data:`MUTED`; the renderer paints it in a + flat grey with no diff colour, so it reads as "still here, does not count". - The count is always stated: this hides noise, it does not drop it, and a - reviewer must be able to see that something was folded and how much. + Collapsing those runs into a ``⋯ N lines hidden`` placeholder is what this + used to do, and it cost the reviewer the one thing a side-by-side view is + for: the code around a change. A regenerated file is mostly banner churn, + so folding it removed most of the file and left the surviving hunks without + context to read them in. Greying keeps the line count, the scroll position + and the shape of the file intact -- and, because row indices no longer + move, whatever the caller holds (navigation stops, find hits) stays valid. - Returns ``(rows, row_map)``. ``row_map[i]`` is where original row *i* now - lives, so a caller holding row indices (navigation stops) can move them - across; a folded row maps to its placeholder. + Returns a new list; the input is untouched, so the modes can be toggled + back and forth off one alignment. """ modes = tuple(m for m in modes if m in FOLDABLE_MODES) if not modes: - return list(rows), list(range(len(rows))) - out, row_map = [], [0] * len(rows) - i = 0 - while i < len(rows): - if rows[i].mode not in modes: - row_map[i] = len(out) - out.append(rows[i]) - i += 1 - continue - j, kinds = i, [] - while j < len(rows) and rows[j].mode in modes: - if rows[j].kind not in kinds: - kinds.append(rows[j].kind) - row_map[j] = len(out) - j += 1 - n = j - i - label = '{} {} {} line{} hidden'.format( - '⋯', n, ' + '.join(kinds), '' if n == 1 else 's') - out.append(Row(None, label, None, label, 'folded', ' + '.join(kinds))) - i = j - return out, row_map + return list(rows) + return [r._replace(mode=MUTED) if r.mode in modes else r for r in rows] def hunk_row_starts(hunks): diff --git a/docs/architecture.md b/docs/architecture.md index b62c64a..b5f715b 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -35,6 +35,7 @@ flowchart TD end subgraph shared[Shared seams] VM[view_model.py
mode_of · char_span · aligned_rows] + TH[theme.py
dark/light palettes by role] RV[review.py
notes keyed by content] SY[syntax.py
token spans, Qt-free] end @@ -50,6 +51,8 @@ flowchart TD QT --> RP RP --> VM QT --> VM + RP --> TH + QT --> TH QT --> SY RP --> RV QT --> RV @@ -58,7 +61,7 @@ flowchart TD Two rules hold this shape: **The core imports nothing but the standard library.** `scanner`, `diff_engine`, -the three rule modules, `report`, `review`, `view_model`, `syntax` and +the three rule modules, `report`, `review`, `view_model`, `theme`, `syntax` and `gitsource` are what ships in `compare_tool.pyz` — ~110 KB, no install, the documented fallback for machines where antivirus blocks the `.exe`. One third-party import in `scanner.py` and the zipapp stops running there. PySide6 @@ -66,8 +69,9 @@ lives only under `compare_tool/qtviewer/` and is imported lazily, when the viewer opens, so the test suite runs headless. **Arrows only point down.** The core never imports a front end. `syntax.py` -says *what* a stretch of text is and never what colour it gets, so the Qt layer -and any second surface can reuse it without the mapping being written twice. +says *what* a stretch of text is and never what colour it gets — that is +`theme.py`'s job, answered once for both surfaces — so the Qt layer and any +second surface can reuse it without the mapping being written twice. ## Data flow of one compare @@ -152,6 +156,13 @@ than mutates, so the rules can be toggled back and forth. The viewer keeps the untouched scan in `MainWindow._raw_results` and folds into `self.results` for display. +Folding a category changes two things, and only these two: the file's +**verdict** (it comes back `identical`, or `real-change` if something real +remains) and how its rows are **painted** — `view_model.mute_rows` greys them, +the minimap stops striping them and `F7`/`F8` never stopped on them anyway. The +lines themselves stay on screen. The hunks are never touched, so the exported +report, built from `_raw_results`, cannot notice that a category was folded. + ## The result dict is the contract Everything downstream — CLI summary, HTML report, viewer tree, review store — @@ -190,10 +201,17 @@ until someone adds a new kind to one of them. - **`view_model.char_span`** — the intra-line highlight as plain character offsets. The report wraps them in a ``; the viewer applies a `QTextCharFormat` over the same numbers. -- **`view_model.aligned_rows` / `collapse_rows`** — whole-file two-pane - alignment, and folding a run of noise rows into one `⋯ N uuid lines hidden` - placeholder that reads as context on both sides so the panes stay in scroll - lockstep. The count is always stated: this hides noise, it does not drop it. +- **`view_model.aligned_rows` / `mute_rows`** — whole-file two-pane alignment, + and playing a switched-off noise category *down* rather than away: the rows + keep their place, their line numbers and their text, and come back with mode + `muted` for the renderer to paint flat grey. Muting moves no row, so + navigation stops and find hits stay valid without translation, and the + reviewer keeps the context the surviving hunks have to be read in. +- **`theme.py`** — every colour as a named role, one value per theme. The + report emits the whole palette as CSS custom properties and uses + `var(--role)`; the Qt widgets look the same role up with `theme.c`. Adding a + role means adding it to **both** palettes — an import-time assert says so, + because the alternative is a `KeyError` on whichever surface nobody opened. - **`review.py`** — notes and sign-offs keyed by a hash of the change's own text, not by line number, so an unrelated edit elsewhere in the file does not detach them on the next scan. @@ -246,6 +264,9 @@ quick-changes rollup. **The HTML report is self-contained.** CSS and JS inline, no CDN, nothing fetched when the file is opened. It gets mailed around and opened on machines with no internet; a report that renders blank there is worse than no report. +That is also why the page carries *both* palettes rather than the one +`--theme` asked for: the reader's dark/light button has to be an attribute +flip, with nothing left to download. **Cosmetic failures degrade, the compare does not.** A missing icon leaves a button with its text label (`resources.py` getters return `None` and callers @@ -272,5 +293,6 @@ un-hidden on a crash. | New file type | `RULES` in `diff_engine.py`, a `*_rules.py` module, shadow + variants | | New semantic extraction | `*_rules.py` extractor, wire into `scanner.compare_file` and `_single_info`, then a `summarize_*` rollup | | Anything both renderers show | `view_model.py` — never inline in one of them | +| A colour, anywhere | `theme.py`, as a role in **both** palettes; the report uses `var(--role)`, Qt uses `theme.c(role)` | | New verdict | `diff_engine._status_of`, and decide explicitly whether it belongs in `scanner.FOLDABLE` (default: no) | | Viewer layout or colour | render it and look at it (`widget.grab().save(png)` under `QT_QPA_PLATFORM=offscreen`), then a real window | diff --git a/docs/vi/README.md b/docs/vi/README.md index 7117bfb..4348a34 100644 --- a/docs/vi/README.md +++ b/docs/vi/README.md @@ -97,6 +97,7 @@ Những gì lần scan tìm được vẫn được in ra. | `--exit-zero` | Luôn exit 0 kể cả khi có thay đổi thật (chế độ chỉ ghi report cho pipeline). Lỗi compare vẫn exit 2 | | `--arxml-only` | Chỉ scan `.arxml`/`.xml`/`.a2l` và ghi report gọn theo từng loại file (mặc định `arxml_update.html`) — luôn được ghi, kể cả khi không có gì đổi | | `--review FILE` | Render note và sign-off từ review file (`codegen-review.json`, do viewer ghi) ngay cạnh change tương ứng, kèm badge `Reviewed` để ẩn các change đã ký duyệt. Phải chỉ tên tường minh — một report không được vô tình mang sign-off của người khác; không có tác dụng với `--arxml-only` | +| `--theme dark\|light` | Bảng màu lúc mở của report và viewer (mặc định `dark`). Report mang sẵn **cả hai** và có nút đổi riêng, nên cờ này chỉ quyết định người đọc thấy màu nào trước | | `--qt`, `--viewer` | Mở viewer trên hai thư mục truyền ở command line, thay vì so sánh trong terminal. Cần extra `viewer` (xem dưới) | Bỏ `old_dir`/`new_dir` thì viewer mở. `--gui` (panel tkinter) đã bị bỏ ở 1.1.0. @@ -132,6 +133,12 @@ mục đó, lấy commit bạn chọn ra một thư mục tạm (read-only — w identifier xuyên suốt lần compare được. - `Hide identical` chỉ để lại các file có khác biệt trên cây. Đây là view: verdict, số đếm và report export ra đều không đổi. +- Bỏ tick `Comment` / `Unimportant` sẽ **làm mờ các dòng đó** chứ không xoá đi: + chúng ở nguyên chỗ cũ, giữ số dòng, mất màu đỏ/xanh, và biến khỏi minimap lẫn + `F7`/`F8`. Phần code xung quanh mới là thứ giúp đọc được một change, mà file + regenerate thì phần lớn là banner churn — gộp chúng lại là gộp mất gần cả file. +- `☀ Light` / `☾ Dark` trên toolbar đổi bảng màu; `--theme` chọn màu lúc mở. C, + ARXML và A2L đều được tô cú pháp ở cả hai theme. `Review mode` bật hộp note và cột `Review` trên cây — xanh khi mọi change trong dòng đã ký duyệt, hổ phách khi mới một phần, xám khi chưa cái nào. Ký duyệt một @@ -234,7 +241,9 @@ model nào rơi vào nhóm cuối **Shared / other**. File self-contained, mỗi lần compare một file: badge bật/tắt, cây thư mục, ô lọc, diff xếp gọn được theo từng file. Mở lên với `Unimportant` đã ẩn và `Modified` đã -mở, để mở ra là thấy ngay cái đáng xem. +mở, để mở ra là thấy ngay cái đáng xem. Nút `☀ Light` / `☾ Dark` nằm ở góc trên bên +phải — cả hai palette đều nhúng sẵn trong file, nên đổi màu không tải gì và chạy +được trên máy không có internet. ![Report viewer](../../resources/pic/report_page.png) @@ -307,7 +316,8 @@ compare_tool/ ├── arxml_rules.py # rule ARXML: UUID, ADMIN-DATA, DATE, comment + trích port interface, SWC (port/runnable/event) ├── a2l_rules.py # rule A2L: bóc comment kiểu C + trích CHARACTERISTIC/MEASUREMENT ├── view_model.py # view model không phụ thuộc renderer (paint mode, span trong dòng, canh dòng) dùng chung cho report và viewer -├── syntax.py # token span C / XML theo từng dòng, không dính Qt nên ship được trong .pyz +├── theme.py # palette sáng và tối dưới dạng role có tên, dùng chung cho CSS của report và mọi mặt Qt +├── syntax.py # token span C / XML / A2L theo từng dòng, không dính Qt nên ship được trong .pyz ├── review.py # note và sign-off của reviewer, khoá theo nội dung change nên sống sót qua lần scan sau ├── gitsource.py # `git archive` read-only một commit ra thư mục tạm, để commit đóng vai bên OLD └── report.py # HTML report self-contained (badge bật/tắt, tổng quan theo model, nhóm, lọc, diff xếp gọn) @@ -315,9 +325,9 @@ compare_tool/ [architecture.md](architecture.md) nói các mảnh này ghép với nhau ra sao và tại sao: hai lượt diff, chỗ verdict được quyết định, các seam dùng chung và contract -của result dict. Cái gì cả hai renderer đều cần thì nằm ở `view_model.py` — viết -lại một mapping ngay tại chỗ là cách để HTML report và viewer trôi lệch nhau về -chuyện cái gì đã đổi. +của result dict. Cái gì cả hai renderer đều cần thì nằm ở `view_model.py` (cái gì +đã đổi) hoặc `theme.py` (nó mang màu gì) — viết lại một mapping ngay tại chỗ là +cách để HTML report và viewer trôi lệch nhau. Thêm một rule: viết hàm strip trong `c_rules.py` / `arxml_rules.py` / `a2l_rules.py`, nối nó vào shadow của ruleset đó, đăng ký một variant có nhãn trong diff --git a/docs/vi/architecture.md b/docs/vi/architecture.md index 3f53e20..133e8cb 100644 --- a/docs/vi/architecture.md +++ b/docs/vi/architecture.md @@ -35,6 +35,7 @@ flowchart TD end subgraph shared[Seam dùng chung] VM[view_model.py
mode_of · char_span · aligned_rows] + TH[theme.py
palette sáng/tối theo role] RV[review.py
note khoá theo nội dung] SY[syntax.py
token span, không dính Qt] end @@ -50,6 +51,8 @@ flowchart TD QT --> RP RP --> VM QT --> VM + RP --> TH + QT --> TH QT --> SY RP --> RV QT --> RV @@ -58,15 +61,16 @@ flowchart TD Hai luật giữ cho hình dạng này đứng vững: **Core không import gì ngoài standard library.** `scanner`, `diff_engine`, ba module -rule, `report`, `review`, `view_model`, `syntax` và `gitsource` là những gì ship +rule, `report`, `review`, `view_model`, `theme`, `syntax` và `gitsource` là những gì ship trong `compare_tool.pyz` — ~110 KB, không cần cài, và là phương án dự phòng đã được ghi rõ cho các máy bị antivirus chặn `.exe`. Chỉ cần một import thư viện ngoài trong `scanner.py` là zipapp hết chạy ở đó. PySide6 chỉ nằm dưới `compare_tool/qtviewer/` và được import lười, lúc viewer mở, nên bộ test chạy headless được. **Mũi tên chỉ đi xuống.** Core không bao giờ import front end. `syntax.py` nói một -đoạn text *là gì* chứ không nói nó tô màu gì, nên lớp Qt và bất kỳ surface thứ hai -nào cũng dùng lại được mà không phải viết mapping đó hai lần. +đoạn text *là gì* chứ không nói nó tô màu gì — màu là việc của `theme.py`, trả lời +một lần cho cả hai surface — nên lớp Qt và bất kỳ surface thứ hai nào cũng dùng lại +được mà không phải viết mapping đó hai lần. ## Luồng dữ liệu của một lần compare @@ -148,6 +152,13 @@ cái đó là phí công. Nó copy chứ không sửa tại chỗ, nên bật t mái. Viewer giữ nguyên lần scan gốc trong `MainWindow._raw_results` và fold vào `self.results` để hiển thị. +Fold một nhóm chỉ đổi đúng hai thứ: **verdict** của file (thành `identical`, hoặc +`real-change` nếu còn thay đổi thật) và cách các dòng đó được **tô** — +`view_model.mute_rows` làm chúng xám đi, minimap thôi kẻ vạch cho chúng, còn +`F7`/`F8` thì vốn đã không dừng ở đó. Bản thân các dòng vẫn nằm trên màn hình. Hunk +không bị đụng tới, nên report xuất ra từ `_raw_results` không thể biết là có nhóm +nào đã bị fold. + ## Result dict là contract Mọi thứ ở phía sau — summary của CLI, HTML report, cây của viewer, review store — @@ -186,11 +197,15 @@ khớp nhau hoàn hảo cho tới lúc ai đó thêm một kind mới vào một - **`view_model.char_span`** — vùng highlight trong dòng, dưới dạng offset ký tự trần. Report bọc nó trong một ``; viewer áp `QTextCharFormat` lên đúng những con số đó. -- **`view_model.aligned_rows` / `collapse_rows`** — canh dòng hai pane cho cả file, - và gộp một chuỗi dòng noise thành một placeholder `⋯ N uuid lines hidden`, đọc - như context ở cả hai bên nên hai pane giữ được cuộn đồng bộ. Số lượng luôn được - ghi ra: cái này giấu noise, không vứt noise, và reviewer phải thấy được là có gì - đó đã bị gộp và gộp bao nhiêu. +- **`view_model.aligned_rows` / `mute_rows`** — canh dòng hai pane cho cả file, và + làm *mờ* một nhóm noise bị tắt thay vì bỏ nó đi: dòng giữ nguyên vị trí, số dòng + và nội dung, chỉ đổi mode thành `muted` để renderer tô một màu xám phẳng. Vì mute + không dời dòng nào, các mốc điều hướng và kết quả tìm kiếm vẫn đúng chỉ số, và + reviewer giữ được phần code xung quanh — thứ giúp đọc được những hunk còn lại. +- **`theme.py`** — mọi màu là một role có tên, mỗi theme một giá trị. Report xuất + cả palette ra CSS custom property rồi dùng `var(--role)`; widget Qt tra cùng role + đó bằng `theme.c`. Thêm role nghĩa là thêm vào **cả hai** palette — có assert lúc + import bắt việc này, vì nếu không thì mặt nào không ai mở sẽ nổ `KeyError`. - **`review.py`** — note và sign-off khoá theo hash nội dung của chính change đó, không theo số dòng, nên một sửa đổi không liên quan ở chỗ khác trong file không làm chúng rớt ra ở lần scan sau. @@ -240,7 +255,9 @@ theo đúng luật đó. **HTML report là self-contained.** CSS và JS nội tuyến, không CDN, mở file không tải gì về. Nó bị gửi email lòng vòng và mở trên máy không có internet; một report render -ra trắng bóc ở đó còn tệ hơn là không có report. +ra trắng bóc ở đó còn tệ hơn là không có report. Cũng vì thế mà trang nhúng *cả hai* +palette chứ không chỉ cái `--theme` yêu cầu: nút sáng/tối của người đọc phải chỉ là +đổi một attribute, không còn gì để tải. **Hỏng phần trang trí thì xuống cấp, hỏng phần compare thì kêu to.** Thiếu icon thì nút còn lại chữ (`resources.py` trả `None`, phía gọi tự lo); không có PySide6 thì @@ -265,5 +282,6 @@ chờ tiến trình nữa và vứt mất exit code, tức là gãy CI gate. Nê | Loại file mới | `RULES` trong `diff_engine.py`, một module `*_rules.py`, shadow + variant | | Trích ngữ nghĩa mới | extractor trong `*_rules.py`, nối vào `scanner.compare_file` và `_single_info`, rồi một rollup `summarize_*` | | Thứ cả hai renderer cùng hiện | `view_model.py` — đừng bao giờ viết thẳng vào một trong hai | +| Một màu bất kỳ | `theme.py`, thành role có trong **cả hai** palette; report dùng `var(--role)`, Qt dùng `theme.c(role)` | | Verdict mới | `diff_engine._status_of`, và quyết định rõ ràng xem nó có thuộc `scanner.FOLDABLE` không (mặc định: không) | | Layout hay màu của viewer | render ra rồi nhìn tận mắt (`widget.grab().save(png)` dưới `QT_QPA_PLATFORM=offscreen`), sau đó mở cửa sổ thật | diff --git a/tests/test_cli_modes.py b/tests/test_cli_modes.py index 823aed7..c5ea3c8 100644 --- a/tests/test_cli_modes.py +++ b/tests/test_cli_modes.py @@ -47,6 +47,30 @@ def test_help_and_bad_usage_keep_the_console(self): self.assertFalse(quiet(viewer_requested, ['--no-such-flag'])) +class TestThemeFlag(unittest.TestCase): + def _parse(self, argv): + from compare_tool.main import _parser + return _parser().parse_args(argv) + + def test_the_default_is_dark(self): + from compare_tool import theme + self.assertEqual(self._parse(['old', 'new']).theme, theme.DARK) + + def test_light_is_accepted(self): + self.assertEqual(self._parse(['old', 'new', '--theme', 'light']).theme, + 'light') + + def test_an_unknown_scheme_is_a_usage_error_not_a_silent_fallback(self): + # on the command line a typo should be told, not guessed at; the + # fallback in theme.normalize is for values read back from a file + with self.assertRaises(SystemExit): + quiet(self._parse, ['old', 'new', '--theme', 'puce']) + + def test_the_flag_does_not_count_as_a_folder(self): + self.assertTrue(viewer_requested(['--theme', 'light'])) + self.assertFalse(viewer_requested(['old', 'new', '--theme', 'light'])) + + class TestTkinterPanelIsGone(unittest.TestCase): def test_gui_flag_is_rejected(self): self.assertFalse(quiet(viewer_requested, ['--gui'])) diff --git a/tests/test_diffpane_qt.py b/tests/test_diffpane_qt.py index 6742790..98ed466 100644 --- a/tests/test_diffpane_qt.py +++ b/tests/test_diffpane_qt.py @@ -505,5 +505,158 @@ def test_hiding_rows_never_changes_a_verdict_or_the_counts(self): self.assertEqual(set(self.win._raw_results), set(raw)) +@unittest.skipUnless(HAVE_QT, 'PySide6 not installed') +class TestMutedCategories(unittest.TestCase): + """Switching a noise category off greys its lines instead of removing them. + + Two claims have to hold together. The lines stay -- they are the context + the surviving changes are read in, and a regenerated file is mostly banner + churn, so dropping them left the real hunks floating. And they stop + counting as changes everywhere that answers "where should I look next": + the minimap and F7/F8. + """ + + REL = 'a2l/cal.a2l' # one comment hunk and one real one + + def setUp(self): + from compare_tool.qtviewer.diffpane import DiffPane + self.app = _app() + self.results = scan(FIX / 'old', FIX / 'new') + self.pane = DiffPane() + self.addCleanup(self.pane.deleteLater) + + def _show(self, muted=()): + self.pane.set_muted_modes(muted) + self.pane.show_file(self.REL, self.results[self.REL], + str(FIX / 'old'), str(FIX / 'new')) + for _ in range(5): + self.app.processEvents() + return self.pane.rows + + def test_no_line_is_taken_away(self): + plain = [(r.old_no, r.old_txt, r.new_no, r.new_txt) for r in self._show()] + muted = [(r.old_no, r.old_txt, r.new_no, r.new_txt) + for r in self._show(('comment',))] + self.assertEqual(muted, plain) + + def test_the_comment_rows_lose_their_diff_colour(self): + from compare_tool import theme + from compare_tool.view_model import MUTED + rows = self._show() + i = next(k for k, r in enumerate(rows) if r.mode == 'comment') + doc = self.pane.old_edit.document() + before = doc.findBlockByNumber(i).blockFormat().background().color().name() + self.assertEqual(before, theme.c('del-bg-dim')) + rows = self._show(('comment',)) + self.assertEqual(rows[i].mode, MUTED) + doc = self.pane.old_edit.document() + after = doc.findBlockByNumber(i).blockFormat().background().color().name() + self.assertEqual(after, theme.c('muted-bg')) + + def test_a_muted_row_carries_no_inline_highlight(self): + i = next(k for k, r in enumerate(self._show()) if r.mode == 'comment') + self._show(('comment',)) + self.assertEqual(_backgrounds(self.pane.old_edit)[i], ['block']) + + def test_the_minimap_stops_marking_them_as_changes(self): + self._show() + with_noise = [k for k, r in enumerate(self.pane.minimap._rows) + if r.mode not in ('ctx', 'muted')] + self._show(('comment',)) + without = [k for k, r in enumerate(self.pane.minimap._rows) + if r.mode not in ('ctx', 'muted')] + self.assertLess(len(without), len(with_noise)) + self.assertTrue(without, 'the real change must still be on the map') + + def test_the_real_change_still_stops_where_it_did(self): + # muting moves no row, so a navigation stop needs no translation -- + # this is the assertion that would catch it if one ever did + before = list(self._show()) and list(self.pane._stops) + self._show(('comment', 'minor')) + self.assertEqual(self.pane._stops, before) + self.assertTrue(self.pane._stops) + + def test_a_real_change_is_never_muted(self): + from compare_tool.view_model import MUTED + rows = self._show(('real', 'moved', 'comment', 'minor')) + self.assertIn('real', [r.mode for r in rows]) + self.assertNotIn(MUTED, [r.mode for r in rows if r.kind == 'real']) + + +@unittest.skipUnless(HAVE_QT, 'PySide6 not installed') +class TestThemeSwitch(unittest.TestCase): + """The light theme has to reach every surface that stamps a colour in. + + Row backgrounds are block formats inside the document and tree colours are + per item, so neither follows a stylesheet swap -- the failure mode is half + a window in the old theme, which no assertion about the palette alone + would catch. + """ + + def setUp(self): + from compare_tool import theme + from compare_tool.qtviewer.app import MainWindow + self.theme = theme + self.app = _app() + self.addCleanup(theme.set_current, theme.DEFAULT) + self.win = MainWindow(str(FIX / 'old'), str(FIX / 'new')) + self.win.resize(1200, 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 _row_bgs(self): + doc = self.win.diff.old_edit.document() + return {doc.findBlockByNumber(i).blockFormat().background().color().name() + for i in range(doc.blockCount())} + + def test_the_viewer_opens_in_the_theme_it_was_asked_for(self): + from compare_tool.qtviewer.app import MainWindow + win = MainWindow(str(FIX / 'old'), str(FIX / 'new'), + theme_name=self.theme.LIGHT) + self.addCleanup(win.close) + self.assertEqual(self.theme.current(), self.theme.LIGHT) + + def test_switching_repaints_the_diff_rows_not_just_the_chrome(self): + self.win._reselect('src/real_change.c') + self._settle_ui() + dark = self._row_bgs() + self.assertIn(self.theme.color('del-bg', self.theme.DARK), dark) + self.win._set_theme(self.theme.LIGHT) + self._settle_ui() + light = self._row_bgs() + self.assertIn(self.theme.color('del-bg', self.theme.LIGHT), light) + self.assertNotIn(self.theme.color('del-bg', self.theme.DARK), light) + + def test_switching_repaints_the_tree_verdict_colours(self): + def first_colour(): + item = self.win.tree.topLevelItem(0) + return item.foreground(0).color().name() + + dark = first_colour() + self.win._set_theme(self.theme.LIGHT) + self._settle_ui() + self.assertNotEqual(first_colour(), dark) + + def test_the_file_on_screen_survives_the_switch(self): + self.win._reselect('src/real_change.c') + self._settle_ui() + self.win._set_theme(self.theme.LIGHT) + self._settle_ui() + self.assertEqual(self.win._selected_rel(), 'src/real_change.c') + self.assertEqual(self.win.diff._rel, 'src/real_change.c') + + def test_the_toggle_goes_back_and_forth(self): + self.win._toggle_theme() + self.assertEqual(self.theme.current(), self.theme.LIGHT) + self.win._toggle_theme() + self.assertEqual(self.theme.current(), self.theme.DARK) + + if __name__ == '__main__': unittest.main() diff --git a/tests/test_qtviewer.py b/tests/test_qtviewer.py index 473e7ef..b1bbf62 100644 --- a/tests/test_qtviewer.py +++ b/tests/test_qtviewer.py @@ -3,6 +3,7 @@ import unittest +from compare_tool import theme from compare_tool.qtviewer.tree import (PRIO, REVIEW_COLOR, STATUS, build_nodes, filter_nodes, review_state) @@ -54,8 +55,12 @@ def test_backslash_paths_split_like_posix(self): def test_every_status_has_metadata(self): for st in PRIO: self.assertIn(st, STATUS) - marker, label, color = STATUS[st] - self.assertTrue(marker and label and color.startswith('#')) + marker, label, role = STATUS[st] + self.assertTrue(marker and label and role) + # the role has to exist in every theme, or the tree paints one + # verdict with a KeyError instead of a colour + for name in theme.THEMES: + self.assertTrue(theme.color(role, name).startswith('#')) class TestFilterNodes(unittest.TestCase): @@ -144,7 +149,10 @@ def test_one_unreviewed_change_is_never_done(self): def test_every_state_has_a_colour(self): for reviewed, total in ((0, 1), (1, 2), (2, 2)): - self.assertIn(review_state(reviewed, total), REVIEW_COLOR) + state = review_state(reviewed, total) + self.assertIn(state, REVIEW_COLOR) + for name in theme.THEMES: + self.assertTrue(theme.color(REVIEW_COLOR[state], name)) if __name__ == '__main__': diff --git a/tests/test_report.py b/tests/test_report.py index ed70458..c521978 100644 --- a/tests/test_report.py +++ b/tests/test_report.py @@ -268,8 +268,17 @@ class TestOneColourLanguage(unittest.TestCase): still has to see which hunks count.""" @staticmethod - def _bg(selector): - """The background colour a CSS rule sets, as (r, g, b).""" + def _rgb(value): + h = value.lstrip('#') + return tuple(int(h[i:i + 2], 16) for i in (0, 2, 4)) + + def _bg(self, selector, theme_name): + """The background colour a CSS rule sets in one theme, as (r, g, b). + + The rules name theme roles, so the var has to be resolved against the + palette the page would be showing -- which is also what makes these + claims testable in BOTH themes instead of only the dark one.""" + from compare_tool import theme from compare_tool.report import _CSS # comments carry commas of their own, which would land inside the # selector list of the rule that follows them @@ -278,27 +287,42 @@ def _bg(selector): head, _, body = block.partition('{') if selector not in [s.strip() for s in head.split(',')]: continue - m = re.search(r'background:\s*#([0-9a-fA-F]{6})', body) + m = re.search(r'background:\s*var\(--([\w-]+)\)', body) if m: - h = m.group(1) - return tuple(int(h[i:i + 2], 16) for i in (0, 2, 4)) + return self._rgb(theme.color(m.group(1), theme_name)) raise AssertionError('no background for ' + selector) + def _themes(self): + from compare_tool import theme + return theme.THEMES + def test_removed_rows_are_red_on_every_category(self): - for sel in ('td.del', 'td.delm', 'td.delc'): - r, g, b = self._bg(sel) - self.assertGreater(r, g, sel) - self.assertGreater(r, b, sel) + for name in self._themes(): + for sel in ('td.del', 'td.delm', 'td.delc'): + r, g, b = self._bg(sel, name) + self.assertGreater(r, g, (name, sel)) + self.assertGreater(r, b, (name, sel)) def test_added_rows_are_green_on_every_category(self): - for sel in ('td.add', 'td.addm', 'td.addc'): - r, g, b = self._bg(sel) - self.assertGreater(g, r, sel) - self.assertGreater(g, b, sel) + for name in self._themes(): + for sel in ('td.add', 'td.addm', 'td.addc'): + r, g, b = self._bg(sel, name) + self.assertGreater(g, r, (name, sel)) + self.assertGreater(g, b, (name, sel)) def test_noise_is_dimmer_than_a_real_change(self): - self.assertLess(sum(self._bg('td.delm')), sum(self._bg('td.del'))) - self.assertLess(sum(self._bg('td.addm')), sum(self._bg('td.add'))) + # 'dimmer' means nearer the page: darker on the dark theme, paler on + # the light one, so the claim is distance from the background, not + # brightness + from compare_tool import theme + for name in self._themes(): + page = self._rgb(theme.color('bg', name)) + + def gap(sel): + return sum(abs(a - b) for a, b in zip(self._bg(sel, name), page)) + + self.assertLess(gap('td.delm'), gap('td.del'), name) + self.assertLess(gap('td.addm'), gap('td.add'), name) def test_the_legend_no_longer_offers_a_noise_swatch(self): # a swatch for a colour the reader cannot tell from 'real change' @@ -349,6 +373,55 @@ def test_a_label_is_escaped_like_any_other_text(self): self.assertIn('<script>', page) +class TestPageTheme(unittest.TestCase): + """The report carries BOTH palettes and a switch between them. + + It is mailed around and opened on machines with no internet, so the switch + cannot fetch a stylesheet; and the flag the report was built with is only + a default, because whoever opens it is the one looking at it. + """ + + def setUp(self): + self.results = scan(FIX / 'old', FIX / 'new') + + def _page(self, **kw): + return build_report(self.results, FIX / 'old', FIX / 'new', **kw) + + def test_the_default_is_dark(self): + from compare_tool import theme + self.assertEqual(theme.DEFAULT, theme.DARK) + self.assertIn('', self._page()) + + def test_the_flag_chooses_which_one_it_opens_with(self): + self.assertIn('', self._page(theme_name='light')) + + def test_an_unknown_theme_name_falls_back_instead_of_raising(self): + self.assertIn('', self._page(theme_name='puce')) + + def test_both_palettes_are_embedded_whichever_one_it_opens_with(self): + from compare_tool import theme + for name in (None, 'light'): + page = self._page() if name is None else self._page(theme_name=name) + self.assertIn(theme.color('bg', theme.DARK), page) + self.assertIn(theme.color('bg', theme.LIGHT), page) + self.assertIn('html[data-theme="light"]', page) + + def test_the_switch_is_on_the_page_and_needs_nothing_downloaded(self): + page = self._page() + self.assertIn('id="thm"', page) + self.assertIn('tgtheme()', page) + # the compared files' own text is full of URLs (xmlns=…), so what is + # checked is the ways a PAGE fetches something, not the string http + for fetch in ('', page) + self.assertIn('id="thm"', page) + + class TestModelGrouping(unittest.TestCase): """File grouping by Embedded Coder model naming (X.c, X_*.h, Rte_X.h).""" diff --git a/tests/test_syntax.py b/tests/test_syntax.py index 930b4ac..2d9e4e6 100644 --- a/tests/test_syntax.py +++ b/tests/test_syntax.py @@ -27,9 +27,8 @@ def test_arxml_and_xml(self): self.assertEqual(syntax.language_for('swc.arxml'), 'arxml') self.assertEqual(syntax.language_for('a.xml'), 'arxml') - def test_a2l_is_deliberately_not_highlighted(self): - # a flat keyword soup: colouring it lights up nearly every line - self.assertIsNone(syntax.language_for('project.a2l')) + def test_a2l(self): + self.assertEqual(syntax.language_for('project.a2l'), 'a2l') def test_unknown_extension_stays_plain(self): self.assertIsNone(syntax.language_for('notes.txt')) @@ -128,6 +127,74 @@ def test_c_line_comment_is_not_a_comment_in_xml(self): self.assertEqual([k for _t, k in got if k == syntax.COMMENT], []) +class TestA2L(unittest.TestCase): + """A2L colouring earns its keep only if the object NAMES stay plain. + + The format is a soup of ALL-CAPS words; a `[A-Z_]+` rule would light up the + calibration names too, and those are the one thing a reviewer scans an a2l + diff for. So: keywords and enum literals from a list, block names from + their position, and everything else untouched. + """ + + LINE = ' /begin CHARACTERISTIC K_Gain "controller gain" VALUE 0x8000' + + def test_begin_and_end_are_keywords(self): + self.assertIn(('/begin', syntax.KEYWORD), kinds(self.LINE, 'a2l')) + self.assertIn(('/end', syntax.KEYWORD), + kinds(' /end CHARACTERISTIC', 'a2l')) + + def test_the_block_name_is_typed_by_its_position(self): + # so a vendor block nobody listed still reads like every other one + self.assertIn(('CHARACTERISTIC', syntax.TYPE), kinds(self.LINE, 'a2l')) + self.assertIn(('ACME_PRIVATE', syntax.TYPE), + kinds('/begin ACME_PRIVATE x', 'a2l')) + + def test_the_object_name_stays_plain(self): + self.assertNotIn('K_Gain', dict(kinds(self.LINE, 'a2l'))) + + def test_an_all_caps_object_name_is_not_mistaken_for_a_keyword(self): + got = dict(kinds('/begin MEASUREMENT ENG_SPD_MAX "rpm" UWORD', 'a2l')) + self.assertNotIn('ENG_SPD_MAX', got) + self.assertEqual(got.get('UWORD'), syntax.TYPE) + + def test_attribute_keywords_and_numbers(self): + got = kinds(' ECU_ADDRESS 0x40001000', 'a2l') + self.assertIn(('ECU_ADDRESS', syntax.KEYWORD), got) + self.assertIn(('0x40001000', syntax.NUMBER), got) + + def test_a_doubled_quote_stays_inside_the_string(self): + # A2L escapes a quote by doubling it; ending the literal at the first + # one would colour the rest of the line as code and the next string as + # its own + got = kinds('x "say ""hi"" now" FORMAT', 'a2l') + self.assertIn(('"say ""hi"" now"', syntax.STRING), got) + self.assertIn(('FORMAT', syntax.KEYWORD), got) + + def test_a_backslash_is_a_literal_not_an_escape(self): + # a Windows path ending in one would otherwise swallow the code after + # it -- the trap a2l_rules exists to avoid, in the highlighter too + got = kinds(r'PROJECT "C:\build\out\" FORMAT', 'a2l') + self.assertIn((r'"C:\build\out\"', syntax.STRING), got) + self.assertIn(('FORMAT', syntax.KEYWORD), got) + + def test_comment_markers_inside_a_string_do_not_open_a_comment(self): + got, state = syntax.spans('DESC "a /* b" ECU_ADDRESS 1', 'a2l') + self.assertEqual(state, PLAIN) + self.assertEqual([k for _a, _b, k in got if k == syntax.COMMENT], []) + + def test_block_comments_carry_across_lines(self): + _spans, state = syntax.spans('/* generated by the toolchain', 'a2l') + self.assertEqual(state, IN_BLOCK_COMMENT) + _spans, state = syntax.spans(' on some date */ ASAP2_VERSION 1 71', + 'a2l', state) + self.assertEqual(state, PLAIN) + + def test_a_hash_is_not_a_preprocessor_line(self): + # A2L has no preprocessor; the C rule must not follow the language in + self.assertEqual([k for _a, _b, k in syntax.spans('# 1 "x"', 'a2l')[0] + if k == syntax.PREPROC], []) + + class TestSpanShape(unittest.TestCase): """Whatever the line, the Qt layer must be able to paint the spans in order without them fighting each other.""" @@ -139,6 +206,9 @@ class TestSpanShape(unittest.TestCase): ('c', 'if (strcmp(s, "/*") == 0) { return 1; }'), ('arxml', ' text'), ('arxml', ''), + ('a2l', ' /begin CHARACTERISTIC K_Gain "gain" VALUE 0x8000 RL 0 CM 1 2'), + ('a2l', ' ECU_ADDRESS 0x40001000 /* linker map */'), + ('a2l', ' /begin MOD_PAR "C:\\\\build\\\\out" // path with a backslash'), ] def test_spans_are_sorted_and_never_overlap(self): diff --git a/tests/test_theme.py b/tests/test_theme.py new file mode 100644 index 0000000..98b9c50 --- /dev/null +++ b/tests/test_theme.py @@ -0,0 +1,96 @@ +"""The shared colour palettes. + +The point of one seam per shared decision is that the surfaces cannot drift, so +what is tested here is that neither palette can go missing a role the other has, +that a role always resolves, and that the CSS emitter never writes a colour the +browser would read differently from Qt. +""" + +import re +import unittest + +from compare_tool import theme + + +class TestPalettes(unittest.TestCase): + def test_both_themes_define_exactly_the_same_roles(self): + # a role in one palette only paints one theme correctly and raises on + # the other -- on whichever surface nobody happened to look at + self.assertEqual(set(theme.PALETTES[theme.DARK]), + set(theme.PALETTES[theme.LIGHT])) + + def test_every_value_is_a_hex_colour(self): + for name in theme.THEMES: + for role, value in theme.palette(name).items(): + self.assertRegex(value, r'^#(?:[0-9a-f]{6}|[0-9a-f]{8})$', + '{} {}'.format(name, role)) + + def test_the_two_themes_are_actually_different(self): + dark, light = theme.palette(theme.DARK), theme.palette(theme.LIGHT) + self.assertNotEqual(dark['bg'], light['bg']) + + def test_dark_is_dark_and_light_is_light(self): + def lum(value): + h = value.lstrip('#') + return sum(int(h[i:i + 2], 16) for i in (0, 2, 4)) + + self.assertLess(lum(theme.color('bg', theme.DARK)), 250) + self.assertGreater(lum(theme.color('bg', theme.LIGHT)), 500) + + def test_text_contrasts_with_its_own_background(self): + # not a WCAG check, just the failure that actually happens: a role + # copied from the other theme and left there + for name in theme.THEMES: + for fg, bg in (('fg', 'bg'), ('code-fg', 'code-bg'), + ('muted-fg', 'muted-bg')): + def lum(role): + h = theme.color(role, name).lstrip('#') + return sum(int(h[i:i + 2], 16) for i in (0, 2, 4)) / 3 + + self.assertGreater(abs(lum(fg) - lum(bg)), 40, + '{} {} on {}'.format(name, fg, bg)) + + +class TestLookup(unittest.TestCase): + def test_an_unknown_theme_falls_back_instead_of_raising(self): + # a colour scheme is never worth refusing to open the tool over + self.assertEqual(theme.normalize('solarized'), theme.DEFAULT) + self.assertEqual(theme.normalize(None), theme.DEFAULT) + + def test_an_unknown_role_raises(self): + # the opposite call: a typo must not become a silently absent colour + with self.assertRaises(KeyError): + theme.color('no-such-role') + + def test_other_is_the_one_the_toggle_goes_to(self): + self.assertEqual(theme.other(theme.DARK), theme.LIGHT) + self.assertEqual(theme.other(theme.LIGHT), theme.DARK) + + def test_set_current_returns_what_took_effect(self): + before = theme.current() + try: + self.assertEqual(theme.set_current(theme.LIGHT), theme.LIGHT) + self.assertEqual(theme.c('bg'), theme.color('bg', theme.LIGHT)) + self.assertEqual(theme.set_current('nonsense'), theme.DEFAULT) + finally: + theme.set_current(before) + + +class TestCssVars(unittest.TestCase): + def test_every_var_is_a_six_digit_hex(self): + # Qt writes #aarrggbb and CSS reads #rrggbbaa, so an alpha role emitted + # here would define a WRONG colour, not merely an unused one + for name in theme.THEMES: + for value in re.findall(r'--[\w-]+:(#[0-9a-f]+);', + theme.css_vars(name)): + self.assertEqual(len(value), 7, '{} {}'.format(name, value)) + + def test_the_roles_the_report_uses_are_all_emitted(self): + from compare_tool.report import _CSS + emitted = set(re.findall(r'--([\w-]+):', theme.css_vars(theme.DARK))) + for role in set(re.findall(r'var\(--([\w-]+)\)', _CSS)): + self.assertIn(role, emitted, role) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/test_view_model.py b/tests/test_view_model.py index 3cf932f..0f5cb68 100644 --- a/tests/test_view_model.py +++ b/tests/test_view_model.py @@ -7,8 +7,8 @@ from compare_tool.diff_engine import compare_pair from compare_tool.report import _char_diff -from compare_tool.view_model import (Row, aligned_rows, char_span, - collapse_rows, hunk_row_starts, row_with) +from compare_tool.view_model import (MUTED, Row, aligned_rows, char_span, + hunk_row_starts, mute_rows, row_with) class TestCharSpan(unittest.TestCase): @@ -156,60 +156,62 @@ def test_no_hunks_no_starts(self): self.assertEqual(hunk_row_starts([]), []) -class TestCollapseRows(unittest.TestCase): - """Unticking a compare category hides its lines in the panes too. What must - hold: a real change can never be folded, and a fold always says how much it - hid.""" +class TestMuteRows(unittest.TestCase): + """Unticking a compare category plays its lines down in the panes. What + must hold: the lines stay (they are the context the surviving changes are + read in), they lose their diff colour, and a real change can never be + muted.""" def _rows(self, *specs): return [Row(i + 1, 'o{}'.format(i), i + 1, 'n{}'.format(i), mode, kind) for i, (mode, kind) in enumerate(specs)] - def test_a_run_becomes_one_row_stating_the_count(self): + def test_muted_rows_keep_their_place_text_and_numbers(self): rows = self._rows(('ctx', 'equal'), ('minor', 'uuid'), ('minor', 'uuid'), ('minor', 'uuid'), ('ctx', 'equal')) - out, _m = collapse_rows(rows, ['minor']) - self.assertEqual([r.mode for r in out], ['ctx', 'folded', 'ctx']) - self.assertIn('3 uuid lines hidden', out[1].old_txt) - - def test_both_sides_carry_the_same_placeholder_text(self): - # identical text on both sides: the row must read as context, not as a - # difference, and the panes must keep the same block count - out, _m = collapse_rows(self._rows(('minor', 'uuid')), ['minor']) - self.assertEqual(out[0].old_txt, out[0].new_txt) - self.assertIsNone(out[0].old_no) - self.assertIn('1 uuid line hidden', out[0].old_txt) - - def test_real_and_moved_can_never_be_folded(self): + out = mute_rows(rows, ['minor']) + self.assertEqual([r.mode for r in out], + ['ctx', MUTED, MUTED, MUTED, 'ctx']) + self.assertEqual(len(out), len(rows)) + for before, after in zip(rows, out): + self.assertEqual((before.old_no, before.old_txt, + before.new_no, before.new_txt), + (after.old_no, after.old_txt, + after.new_no, after.new_txt)) + + def test_a_muted_row_still_says_why(self): + # the kind survives, so the pane and the map can still tell a muted + # comment from a muted uuid without going back to the hunks + out = mute_rows(self._rows(('minor', 'uuid')), ['minor']) + self.assertEqual(out[0].kind, 'uuid') + + def test_real_and_moved_can_never_be_muted(self): rows = self._rows(('real', 'real'), ('moved', 'moved')) - out, _m = collapse_rows(rows, ['real', 'moved', 'minor']) + out = mute_rows(rows, ['real', 'moved', 'minor']) self.assertEqual([r.mode for r in out], ['real', 'moved']) - def test_only_the_unticked_category_folds(self): + def test_only_the_unticked_category_is_muted(self): rows = self._rows(('comment', 'comment'), ('minor', 'uuid')) - out, _m = collapse_rows(rows, ['comment']) - self.assertEqual([r.mode for r in out], ['folded', 'minor']) - out, _m = collapse_rows(rows, ['comment', 'minor']) - self.assertEqual([r.mode for r in out], ['folded']) - self.assertIn('comment + uuid', out[0].old_txt) + out = mute_rows(rows, ['comment']) + self.assertEqual([r.mode for r in out], [MUTED, 'minor']) + out = mute_rows(rows, ['comment', 'minor']) + self.assertEqual([r.mode for r in out], [MUTED, MUTED]) - def test_row_map_carries_navigation_stops_across(self): + def test_row_indices_are_untouched_so_navigation_stops_still_point_home(self): rows = self._rows(('minor', 'uuid'), ('minor', 'uuid'), ('real', 'real'), ('minor', 'uuid'), ('real', 'real')) - out, row_map = collapse_rows(rows, ['minor']) - self.assertEqual([r.mode for r in out], - ['folded', 'real', 'folded', 'real']) - self.assertEqual(row_map[2], 1) # the real rows still land on themselves - self.assertEqual(row_map[4], 3) - for i, r in enumerate(rows): - if r.mode == 'real': - self.assertEqual(out[row_map[i]].mode, 'real') + out = mute_rows(rows, ['minor']) + self.assertEqual([i for i, r in enumerate(out) if r.mode == 'real'], + [i for i, r in enumerate(rows) if r.mode == 'real']) def test_no_modes_is_the_identity(self): rows = self._rows(('minor', 'uuid'), ('real', 'real')) - out, row_map = collapse_rows(rows, []) - self.assertEqual(out, rows) - self.assertEqual(row_map, [0, 1]) + self.assertEqual(mute_rows(rows, []), rows) + + def test_the_input_is_left_alone(self): + rows = self._rows(('minor', 'uuid')) + mute_rows(rows, ['minor']) + self.assertEqual(rows[0].mode, 'minor') if __name__ == '__main__': From 6f0c738099ced804a5f5ffe123f43d5c235d4ff3 Mon Sep 17 00:00:00 2001 From: longvo920 Date: Thu, 30 Jul 2026 21:31:58 +0700 Subject: [PATCH 2/7] fix: theme switch must not overwrite the reader's choice or their place Four things found reviewing the previous commit. The report persisted the theme on load, not only on a click. Opening one report built with --theme light therefore wrote 'light' into localStorage, and every later report opened light -- the flag silently became a preference the reader never expressed. It is saved on a toggle now, and only then. Switching the theme in the viewer sent the reviewer back to change 1. The pane does restore its own position across the re-render, but the window rebuilds the tree afterwards, and re-selecting the open file re-renders it a second time and undoes that. The position is now taken before the repaint and handed back after all of it, through reading_position / restore_reading_position rather than MainWindow reaching into the pane. The report's folded-run placeholders shared --muted-fg with the viewer's greyed line. They sit on different backgrounds, so tuning the role for the viewer's band dimmed the report's rows with it: exactly the drift the seam exists to stop. They have the verdict grey back. Also: the theme button could keep a checked state the window was not in when the repaint was skipped, and the review-file label kept the previous theme's colour while no review file was loaded. --- compare_tool/qtviewer/app.py | 31 +++++++++++++++++------- compare_tool/qtviewer/diffpane.py | 40 ++++++++++++++++++++++++++----- compare_tool/report.py | 21 ++++++++++------ tests/test_diffpane_qt.py | 15 ++++++++++++ tests/test_report.py | 16 +++++++++++++ 5 files changed, 101 insertions(+), 22 deletions(-) diff --git a/compare_tool/qtviewer/app.py b/compare_tool/qtviewer/app.py index 90ec0cb..37c7d5f 100644 --- a/compare_tool/qtviewer/app.py +++ b/compare_tool/qtviewer/app.py @@ -298,16 +298,24 @@ def _set_theme(self, name): Missing one leaves half the window in the old theme, which is why they are listed here and not discovered by walking children. """ - if theme.set_current(name) == self._theme: - return + changed = theme.set_current(name) != self._theme self._theme = theme.current() - apply_theme(QApplication.instance()) - self._style_widgets() - self._set_state(*self._state) - self._apply_icons() - self.summary.apply_theme() - self.diff.apply_theme() - self._refresh_tree_keep_selection() # verdict colours are per item + if changed: + # taken FIRST and handed back LAST: rebuilding the tree re-selects + # the open file, which re-renders it and parks on its first change. + # Restoring inside the pane alone would be undone a line later. + at = self.diff.reading_position() + apply_theme(QApplication.instance()) + self._style_widgets() + self._set_state(*self._state) + self._apply_icons() + self.summary.apply_theme() + self.diff.apply_theme() + self._refresh_tree_keep_selection() # verdict colours are per item + self.diff.restore_reading_position(at) + # outside the guard: the toolbar button is checkable, so Qt has already + # flipped its state by the time this runs. Skipping the repaint must + # not leave the button claiming a theme the window is not in. self.act_theme.setText(self._theme_label()) self.act_theme.setChecked(self._theme == theme.LIGHT) @@ -655,7 +663,12 @@ def _load_review(self): def _show_review_file(self): path = self._reviews.path if path is None: + # the stylesheet is set even with nothing to show: this label is + # red while a review file is broken, and a theme switch that left + # the old red behind would outlive the state that earned it self.review_file.setText('') + self.review_file.setStyleSheet('color:{}; font-size:11px;' + .format(theme.c('fg-muted'))) return if self._reviews.error: self.review_file.setText('⚠ {}'.format(path.name)) diff --git a/compare_tool/qtviewer/diffpane.py b/compare_tool/qtviewer/diffpane.py index 3a6859a..d59e00a 100644 --- a/compare_tool/qtviewer/diffpane.py +++ b/compare_tool/qtviewer/diffpane.py @@ -404,12 +404,40 @@ def apply_theme(self): editor.apply_theme() hl.apply_theme() self.minimap.update() - if self._last is not None and self.currentIndex() == 1: - # re-rendering parks on change 1 again; put the reviewer back where - # they were reading -- a colour switch is not a navigation command - at = self._drive.verticalScrollBar().value() - self.show_file(*self._last) - self._drive.verticalScrollBar().setValue(at) + if self._last is None or self.currentIndex() != 1: + return + at = self.reading_position() + self.show_file(*self._last) + self.restore_reading_position(at) + + def reading_position(self): + """Where the reviewer is in the file on screen, as an opaque token. + + Re-rendering a file parks on its first change, and a repaint is not a + navigation command -- so anything that re-renders has to take this + first and hand it back afterwards. None when there is no file to hold + a position in.""" + if self.currentIndex() != 1: + return None + return (self._rel, self._drive.textCursor().blockNumber(), + self._drive.verticalScrollBar().value()) + + def restore_reading_position(self, at): + """Put the cursor, the current-change overlay, the `change k of N` and + the scroll back where :meth:`reading_position` found them. + + Ignored when the file changed underneath it: landing a stale row number + on a different file would scroll somewhere arbitrary.""" + if not at or at[0] != self._rel or self.currentIndex() != 1: + return + _rel, row, scroll = at + if row < len(self.rows): + self._drive.setTextCursor( + QTextCursor(self._drive.document().findBlockByNumber(row))) + self._highlight_block(row) + self._update_position(row) + self._drive.verticalScrollBar().setValue(scroll) + self.unitChanged.emit() @staticmethod def _pane(banner, editor): diff --git a/compare_tool/report.py b/compare_tool/report.py index 7af55b0..01d5837 100644 --- a/compare_tool/report.py +++ b/compare_tool/report.py @@ -28,7 +28,7 @@ .summary { margin: 14px 0 22px; } .badge { display: inline-block; padding: 2px 10px; border-radius: 10px; font-size: 12px; margin-right: 8px; cursor: pointer; user-select: none; border: 1px solid transparent; } -.badge:hover { border-color: var(--border-strong); } +.badge:hover { border-color: var(--fg-muted); } .badge.off { opacity: .35; text-decoration: line-through; } .b-real { background: var(--tag-real-bg); color: var(--tag-real-fg); } .b-ign { background: var(--tag-ign-bg); color: var(--tag-ign-fg); } @@ -121,8 +121,11 @@ silently dropped. (The viewer still shows them in full -- it is the reading surface, this is the record to send.) */ tr.comment, .grp-cmt { display: none; } -tr.commentph { display: table-row; color: var(--muted-fg); } -tr.minorph td { color: var(--muted-fg); } +/* the folded-run placeholders. NOT --muted-fg: that one is the viewer's greyed + line, on the editor background, and tuning it for that band dragged these + rows down with it. Different surface, different role. */ +tr.commentph { display: table-row; color: var(--st-cmt); } +tr.minorph td { color: var(--st-cmt); } .filenote { color: var(--fg-muted); font-size: 12px; margin: 2px 0 10px; } .renames { font-size: 12px; color: var(--st-ign); margin: 2px 0 8px; } .iflist { font-family: Consolas, monospace; font-size: 13px; background: var(--panel); @@ -228,16 +231,20 @@ _THEME_BUTTON = ('') +# `save` is only true on a click. Persisting on load as well would turn the +# --theme flag INTO the reader's preference the first time they open a report +# built with it -- silently answering, on their behalf, a question they never +# answered. _THEME_JS = ( - 'function sttheme(t){document.documentElement.setAttribute("data-theme",t);' + 'function sttheme(t,save){document.documentElement.setAttribute("data-theme",t);' 'var b=document.getElementById("thm");' 'if(b)b.innerHTML=(t==="dark"?"\\u2600 Light":"\\u263e Dark");' - 'try{localStorage.setItem("cgc-theme",t);}catch(e){}}' + 'if(save){try{localStorage.setItem("cgc-theme",t);}catch(e){}}}' 'function tgtheme(){sttheme(document.documentElement.getAttribute("data-theme")' - '==="dark"?"light":"dark");}' + '==="dark"?"light":"dark",true);}' '(function(){var t=null;try{t=localStorage.getItem("cgc-theme");}catch(e){}' 'sttheme(t==="dark"||t==="light"?t' - ':document.documentElement.getAttribute("data-theme"));})();') + ':document.documentElement.getAttribute("data-theme"),false);})();') def _head(title, initial, body_class=''): diff --git a/tests/test_diffpane_qt.py b/tests/test_diffpane_qt.py index 98ed466..4f56515 100644 --- a/tests/test_diffpane_qt.py +++ b/tests/test_diffpane_qt.py @@ -651,6 +651,21 @@ def test_the_file_on_screen_survives_the_switch(self): self.assertEqual(self.win._selected_rel(), 'src/real_change.c') self.assertEqual(self.win.diff._rel, 'src/real_change.c') + def test_the_change_being_read_survives_the_switch(self): + # re-rendering the file parks on change 1; a colour switch is not a + # navigation command, so the reviewer must come back to where they were + self.win._reselect('src/rename_conflict.c') + self._settle_ui() + self.assertTrue(self.win.diff.next_change()) + self._settle_ui() + row = self.win.diff._drive.textCursor().blockNumber() + idx = self.win.diff._cur_idx + self.assertGreater(idx, 0) + self.win._set_theme(self.theme.LIGHT) + self._settle_ui() + self.assertEqual(self.win.diff._drive.textCursor().blockNumber(), row) + self.assertEqual(self.win.diff._cur_idx, idx) + def test_the_toggle_goes_back_and_forth(self): self.win._toggle_theme() self.assertEqual(self.theme.current(), self.theme.LIGHT) diff --git a/tests/test_report.py b/tests/test_report.py index c521978..0b9efa6 100644 --- a/tests/test_report.py +++ b/tests/test_report.py @@ -415,6 +415,22 @@ def test_the_switch_is_on_the_page_and_needs_nothing_downloaded(self): for fetch in (''):] + # the load-time call passes save=false; only the click passes true + self.assertIn('sttheme(t==="dark"||t==="light"?t', js) + self.assertIn('),false);})();', js) + self.assertIn('"dark",true);}', js) + self.assertEqual(js.count('localStorage.setItem'), 1) + self.assertIn('if(save){try{localStorage.setItem', js) + def test_the_arxml_report_switches_too(self): page = build_arxml_report(self.results, FIX / 'old', FIX / 'new', theme_name='light') From 2001dbd48b57202fc8397c370dbfe2a66b4571cb Mon Sep 17 00:00:00 2001 From: longvo920 Date: Thu, 30 Jul 2026 21:54:38 +0700 Subject: [PATCH 3/7] feat: report reveals Comment/Unimportant lines behind their badge, greyed The report used to draw a hard line the viewer no longer draws: Unimportant hid behind a badge but revealed in the same dim red/green as a real change, and Comment had no toggle at all -- always display:none, permanently a placeholder count. Now both hide by default and both reveal the same way: click the badge, the actual lines show in flat grey, no red/green tint and no character-level highlight, so a revealed noise section still reads as "does not count" instead of looking like another change. Comment gets its own badge, matching the rule that comment-only is a separate category from ignorable-only. A whole file whose ONLY differences are comments still gets no detail section -- that is a separate, wider question nobody asked for -- but comment hunks mixed into a real-change or Unimportant file's section now behave exactly like minor ones. Fixed along the way: a group whose every hunk was noise used to be wrapped in a single div and hidden as a whole (.grp-min / .grp-cmt), which took its placeholder and its own leading/trailing context lines down with it -- a file that was entirely Unimportant opened to a genuinely empty box under its own summary line, never showing even the count the placeholder exists to state. Hiding is per row now (tr.minor / tr.comment), so a pure-noise group renders exactly like a mixed one. Report only; the viewer's mute_rows behaviour from the previous commit is unchanged. Both surfaces now default-hide the same two categories and reveal them in a muted colour, but the report's "hidden" still means gone (CSS display:none, no row in the layout) where the viewer's means present-but-grey -- a report is exported once for someone else to read top to bottom, the viewer is where the reviewer works file by file, and that difference in what "hidden" costs is still worth keeping. --- README.md | 6 +- compare_tool/report.py | 91 ++++++++++------------- docs/vi/README.md | 18 +++-- tests/test_report.py | 160 ++++++++++++++++++++++++++--------------- 4 files changed, 156 insertions(+), 119 deletions(-) diff --git a/README.md b/README.md index bc1851f..d4c5b47 100644 --- a/README.md +++ b/README.md @@ -141,7 +141,7 @@ A shorter name can stop an argument wrapping at 80 columns, so the two sides hol Everything else keeps its suffix as meaning. `SIG_TORQUE_MIN` → `SIG_TORQUE_MAX` and `CFG_TIMEOUT_MS` → `CFG_TIMEOUT_US` are real changes, and so are `rtb_AND_…` → `rtb_OR_…` (a different block drives that buffer) and `Sub_…_step` → `Sub_…_Init` (a different entry point). Digits glued to a block name (`rtb_Switch1` vs `rtb_Switch2`) are part of the name, not a mangle tail. -**Comment changes are their own category.** A file whose differences are *only* comments is reported as **Comment**, separate from **Unimportant** (UUIDs, timestamps, SW-VERSION, renames, whitespace) — a rewritten comment banner triages differently from a renamed identifier. Separate counts in the CLI summary and its own tree marker in the viewer. A file mixing comments *with* other noise stays Unimportant. The HTML report keeps the verdict but does not display comment content — see [HTML report](#html-report). +**Comment changes are their own category.** A file whose differences are *only* comments is reported as **Comment**, separate from **Unimportant** (UUIDs, timestamps, SW-VERSION, renames, whitespace) — a rewritten comment banner triages differently from a renamed identifier. Separate counts in the CLI summary and its own tree marker in the viewer. A file mixing comments *with* other noise stays Unimportant. In the HTML report, `Comment` and `Unimportant` are each a badge of their own — see [HTML report](#html-report). ## Moved block detection @@ -175,7 +175,9 @@ Files are grouped by **Simulink model** using the Embedded Coder AUTOSAR naming ## HTML report -Self-contained file, one per compare: badge toggles, folder tree, filter box, collapsible diffs per file. Opens `Unimportant` hidden and `Modified` expanded, so it opens on what matters. A `☀ Light` / `☾ Dark` button sits in the top right — both palettes are embedded in the file, so switching fetches nothing and works on a machine with no internet. +Self-contained file, one per compare: badge toggles, folder tree, filter box, collapsible diffs per file. Opens `Unimportant` and `Comment` hidden, `Modified` expanded, so it opens on what matters. Clicking either badge reveals the actual comment/noise lines — painted flat grey rather than red/green, so a revealed category still reads as "does not count" instead of looking like another change. A `☀ Light` / `☾ Dark` button sits in the top right — both palettes are embedded in the file, so switching fetches nothing and works on a machine with no internet. + +A whole file with nothing but comment differences still gets no detail section of its own (there is nothing beyond the comment lines to show); it keeps its own `≉` mark and `Comment` count in the folder tree either way. ![Report viewer](resources/pic/report_page.png) diff --git a/compare_tool/report.py b/compare_tool/report.py index 01d5837..450471d 100644 --- a/compare_tool/report.py +++ b/compare_tool/report.py @@ -32,6 +32,7 @@ .badge.off { opacity: .35; text-decoration: line-through; } .b-real { background: var(--tag-real-bg); color: var(--tag-real-fg); } .b-ign { background: var(--tag-ign-bg); color: var(--tag-ign-fg); } +.b-cmt { background: var(--tag-cmt-bg); color: var(--tag-cmt-fg); } .b-id { background: var(--tag-id-bg); color: var(--tag-id-fg); } /* added and deleted share one control: both are "a whole file appeared or vanished", and a reviewer flips them together */ @@ -83,49 +84,38 @@ word-break: break-all; border: none; } td.ln { width: 44px; color: var(--ln-fg); text-align: right; user-select: none; } td.del { background: var(--del-bg); } td.add { background: var(--add-bg); } -/* Noise (comment, uuid, rename, whitespace) uses the SAME red/green as a real - change, one notch dimmer -- one colour language instead of three. Yellow and - purple were a third and fourth hue competing with the syntax colours for the - reader's attention, and a diff that needs a legend to be read is too loud. - Dimmer, not identical: inside a Modified file the reviewer still has to see - which hunks are the ones that count. */ -td.delm, td.delc { background: var(--del-bg-dim); } -td.addm, td.addc { background: var(--add-bg-dim); } +/* Comment and Unimportant hide behind their own badge, default OFF -- the + report opens on real changes, a click reveals the rest. Revealed rows are + flat neutral grey, not a dim red/green: a toggled-open noise section still + has to read as "off to the side", off the one-colour-language rule real + changes and moved blocks use, not a quieter member of it. No character-level + highlight either (see _row) -- marking what changed inside a line nobody + was asked to read closely would be noise on noise. */ +td.delm, td.delc, td.addm, td.addc { background: var(--muted-bg); color: var(--muted-fg); } td.mvd, td.mva { background: var(--mv-bg); } td.ctx { color: var(--fg-dim); } td.del .chg-seg { background: var(--seg-del-bg); color: var(--seg-del-fg); font-weight: 700; border-radius: 2px; } td.add .chg-seg { background: var(--seg-add-bg); color: var(--seg-add-fg); font-weight: 700; border-radius: 2px; } -td.delm .chg-seg, td.delc .chg-seg { background: var(--seg-del-dim-bg); - color: var(--seg-del-dim-fg); font-weight: 700; - border-radius: 2px; } -td.addm .chg-seg, td.addc .chg-seg { background: var(--seg-add-dim-bg); - color: var(--seg-add-dim-fg); font-weight: 700; - border-radius: 2px; } .sw { display: inline-block; width: 10px; height: 10px; border-radius: 2px; margin: 0 4px 0 2px; vertical-align: -1px; } .sw-del { background: var(--seg-del-bg); } .sw-add { background: var(--seg-add-bg); } -.sw-mv { background: var(--seg-mv-bg); } +.sw-mv { background: var(--seg-mv-bg); } .sw-mut { background: var(--muted-bg); } tr.gap td { text-align: center; color: var(--gap-fg); background: var(--panel-2); font-size: 11px; } tr.mvnote td { text-align: center; color: var(--mv-fg); background: var(--panel-2); font-size: 11px; } -body.hide-ign tr.minor, body.hide-ign .grp-min { display: none; } -tr.minorph { display: none; } +/* Comment and Unimportant rows hide per ROW, not per group: a group used to + be wrapped whole and hidden together, which took the placeholder below and + the ordinary context lines around it down with it -- a noise-only file + opened to an empty box under its own summary line. */ +body.hide-ign tr.minor { display: none; } +body.hide-cmt tr.comment { display: none; } +tr.minorph, tr.commentph { display: none; } body.hide-ign tr.minorph { display: table-row; } -/* Comment churn is not a reported category here at all -- a regenerated banner - is the noisiest and least informative thing a codegen diff produces. The rows - stay IN the file, because the report is the record, but they are never shown; - the placeholder always states how many lines were folded, so nothing is - silently dropped. (The viewer still shows them in full -- it is the reading - surface, this is the record to send.) */ -tr.comment, .grp-cmt { display: none; } -/* the folded-run placeholders. NOT --muted-fg: that one is the viewer's greyed - line, on the editor background, and tuning it for that band dragged these - rows down with it. Different surface, different role. */ -tr.commentph { display: table-row; color: var(--st-cmt); } -tr.minorph td { color: var(--st-cmt); } +body.hide-cmt tr.commentph { display: table-row; } +tr.minorph td, tr.commentph td { color: var(--st-cmt); } .filenote { color: var(--fg-muted); font-size: 12px; margin: 2px 0 10px; } .renames { font-size: 12px; color: var(--st-ign); margin: 2px 0 8px; } .iflist { font-family: Consolas, monospace; font-size: 13px; background: var(--panel); @@ -424,29 +414,20 @@ def _group_notes(group, notes): def _groups_html(old_lines, new_lines, hunks, notes=None): - """All hunk groups of one file. A group with no real/moved hunk is - wrapped in .grp-min so the Unimportant badge hides it (label + context - included); minor rows inside mixed groups hide individually via tr.minor. + """All hunk groups of one file. Comment and Unimportant rows hide behind + their own badge individually (``tr.comment`` / ``tr.minor`` in the CSS) -- + a group used to be wrapped whole and hidden together when every hunk in it + was noise, which took the placeholder and the ordinary context lines + around it down with the rest: a noise-only file opened to an empty box. Moved blocks never hide: they are real changes, just shown in blue. - A group whose every real/moved hunk is signed off also gets .grp-rev, so - the Reviewed badge can fold it away -- with its notes, which belong to the + A group whose every real/moved hunk is signed off gets .grp-rev, so the + Reviewed badge can fold it away -- with its notes, which belong to the changes being hidden.""" out = [] for g in _group_hunks(hunks): - kinds = {h['kind'] for h in g} - # a group hides as a whole only when it is ONE hideable category; a - # group mixing comment with other noise would otherwise vanish behind - # a single badge, so its rows hide individually instead - if kinds == {'comment'}: - cls = ' grp-cmt' - elif not (kinds & {'real', 'moved'}): - cls = ' grp-min' - else: - cls = '' notes_html, done = _group_notes(g, notes) - if done: - cls += ' grp-rev' + cls = ' grp-rev' if done else '' out.append('
'.format(cls)) if any(h['kind'] != 'real' for h in g): out.append('
{}
'.format(_esc(_group_label(g)))) @@ -490,7 +471,9 @@ def _row(o_no, o_txt, n_no, n_txt, mode): dcls, acls = _MODE_CLS[mode] lcls = dcls if o_txt is not None else '' rcls = acls if n_txt is not None else '' - if o_txt is not None and n_txt is not None: + # comment/minor rows are muted grey, not a diff colour, when revealed + # -- so there is no changed SPAN to point at inside them either + if o_txt is not None and n_txt is not None and mode not in _MODE_TR: l, r = _char_diff(o_txt, n_txt) else: l = _esc(o_txt) if o_txt is not None else '' @@ -1199,7 +1182,7 @@ def build_report(results, old_root, new_root, reviews=None, old_label=None, parts = [] parts.append(_head('AUTOSAR Code Generation Report', theme_name, - body_class='hide-ign')) + body_class='hide-ign hide-cmt')) parts.append('

AUTOSAR Code Generation Report

') parts.append('
{} → {} · {}
'.format( _root_html('BASELINE', old_root, old_label), _root_html('CURRENT', new_root), now)) @@ -1228,11 +1211,13 @@ def build_report(results, old_root, new_root, reviews=None, old_label=None, parts.append('
' + err_badge + '{real-change} Modified' '{ignorable-only} Unimportant' + '{comment-only} Comment' '' '{added} Added / {deleted} Deleted' ''.format(**counts) + rev_group + '
') - hint = ('Click a badge to show/hide a category. Unimportant starts hidden ' - '— only real changes are shown.') + hint = ('Click a badge to show/hide a category. Unimportant and Comment ' + 'start hidden and, revealed, show in grey rather than red/green ' + '— only real changes keep that colour.') if rev_group: hint += (' Reviewed starts shown: click it to hide the changes ' 'already signed off.') @@ -1254,12 +1239,10 @@ def build_report(results, old_root, new_root, reviews=None, old_label=None, if detail_files: parts.append('

Detailed changes

') - # two entries only: noise now shares the red/green of a real change, so - # a swatch for it would describe a colour the reader cannot tell apart, - # and comment rows are never displayed here at all parts.append('
' '/removed / added ' - 'moved block
') + 'moved block ' + 'Comment / Unimportant, revealed
') parts.append('
' '' '' diff --git a/docs/vi/README.md b/docs/vi/README.md index 4348a34..6e37d55 100644 --- a/docs/vi/README.md +++ b/docs/vi/README.md @@ -189,8 +189,8 @@ một phần của tên, không phải đuôi mangle. được báo là **Comment**, tách khỏi **Unimportant** (UUID, timestamp, SW-VERSION, rename, whitespace) — một banner comment bị viết lại triage khác hẳn một identifier bị đổi tên. Đếm riêng trong summary của CLI và có marker riêng trên cây của viewer. -File trộn comment *với* noise loại khác thì vẫn là Unimportant. HTML report giữ -verdict nhưng không hiện nội dung comment — xem [HTML report](#html-report). +File trộn comment *với* noise loại khác thì vẫn là Unimportant. Trong HTML report, +`Comment` và `Unimportant` mỗi cái có badge riêng — xem [HTML report](#html-report). ## Phát hiện block bị di chuyển @@ -240,10 +240,16 @@ model nào rơi vào nhóm cuối **Shared / other**. ## HTML report File self-contained, mỗi lần compare một file: badge bật/tắt, cây thư mục, ô lọc, -diff xếp gọn được theo từng file. Mở lên với `Unimportant` đã ẩn và `Modified` đã -mở, để mở ra là thấy ngay cái đáng xem. Nút `☀ Light` / `☾ Dark` nằm ở góc trên bên -phải — cả hai palette đều nhúng sẵn trong file, nên đổi màu không tải gì và chạy -được trên máy không có internet. +diff xếp gọn được theo từng file. Mở lên với `Unimportant` và `Comment` đã ẩn, +`Modified` đã mở, để mở ra là thấy ngay cái đáng xem. Bấm vào badge nào thì hiện +đúng các dòng comment/noise của nhóm đó — tô màu xám phẳng thay vì đỏ/xanh, để dù +hiện ra rồi vẫn đọc được ngay là "không tính", không lẫn với thay đổi thật. Nút +`☀ Light` / `☾ Dark` nằm ở góc trên bên phải — cả hai palette đều nhúng sẵn trong +file, nên đổi màu không tải gì và chạy được trên máy không có internet. + +Một file mà toàn bộ khác biệt chỉ là comment thì vẫn không có mục chi tiết riêng +(không còn gì ngoài comment để mà xem) — nhưng vẫn giữ marker `≉` và đếm vào +`Comment` trên cây thư mục. ![Report viewer](../../resources/pic/report_page.png) diff --git a/tests/test_report.py b/tests/test_report.py index 0b9efa6..30acd62 100644 --- a/tests/test_report.py +++ b/tests/test_report.py @@ -55,7 +55,9 @@ def test_minor_hunks_get_their_own_row_class(self): self.assertIn('class="addm"', table) self.assertNotIn('class="del"', table) self.assertNotIn('class="add"', table) - self.assertIn('chg-seg', table) # char-level highlight kept + # revealed minor rows are flat grey, not a diff colour, so there is no + # changed SPAN inside them to point at either + self.assertNotIn('chg-seg', table) def test_context_is_three_lines(self): table = _group_table(self.old, self.new, _group_hunks(self.r['hunks'])[0]) @@ -95,7 +97,9 @@ def test_report_shows_minor_hunks_in_modified_files(self): class TestUnimportantToggle(unittest.TestCase): - """Unimportant badge must also hide minor changes inside Modified files.""" + """Comment and Unimportant each hide behind their own badge -- per ROW, not + per group, so a pure-noise group still shows its context and its + placeholder while collapsed (see TestNoisyGroupNeverEmpty).""" MIXED_OLD = "/* gen Mon */\nint lim = 5;\nint keep = 0;\n" MIXED_NEW = "/* gen Tue */\nint lim = 10;\nint keep = 0;\n" @@ -120,24 +124,45 @@ def test_placeholder_row_per_hidden_hunk(self): self.assertIn('commentph', table) self.assertIn('1 comment line hidden', table) - def test_minor_only_group_wrapped_grp_min(self): - r = compare_pair(OLD_ARXML, NEW_ARXML, 'f.arxml') - out = _groups_html(OLD_ARXML.split('\n'), NEW_ARXML.split('\n'), r['hunks']) - self.assertIn('
', out) - - def test_mixed_group_not_wrapped_grp_min(self): - r = compare_pair(self.MIXED_OLD, self.MIXED_NEW, 'f.c') - out = _groups_html(self.MIXED_OLD.split('\n'), self.MIXED_NEW.split('\n'), - r['hunks']) - self.assertIn('
', out) - self.assertNotIn('grp-min', out) - - def test_css_hides_minor_on_toggle(self): + def test_no_group_is_wrapped_for_hiding_any_more(self): + # a whole-group wrapper (grp-min / grp-cmt) used to carry the + # display:none for a pure-noise group, and took its placeholder and + # its own context lines down with it -- hiding is per row now, so no + # group-level class drives visibility at all (grp-rev, for a fully + # reviewed group, is unrelated and still applies) + for old, new, rel in ((OLD_ARXML, NEW_ARXML, 'f.arxml'), + (self.MIXED_OLD, self.MIXED_NEW, 'f.c')): + r = compare_pair(old, new, rel) + out = _groups_html(old.split('\n'), new.split('\n'), r['hunks']) + self.assertIn('
', out) + self.assertNotIn('grp-min', out) + self.assertNotIn('grp-cmt', out) + + def test_css_hides_minor_and_comment_rows_on_toggle(self): results = scan(FIX / 'old', FIX / 'new') page = build_report(results, FIX / 'old', FIX / 'new') - self.assertIn('body.hide-ign tr.minor, body.hide-ign .grp-min { display: none; }', - page) + self.assertIn('body.hide-ign tr.minor { display: none; }', page) + self.assertIn('body.hide-cmt tr.comment { display: none; }', page) self.assertIn('body.hide-ign tr.minorph { display: table-row; }', page) + self.assertIn('body.hide-cmt tr.commentph { display: table-row; }', page) + + +class TestNoisyGroupNeverEmpty(unittest.TestCase): + """Regression: a group whose every hunk is noise must still show its + leading/trailing context and its placeholder while collapsed. + + It used to be wrapped whole in a hideable div, so a file that was ENTIRELY + Unimportant (e.g. uuid_only.arxml) rendered nothing at all under its own + summary line until the reviewer clicked the badge -- not even the count + the placeholder is supposed to state. Hiding moved to the row level to fix + this; this test is what would have caught the bug.""" + + def test_a_pure_noise_group_still_shows_context_and_a_placeholder(self): + r = compare_pair(OLD_ARXML, NEW_ARXML, 'f.arxml') + out = _groups_html(OLD_ARXML.split('\n'), NEW_ARXML.split('\n'), r['hunks']) + self.assertIn('class="ctx"', out) # the lines around the change + self.assertIn('class="gap minorph"', out) + self.assertIn('', out) # in the record, just hidden class TestCharDiff(unittest.TestCase): @@ -180,9 +205,10 @@ def setUpClass(cls): results = scan(FIX / 'old', FIX / 'new') cls.page = build_report(results, FIX / 'old', FIX / 'new') - def test_unimportant_hidden_by_default(self): - self.assertIn('', self.page) + def test_unimportant_and_comment_hidden_by_default(self): + self.assertIn('', self.page) self.assertRegex(self.page, r'badge b-ign off[^>]*>\d+ Unimportant<') + self.assertRegex(self.page, r'badge b-cmt off[^>]*>\d+ Comment<') def test_added_and_deleted_share_one_badge(self): self.assertRegex(self.page, @@ -191,21 +217,26 @@ def test_added_and_deleted_share_one_badge(self): self.assertNotIn('class="badge b-add"', self.page) self.assertNotIn('class="badge b-del"', self.page) - def test_comment_and_identical_are_not_reported_categories(self): - # no badge, no toggle, no detail section -- but the files keep their row - # and verdict mark in the folder tree, so nothing goes unaccounted for - self.assertNotIn('b-cmt', self.page) + def test_comment_only_files_still_have_no_detail_section(self): + # a whole file whose only differences are comments still gets no + # section of its own -- there is nothing beyond the comment lines to + # show it, and it keeps its row and verdict mark in the folder tree + # either way, so nothing goes unaccounted for. Individual comment + # HUNKS mixed into a real-change or Unimportant file's section are a + # separate thing and DO show, behind the Comment badge (see below). self.assertNotIn('badge b-id', self.page) self.assertNotIn('

Identical files

', self.page) self.assertNotIn('
', self.page) def test_modified_files_expanded_by_default(self): self.assertRegex(self.page, r'
]* open>') @@ -259,13 +290,14 @@ def test_no_section_without_arxml_iface_info(self): class TestOneColourLanguage(unittest.TestCase): - """Every category of difference is red on the left and green on the right. + """Real changes and moved blocks are red / green / blue, always visible. - Noise used to get a yellow and a purple of its own. With syntax colours in - the panes that made four hues compete, and a diff whose colours need a - legend is not readable at a glance. Noise is now the same red/green, dimmer - -- dimmer and not identical, because inside a Modified file the reviewer - still has to see which hunks count.""" + Comment and Unimportant used to share that same red/green, one notch + dimmer, so a diff never needed a legend to be read. They are hidden by + default now and, when a badge reveals them, painted a flat NEUTRAL grey + instead -- on purpose: unlike a permanently-visible dim tint, a + toggled-open noise section has to read as "off to the side", not as a + quieter member of the same red/green language real changes own.""" @staticmethod def _rgb(value): @@ -296,41 +328,55 @@ def _themes(self): from compare_tool import theme return theme.THEMES - def test_removed_rows_are_red_on_every_category(self): + def test_removed_rows_are_red(self): for name in self._themes(): - for sel in ('td.del', 'td.delm', 'td.delc'): - r, g, b = self._bg(sel, name) - self.assertGreater(r, g, (name, sel)) - self.assertGreater(r, b, (name, sel)) + r, g, b = self._bg('td.del', name) + self.assertGreater(r, g, name) + self.assertGreater(r, b, name) - def test_added_rows_are_green_on_every_category(self): + def test_added_rows_are_green(self): for name in self._themes(): - for sel in ('td.add', 'td.addm', 'td.addc'): + r, g, b = self._bg('td.add', name) + self.assertGreater(g, r, name) + self.assertGreater(g, b, name) + + def test_revealed_noise_is_neutral_grey_not_red_or_green(self): + # neither channel dominates -- unlike td.del/td.add, this colour makes + # no claim about removed or added. A slight cool cast is fine (that's + # what makes a grey read as UI chrome rather than paper); a channel + # spread anywhere near a real red/green background's (~150+) is not. + for name in self._themes(): + for sel in ('td.delm', 'td.delc', 'td.addm', 'td.addc'): r, g, b = self._bg(sel, name) - self.assertGreater(g, r, (name, sel)) - self.assertGreater(g, b, (name, sel)) + self.assertLessEqual(max(r, g, b) - min(r, g, b), 12, (name, sel)) - def test_noise_is_dimmer_than_a_real_change(self): - # 'dimmer' means nearer the page: darker on the dark theme, paler on - # the light one, so the claim is distance from the background, not - # brightness - from compare_tool import theme + def test_every_noise_selector_shares_the_same_grey(self): + # comment and minor rows share one muted role: two greys would be its + # own small violation of "one colour per meaning" for name in self._themes(): - page = self._rgb(theme.color('bg', name)) + shades = {self._bg(sel, name) + for sel in ('td.delm', 'td.delc', 'td.addm', 'td.addc')} + self.assertEqual(len(shades), 1, name) - def gap(sel): - return sum(abs(a - b) for a, b in zip(self._bg(sel, name), page)) - - self.assertLess(gap('td.delm'), gap('td.del'), name) - self.assertLess(gap('td.addm'), gap('td.add'), name) - - def test_the_legend_no_longer_offers_a_noise_swatch(self): - # a swatch for a colour the reader cannot tell from 'real change' - # explains nothing + def test_revealed_noise_is_visibly_off_the_page(self): + # 'muted' still has to mean something visible, not a wash one shade + # from invisible against the panel it sits on + from compare_tool import theme + for name in self._themes(): + panel = self._rgb(theme.color('panel', name)) + grey = self._bg('td.delm', name) + gap = sum(abs(a - b) for a, b in zip(grey, panel)) + self.assertGreater(gap, 15, name) + + def test_the_legend_still_has_no_noise_colour_swatch_for_it(self): + # the muted grey gets its own swatch (sw-mut) now that noise can be + # revealed; what it must NOT do is reuse or resemble the real-change + # red/green swatches, which is what sw-min / sw-cmt would have implied page = build_report(scan(FIX / 'old', FIX / 'new'), FIX / 'old', FIX / 'new') legend = page.split('class="legend"')[1].split('
')[0] self.assertNotIn('sw-min', legend) self.assertNotIn('sw-cmt', legend) + self.assertIn('sw-mut', legend) class TestOldSideNaming(unittest.TestCase): From a65b345408fce41c90170b13259bc739e4ea2aec Mon Sep 17 00:00:00 2001 From: longvo920 Date: Fri, 31 Jul 2026 20:07:54 +0700 Subject: [PATCH 4/7] fix: group _data.c companion files with their model, not Shared SWC_data.c was its own longer candidate model name and won the match against itself before "SWC" was tried, splitting it into a <3-file group that fell to Shared / other instead of joining SWC's section. --- compare_tool/report.py | 15 ++++++++++++--- tests/test_report.py | 9 +++++++++ 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/compare_tool/report.py b/compare_tool/report.py index 450471d..5992d27 100644 --- a/compare_tool/report.py +++ b/compare_tool/report.py @@ -521,6 +521,12 @@ def _row(o_no, o_txt, n_no, n_txt, mode): _ARXML_SPLIT_RE = re.compile( r'(.+)_(component|datatypes?|interfaces?|implementation|behavior|timing)$', re.IGNORECASE) +# Embedded Coder companion file: _data.c holds the model's constant/ +# calibration tables. Without this, "SWC_data.c" is its own candidate model +# name and -- being longer than "SWC" -- wins the match against itself, +# splitting off into its own (usually <3-file, so Shared) group instead of +# joining SWC's. +_C_ROLE_SPLIT_RE = re.compile(r'(.+)_data$', re.IGNORECASE) # which statuses get a detail section, and in what order. 'comment-only' and # 'identical' are absent: neither is a reported category in this report, so a # section for them would be markup nothing could ever reveal. Both keep their @@ -536,13 +542,16 @@ def _stem(rel): def _detect_models(paths): - """Model-name candidates: X for any X.c, plus X for the modular arxml - export names (X_component.arxml, X_interface.arxml, ...).""" + """Model-name candidates: X for any X.c (X_data.c counts as X, not + X_data), plus X for the modular arxml export names (X_component.arxml, + X_interface.arxml, ...).""" cands = set() for rel in paths: low = rel.lower() if low.endswith('.c'): - cands.add(_stem(rel)) + stem = _stem(rel) + m = _C_ROLE_SPLIT_RE.match(stem) + cands.add(m.group(1) if m else stem) elif low.endswith('.arxml'): m = _ARXML_SPLIT_RE.match(_stem(rel)) if m: diff --git a/tests/test_report.py b/tests/test_report.py index 30acd62..46860df 100644 --- a/tests/test_report.py +++ b/tests/test_report.py @@ -518,6 +518,15 @@ def test_longest_model_name_wins(self): def test_no_models_returns_none(self): self.assertIsNone(_model_groups(self._results(['readme.txt', 'a.h']))) + def test_data_companion_joins_its_model_not_shared(self): + # SWC_data.c used to out-rank "SWC" as its own (longer) candidate + # model, splitting it into a <3-file group that fell to Shared. + paths = ['SWC.c', 'SWC.h', 'SWC_types.h', 'SWC_data.c', + 'SWC_data.h', 'Rte_SWC.h'] + g = _model_groups(self._results(paths)) + self.assertEqual(list(g), ['SWC']) + self.assertEqual(g['SWC'], sorted(paths)) + class TestModelReport(unittest.TestCase): """Full report over the model fixtures: overview table, grouped details, From 4b34f796e020fc0ca649a713fb60c68207caebce Mon Sep 17 00:00:00 2001 From: longvo920 Date: Fri, 31 Jul 2026 20:08:08 +0700 Subject: [PATCH 5/7] fix: report no longer shows comment-type changes, even revealed Comment rows go back to always-hidden with no badge and no toggle -- only the placeholder states how many lines were hidden. The report is a record meant to be sent around; comment churn is left out of it entirely. Unimportant keeps its click-to-reveal grey behaviour. The side-by-side viewer is unchanged and still shows comment rows, muted. --- README.md | 2 +- compare_tool/report.py | 46 +++++++++++++++++++++++------------------- docs/vi/README.md | 21 +++++++++++-------- tests/test_report.py | 23 +++++++++++---------- 4 files changed, 51 insertions(+), 41 deletions(-) diff --git a/README.md b/README.md index d4c5b47..2101d9f 100644 --- a/README.md +++ b/README.md @@ -175,7 +175,7 @@ Files are grouped by **Simulink model** using the Embedded Coder AUTOSAR naming ## HTML report -Self-contained file, one per compare: badge toggles, folder tree, filter box, collapsible diffs per file. Opens `Unimportant` and `Comment` hidden, `Modified` expanded, so it opens on what matters. Clicking either badge reveals the actual comment/noise lines — painted flat grey rather than red/green, so a revealed category still reads as "does not count" instead of looking like another change. A `☀ Light` / `☾ Dark` button sits in the top right — both palettes are embedded in the file, so switching fetches nothing and works on a machine with no internet. +Self-contained file, one per compare: badge toggles, folder tree, filter box, collapsible diffs per file. Opens `Unimportant` hidden, `Modified` expanded, so it opens on what matters. Clicking `Unimportant` reveals the actual noise lines — painted flat grey rather than red/green, so a revealed category still reads as "does not count" instead of looking like another change. Comment changes never render in the report at all — only a placeholder states how many comment lines were hidden — the report is a record meant to be sent around, and comment churn is left out of it entirely; the side-by-side viewer still shows them, greyed, for a reviewer working file by file. A `☀ Light` / `☾ Dark` button sits in the top right — both palettes are embedded in the file, so switching fetches nothing and works on a machine with no internet. A whole file with nothing but comment differences still gets no detail section of its own (there is nothing beyond the comment lines to show); it keeps its own `≉` mark and `Comment` count in the folder tree either way. diff --git a/compare_tool/report.py b/compare_tool/report.py index 5992d27..5a00266 100644 --- a/compare_tool/report.py +++ b/compare_tool/report.py @@ -32,7 +32,6 @@ .badge.off { opacity: .35; text-decoration: line-through; } .b-real { background: var(--tag-real-bg); color: var(--tag-real-fg); } .b-ign { background: var(--tag-ign-bg); color: var(--tag-ign-fg); } -.b-cmt { background: var(--tag-cmt-bg); color: var(--tag-cmt-fg); } .b-id { background: var(--tag-id-bg); color: var(--tag-id-fg); } /* added and deleted share one control: both are "a whole file appeared or vanished", and a reviewer flips them together */ @@ -84,13 +83,15 @@ word-break: break-all; border: none; } td.ln { width: 44px; color: var(--ln-fg); text-align: right; user-select: none; } td.del { background: var(--del-bg); } td.add { background: var(--add-bg); } -/* Comment and Unimportant hide behind their own badge, default OFF -- the - report opens on real changes, a click reveals the rest. Revealed rows are - flat neutral grey, not a dim red/green: a toggled-open noise section still - has to read as "off to the side", off the one-colour-language rule real - changes and moved blocks use, not a quieter member of it. No character-level - highlight either (see _row) -- marking what changed inside a line nobody - was asked to read closely would be noise on noise. */ +/* Unimportant hides behind its own badge, default OFF -- the report opens + on real changes, a click reveals the rest. Revealed rows are flat neutral + grey, not a dim red/green: a toggled-open noise section still has to read + as "off to the side", off the one-colour-language rule real changes and + moved blocks use, not a quieter member of it. No character-level highlight + either (see _row) -- marking what changed inside a line nobody was asked + to read closely would be noise on noise. Comment rows use the same grey + classes but never get a toggle: they stay hidden always (see the CSS + below), only counted in the placeholder, never rendered in the report. */ td.delm, td.delc, td.addm, td.addc { background: var(--muted-bg); color: var(--muted-fg); } td.mvd, td.mva { background: var(--mv-bg); } td.ctx { color: var(--fg-dim); } @@ -106,15 +107,18 @@ font-size: 11px; } tr.mvnote td { text-align: center; color: var(--mv-fg); background: var(--panel-2); font-size: 11px; } -/* Comment and Unimportant rows hide per ROW, not per group: a group used to - be wrapped whole and hidden together, which took the placeholder below and - the ordinary context lines around it down with it -- a noise-only file - opened to an empty box under its own summary line. */ +/* Unimportant rows hide per ROW, not per group: a group used to be wrapped + whole and hidden together, which took the placeholder below and the + ordinary context lines around it down with it -- a noise-only file opened + to an empty box under its own summary line. Comment rows are never shown + in the report -- unlike Unimportant they have no badge and no toggle, only + the placeholder below stating how many lines were hidden, so a comment + change is never silently dropped from the record, just never rendered. */ body.hide-ign tr.minor { display: none; } -body.hide-cmt tr.comment { display: none; } -tr.minorph, tr.commentph { display: none; } +tr.comment { display: none; } +tr.minorph { display: none; } body.hide-ign tr.minorph { display: table-row; } -body.hide-cmt tr.commentph { display: table-row; } +tr.commentph { display: table-row; } tr.minorph td, tr.commentph td { color: var(--st-cmt); } .filenote { color: var(--fg-muted); font-size: 12px; margin: 2px 0 10px; } .renames { font-size: 12px; color: var(--st-ign); margin: 2px 0 8px; } @@ -1191,7 +1195,7 @@ def build_report(results, old_root, new_root, reviews=None, old_label=None, parts = [] parts.append(_head('AUTOSAR Code Generation Report', theme_name, - body_class='hide-ign hide-cmt')) + body_class='hide-ign')) parts.append('

AUTOSAR Code Generation Report

') parts.append('
{} → {} · {}
'.format( _root_html('BASELINE', old_root, old_label), _root_html('CURRENT', new_root), now)) @@ -1220,13 +1224,13 @@ def build_report(results, old_root, new_root, reviews=None, old_label=None, parts.append('
' + err_badge + '{real-change} Modified' '{ignorable-only} Unimportant' - '{comment-only} Comment' '' '{added} Added / {deleted} Deleted' ''.format(**counts) + rev_group + '
') - hint = ('Click a badge to show/hide a category. Unimportant and Comment ' - 'start hidden and, revealed, show in grey rather than red/green ' - '— only real changes keep that colour.') + hint = ('Click a badge to show/hide a category. Unimportant starts hidden ' + 'and, revealed, shows in grey rather than red/green — only ' + 'real changes keep that colour. Comment changes are never shown ' + 'here, only counted.') if rev_group: hint += (' Reviewed starts shown: click it to hide the changes ' 'already signed off.') @@ -1251,7 +1255,7 @@ def build_report(results, old_root, new_root, reviews=None, old_label=None, parts.append('
' '/removed / added ' 'moved block ' - 'Comment / Unimportant, revealed
') + 'Unimportant, revealed
') parts.append('
' '' '' diff --git a/docs/vi/README.md b/docs/vi/README.md index 6e37d55..2cc6418 100644 --- a/docs/vi/README.md +++ b/docs/vi/README.md @@ -189,8 +189,10 @@ một phần của tên, không phải đuôi mangle. được báo là **Comment**, tách khỏi **Unimportant** (UUID, timestamp, SW-VERSION, rename, whitespace) — một banner comment bị viết lại triage khác hẳn một identifier bị đổi tên. Đếm riêng trong summary của CLI và có marker riêng trên cây của viewer. -File trộn comment *với* noise loại khác thì vẫn là Unimportant. Trong HTML report, -`Comment` và `Unimportant` mỗi cái có badge riêng — xem [HTML report](#html-report). +File trộn comment *với* noise loại khác thì vẫn là Unimportant. Trong viewer, +`Comment` và `Unimportant` mỗi cái có rule bật/tắt riêng; trong HTML report, +comment không hiện dòng nào cả, chỉ `Unimportant` có badge để bấm hiện — xem +[HTML report](#html-report). ## Phát hiện block bị di chuyển @@ -240,12 +242,15 @@ model nào rơi vào nhóm cuối **Shared / other**. ## HTML report File self-contained, mỗi lần compare một file: badge bật/tắt, cây thư mục, ô lọc, -diff xếp gọn được theo từng file. Mở lên với `Unimportant` và `Comment` đã ẩn, -`Modified` đã mở, để mở ra là thấy ngay cái đáng xem. Bấm vào badge nào thì hiện -đúng các dòng comment/noise của nhóm đó — tô màu xám phẳng thay vì đỏ/xanh, để dù -hiện ra rồi vẫn đọc được ngay là "không tính", không lẫn với thay đổi thật. Nút -`☀ Light` / `☾ Dark` nằm ở góc trên bên phải — cả hai palette đều nhúng sẵn trong -file, nên đổi màu không tải gì và chạy được trên máy không có internet. +diff xếp gọn được theo từng file. Mở lên với `Unimportant` đã ẩn, `Modified` đã +mở, để mở ra là thấy ngay cái đáng xem. Bấm badge `Unimportant` thì hiện đúng các +dòng noise loại đó — tô màu xám phẳng thay vì đỏ/xanh, để dù hiện ra rồi vẫn đọc +được ngay là "không tính", không lẫn với thay đổi thật. Thay đổi comment thì +**không hiện trong report ở bất kỳ trạng thái nào** — chỉ có placeholder đếm số +dòng bị ẩn; report là bản ghi để gửi đi nên bỏ hẳn comment churn ra khỏi đó, còn +viewer (xem file theo file) vẫn hiện đầy đủ, tô xám. Nút `☀ Light` / `☾ Dark` +nằm ở góc trên bên phải — cả hai palette đều nhúng sẵn trong file, nên đổi màu +không tải gì và chạy được trên máy không có internet. Một file mà toàn bộ khác biệt chỉ là comment thì vẫn không có mục chi tiết riêng (không còn gì ngoài comment để mà xem) — nhưng vẫn giữ marker `≉` và đếm vào diff --git a/tests/test_report.py b/tests/test_report.py index 46860df..9c3fe35 100644 --- a/tests/test_report.py +++ b/tests/test_report.py @@ -138,13 +138,13 @@ def test_no_group_is_wrapped_for_hiding_any_more(self): self.assertNotIn('grp-min', out) self.assertNotIn('grp-cmt', out) - def test_css_hides_minor_and_comment_rows_on_toggle(self): + def test_css_hides_minor_on_toggle_and_comment_unconditionally(self): results = scan(FIX / 'old', FIX / 'new') page = build_report(results, FIX / 'old', FIX / 'new') self.assertIn('body.hide-ign tr.minor { display: none; }', page) - self.assertIn('body.hide-cmt tr.comment { display: none; }', page) + self.assertIn('tr.comment { display: none; }', page) self.assertIn('body.hide-ign tr.minorph { display: table-row; }', page) - self.assertIn('body.hide-cmt tr.commentph { display: table-row; }', page) + self.assertIn('tr.commentph { display: table-row; }', page) class TestNoisyGroupNeverEmpty(unittest.TestCase): @@ -205,10 +205,10 @@ def setUpClass(cls): results = scan(FIX / 'old', FIX / 'new') cls.page = build_report(results, FIX / 'old', FIX / 'new') - def test_unimportant_and_comment_hidden_by_default(self): - self.assertIn('', self.page) + def test_unimportant_hidden_by_default(self): + self.assertIn('', self.page) self.assertRegex(self.page, r'badge b-ign off[^>]*>\d+ Unimportant<') - self.assertRegex(self.page, r'badge b-cmt off[^>]*>\d+ Comment<') + self.assertNotIn('class="badge b-cmt', self.page) def test_added_and_deleted_share_one_badge(self): self.assertRegex(self.page, @@ -230,13 +230,14 @@ def test_comment_only_files_still_have_no_detail_section(self): self.assertIn('
', self.page) + self.assertNotIn('class="badge b-cmt', self.page) def test_modified_files_expanded_by_default(self): self.assertRegex(self.page, r'
]* open>') From e91abc90c52b7262b329c53290cf8a269fb0f0cd Mon Sep 17 00:00:00 2001 From: longvo920 Date: Fri, 31 Jul 2026 20:19:11 +0700 Subject: [PATCH 6/7] fix: only strip a _data suffix when the base model name exists elsewhere Stripping it unconditionally broke a model genuinely called Foo_data: the group was labelled after a Foo that does not exist and Rte_Foo_data.h matched nothing, falling to Shared. The suffix is now dropped only when another file evidences the base name, which is the case the fix was for. Also tighten two report assertions that passed either way: the bare "tr.comment { display: none; }" they looked for is a substring of the body.hide-cmt-qualified rule it replaced, so neither proved the rule is unconditional. They now assert the qualifier is absent from the page. --- compare_tool/report.py | 22 ++++++++++++++-------- tests/test_report.py | 30 +++++++++++++++++++++++++++--- 2 files changed, 41 insertions(+), 11 deletions(-) diff --git a/compare_tool/report.py b/compare_tool/report.py index 5a00266..cf97f5a 100644 --- a/compare_tool/report.py +++ b/compare_tool/report.py @@ -529,8 +529,11 @@ def _row(o_no, o_txt, n_no, n_txt, mode): # calibration tables. Without this, "SWC_data.c" is its own candidate model # name and -- being longer than "SWC" -- wins the match against itself, # splitting off into its own (usually <3-file, so Shared) group instead of -# joining SWC's. -_C_ROLE_SPLIT_RE = re.compile(r'(.+)_data$', re.IGNORECASE) +# joining SWC's. Dropped only when the base name is evidenced by ANOTHER +# file: a model genuinely called Foo_data, with no Foo.c beside it, keeps +# its own name -- otherwise Rte_Foo_data.h would match nothing and the +# group would be labelled with a model that does not exist. +_C_DATA_RE = re.compile(r'(.+)_data$', re.IGNORECASE) # which statuses get a detail section, and in what order. 'comment-only' and # 'identical' are absent: neither is a reported category in this report, so a # section for them would be markup nothing could ever reveal. Both keep their @@ -546,20 +549,23 @@ def _stem(rel): def _detect_models(paths): - """Model-name candidates: X for any X.c (X_data.c counts as X, not - X_data), plus X for the modular arxml export names (X_component.arxml, - X_interface.arxml, ...).""" + """Model-name candidates: X for any X.c, plus X for the modular arxml + export names (X_component.arxml, X_interface.arxml, ...). X_data is then + dropped whenever X itself is a candidate, so SWC_data.c joins SWC instead + of out-ranking it -- see _C_DATA_RE.""" cands = set() for rel in paths: low = rel.lower() if low.endswith('.c'): - stem = _stem(rel) - m = _C_ROLE_SPLIT_RE.match(stem) - cands.add(m.group(1) if m else stem) + cands.add(_stem(rel)) elif low.endswith('.arxml'): m = _ARXML_SPLIT_RE.match(_stem(rel)) if m: cands.add(m.group(1)) + for c in list(cands): + m = _C_DATA_RE.match(c) + if m and m.group(1) in cands: + cands.discard(c) return cands diff --git a/tests/test_report.py b/tests/test_report.py index 9c3fe35..fdaace4 100644 --- a/tests/test_report.py +++ b/tests/test_report.py @@ -142,9 +142,14 @@ def test_css_hides_minor_on_toggle_and_comment_unconditionally(self): results = scan(FIX / 'old', FIX / 'new') page = build_report(results, FIX / 'old', FIX / 'new') self.assertIn('body.hide-ign tr.minor { display: none; }', page) - self.assertIn('tr.comment { display: none; }', page) self.assertIn('body.hide-ign tr.minorph { display: table-row; }', page) - self.assertIn('tr.commentph { display: table-row; }', page) + self.assertIn('\ntr.comment { display: none; }', page) + self.assertIn('\ntr.commentph { display: table-row; }', page) + # decisive: the comment rules carry NO body.hide-cmt qualifier, so + # there is no state in which they turn back on. Asserting the bare + # rule alone would not prove it -- it is a substring of the + # qualified one. + self.assertNotIn('hide-cmt', page) class TestNoisyGroupNeverEmpty(unittest.TestCase): @@ -234,10 +239,11 @@ def test_comment_rows_stay_in_the_record_but_never_render(self): # the report is the record: the lines are always in the HTML, in a # tr.comment row -- CSS just always hides that row, unconditionally, # with no badge to reveal it. Only the placeholder count is visible. - self.assertIn('tr.comment { display: none; }', self.page) + self.assertIn('\ntr.comment { display: none; }', self.page) self.assertRegex(self.page, r'class="gap commentph"') self.assertIn('', self.page) self.assertNotIn('class="badge b-cmt', self.page) + self.assertNotIn('hide-cmt', self.page) def test_modified_files_expanded_by_default(self): self.assertRegex(self.page, r'
]* open>') @@ -528,6 +534,24 @@ def test_data_companion_joins_its_model_not_shared(self): self.assertEqual(list(g), ['SWC']) self.assertEqual(g['SWC'], sorted(paths)) + def test_data_companion_named_from_an_arxml_model_too(self): + paths = ['SWC_component.arxml', 'SWC_interface.arxml', + 'SWC_data.c', 'SWC_data.h'] + g = _model_groups(self._results(paths)) + self.assertEqual(list(g), ['SWC']) + self.assertEqual(g['SWC'], sorted(paths)) + + def test_a_model_genuinely_named_x_data_keeps_its_own_name(self): + # the _data suffix is only stripped when the base name is evidenced + # elsewhere. With no Foo.c anywhere, Foo_data IS the model: stripping + # it would label the group after a model that does not exist and + # leave Rte_Foo_data.h matching nothing. + paths = ['Foo_data.c', 'Foo_data.h', 'Foo_data_types.h', + 'Rte_Foo_data.h'] + g = _model_groups(self._results(paths)) + self.assertEqual(list(g), ['Foo_data']) + self.assertEqual(g['Foo_data'], sorted(paths)) + class TestModelReport(unittest.TestCase): """Full report over the model fixtures: overview table, grouped details, From 268005bef74dde5372b5796b54bd735a05e9b741 Mon Sep 17 00:00:00 2001 From: longvo920 Date: Fri, 31 Jul 2026 20:19:18 +0700 Subject: [PATCH 7/7] chore: release 1.3.0 Minor bump: themes, A2L colouring and the --theme flag are additive, and the exit-code contract is unchanged. --- CHANGELOG.md | 15 +++++++++++++++ README.md | 4 ++-- compare_tool/__init__.py | 2 +- docs/vi/README.md | 4 ++-- 4 files changed, 20 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c4a0ee0..e1676fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,11 @@ All notable changes to this project are documented here. Versions follow ## [Unreleased] +## [1.3.0] — 2026-07-31 + +Read the diff in whichever colour scheme suits the screen, with A2L coloured +like the rest and noise pushed out of the way instead of out of the file. + ### Added - **Light and dark colour schemes.** Both the viewer and the report can be read @@ -22,6 +27,16 @@ All notable changes to this project are documented here. Versions follow of removing them.** They keep their place and their line numbers, so the code around a real change is still there to read it in, and they no longer count as changes on the minimap or when stepping through changes. +- **The report shows unimportant differences when you click their badge**, in + grey rather than red or green, so a revealed category still reads as one that + does not count. Comment differences stay out of the report entirely — only + the count of hidden lines is shown — while the viewer keeps showing them. + +### Fixed + +- **A model's `_data` companion file is filed under that model** in the report + instead of landing in Shared / other, so everything generated for one + component is read in one place. ## [1.2.0] — 2026-07-29 diff --git a/README.md b/README.md index 2101d9f..476621f 100644 --- a/README.md +++ b/README.md @@ -141,7 +141,7 @@ A shorter name can stop an argument wrapping at 80 columns, so the two sides hol Everything else keeps its suffix as meaning. `SIG_TORQUE_MIN` → `SIG_TORQUE_MAX` and `CFG_TIMEOUT_MS` → `CFG_TIMEOUT_US` are real changes, and so are `rtb_AND_…` → `rtb_OR_…` (a different block drives that buffer) and `Sub_…_step` → `Sub_…_Init` (a different entry point). Digits glued to a block name (`rtb_Switch1` vs `rtb_Switch2`) are part of the name, not a mangle tail. -**Comment changes are their own category.** A file whose differences are *only* comments is reported as **Comment**, separate from **Unimportant** (UUIDs, timestamps, SW-VERSION, renames, whitespace) — a rewritten comment banner triages differently from a renamed identifier. Separate counts in the CLI summary and its own tree marker in the viewer. A file mixing comments *with* other noise stays Unimportant. In the HTML report, `Comment` and `Unimportant` are each a badge of their own — see [HTML report](#html-report). +**Comment changes are their own category.** A file whose differences are *only* comments is reported as **Comment**, separate from **Unimportant** (UUIDs, timestamps, SW-VERSION, renames, whitespace) — a rewritten comment banner triages differently from a renamed identifier. Separate counts in the CLI summary and its own tree marker in the viewer. A file mixing comments *with* other noise stays Unimportant. The viewer has a rule toggle for each; the HTML report gives `Unimportant` a badge and leaves comment lines out altogether — see [HTML report](#html-report). ## Moved block detection @@ -171,7 +171,7 @@ A file whose XML fails to parse is skipped from this summary (its text diff stil ## Grouping by model / SWC -Files are grouped by **Simulink model** using the Embedded Coder AUTOSAR naming convention (`X.c`, `X.h`, `X.arxml`, `Rte_X.h`, the modular ARXML set, …). Files that match no model land in a final **Shared / other** group. +Files are grouped by **Simulink model** using the Embedded Coder AUTOSAR naming convention (`X.c`, `X.h`, `X.arxml`, `Rte_X.h`, `X_data.c`, the modular ARXML set, …). Files that match no model land in a final **Shared / other** group. ## HTML report diff --git a/compare_tool/__init__.py b/compare_tool/__init__.py index 55cd6f8..48f4a76 100644 --- a/compare_tool/__init__.py +++ b/compare_tool/__init__.py @@ -1,3 +1,3 @@ """CodeGen Compare Tool - AUTOSAR MATLAB codegen diff with noise filtering.""" -__version__ = "1.2.0" +__version__ = "1.3.0" diff --git a/docs/vi/README.md b/docs/vi/README.md index 2cc6418..39a87fa 100644 --- a/docs/vi/README.md +++ b/docs/vi/README.md @@ -236,8 +236,8 @@ File có XML parse lỗi bị bỏ khỏi phần summary này (text diff của n ## Nhóm theo model / SWC File được nhóm theo **model Simulink** dựa trên quy ước đặt tên AUTOSAR của Embedded -Coder (`X.c`, `X.h`, `X.arxml`, `Rte_X.h`, bộ ARXML modular, …). File không khớp -model nào rơi vào nhóm cuối **Shared / other**. +Coder (`X.c`, `X.h`, `X.arxml`, `Rte_X.h`, `X_data.c`, bộ ARXML modular, …). File +không khớp model nào rơi vào nhóm cuối **Shared / other**. ## HTML report