added key "?" for help

Default.sublime-keymap

@@ -23,6 +23,8 @@
       "context": [{"key": "setting.vdiff_view"}, {"key": "setting.command_mode", "operand": false}] },
     { "keys": ["p"], "command": "vdiff_action", "args": {"action": "prev_file"},
       "context": [{"key": "setting.vdiff_view"}, {"key": "setting.command_mode", "operand": false}] },
+    { "keys": ["?"], "command": "vdiff_action", "args": {"action": "help"},
+      "context": [{"key": "setting.vdiff_view"}, {"key": "setting.command_mode", "operand": false}] },
     { "keys": ["escape"], "command": "vdiff_action", "args": {"action": "close"},
       "context": [
         {"key": "setting.vdiff_view"},

README.md

@@ -23,6 +23,7 @@ Keys inside the review view:
 | `]` / `[` | next / previous change |
 | `n` / `p` | next / previous file of the current list |
 | `r` | refresh |
+| `?` | show this key list in a popup |
 | Esc | close the view |
 
 The header links do the same with the mouse. Settings: `vDiff.sublime-settings`.

tests/stubs/sublime.py

@@ -113,6 +113,9 @@ class View:
         self.name_ = ""
         self.sel_ = []
         self.status = {}
+        self.commands = []
+        self.popup = None
+        self.popup_navigate = None
 
     def id(self):
         return 1
@@ -129,6 +132,8 @@ class View:
     def run_command(self, name, args=None):
         if name == "vdiff_replace":
             self.replace(None, Region(0, self.size()), args["text"])
+        else:
+            self.commands.append((name, args))
 
     def set_read_only(self, flag):
         self.read_only = flag
@@ -185,6 +190,18 @@ class View:
     def show_at_center(self, point):
         pass
 
+    def show_popup(self, content, flags=0, location=-1, max_width=320, max_height=240, on_navigate=None, on_hide=None):
+        self.popup, self.popup_navigate = content, on_navigate
+
+    def hide_popup(self):
+        self.popup = None
+
+    def is_popup_visible(self):
+        return self.popup is not None
+
+    def em_width(self):
+        return 8.0
+
     def style_for_scope(self, scope):
         return {"foreground": "#ffffff", "background": "#333333"}
 

tests/test_commands.py

@@ -124,3 +124,14 @@ def test_run_async_reports_git_error(monkeypatch):
 
     commands.run_async(boom, lambda r: messages.append("done"))
     assert messages == ["vDiff: fatal: bad revision 'nope'"]
+
+
+def test_run_async_reports_generic_error(monkeypatch):
+    messages = []
+    monkeypatch.setattr(commands.sublime, "status_message", lambda m: messages.append(m))
+
+    def boom():
+        raise ValueError("boom")
+
+    commands.run_async(boom, lambda r: messages.append("done"))
+    assert messages == ["vDiff: boom"]

tests/test_render.py

@@ -1,3 +1,7 @@
+import json
+import os
+import re
+
 import sublime  # the stub, via conftest
 
 from vdiff import commands, git, listeners, parse, render, state  # noqa: F401
@@ -110,3 +114,93 @@ def test_binary_and_caret_index():
     view.sel().clear()
     view.sel().add(sublime.Region(0))
     assert render.caret_index(view) == -1
+
+
+# --- key help popup ---------------------------------------------------------
+
+
+def help_popup(st):
+    """Open the popup through the action registry and return (view, html)"""
+    view = sublime.View()
+    render.init_view(view)
+    commands.ACTIONS["help"](view, None, st)
+    return view, view.popup
+
+
+def test_help_popup_lists_every_key():
+    st = make_state()
+    view, html = help_popup(st)
+
+    assert view.is_popup_visible()
+    for group in ("Navigate", "Panels", "This file", "View"):
+        assert ">{}<".format(group) in html
+    for keys in ("] [", "n p", "o ⏎", "f", "c", "s", "u", "d", "r", "?", "esc"):
+        assert ">{}<".format(keys) in html
+    assert "next / previous change" in html and "open the real file at this line" in html
+    # working-tree mode: nothing dimmed, every single-action row is a link
+    assert 'class="dim"' not in html and "uncommitted only" not in html
+    assert 'href="stage"' in html and 'href="files"' in html and 'href="close"' in html
+    # rows covering two actions stay plain text
+    assert 'href="next_change"' not in html
+    # the status bar advertises it too
+    assert "? keys" in view.status[render.STATUS_KEY]
+
+
+def test_help_popup_dims_file_actions_in_commit_mode():
+    st = make_state()
+    st.enter("abc", git.CommitInfo("abc", "abc1234", "Ann", "2026-09-23 10:00", "Fix things", ""))
+    _, html = help_popup(st)
+
+    assert "uncommitted only" in html
+    assert html.count('class="dim"') == 3
+    assert 'href="stage"' not in html and 'href="unstage"' not in html and 'href="discard"' not in html
+    # everything that still applies stays clickable
+    assert 'href="files"' in html and 'href="open"' in html and 'href="refresh"' in html
+
+
+def test_help_popup_toggles_and_navigates():
+    st = make_state()
+    view, _ = help_popup(st)
+
+    commands.ACTIONS["help"](view, None, st)
+    assert not view.is_popup_visible()
+
+    commands.ACTIONS["help"](view, None, st)
+    assert view.is_popup_visible()
+    view.popup_navigate("files")
+    assert not view.is_popup_visible()
+    assert view.commands[-1] == ("vdiff_action", {"action": "files"})
+
+
+# keymap spellings that the popup shows as a symbol
+DISPLAY_KEYS = {"enter": "⏎", "escape": "esc"}
+
+
+def test_keymap_matches_help_table():
+    """The keymap, ACTIONS and the popup agree on both the actions and the keys that reach them"""
+    path = os.path.join(os.path.dirname(os.path.dirname(os.path.abspath(__file__))), "Default.sublime-keymap")
+    with open(path, encoding="utf-8") as f:
+        bindings = json.load(f)
+
+    assert all(b["command"] == "vdiff_action" for b in bindings)
+    bound = {b["args"]["action"] for b in bindings}
+    listed = {action for k in render.KEYS for action in k.actions}
+    assert bound == set(commands.ACTIONS) == listed
+
+    # rebinding a key without updating the table would leave the popup and status bar lying
+    bound_keys = {DISPLAY_KEYS.get(key, key) for b in bindings for key in b["keys"]}
+    assert bound_keys == {token for k in render.KEYS for token in k.keys.split()}
+
+
+def test_minihtml_stylesheets_reject_unsupported_selectors():
+    """minihtml drops the rest of a stylesheet at a selector it cannot parse; these are the forms
+    that silently broke HELP_CSS, so no stylesheet constant may use them"""
+    # descendant/child/sibling combinators, pseudo-classes, attribute selectors, compound classes
+    unsupported = re.compile(r"[\s:>~+\[]|[.#][^.#]*[.#]")
+    sheets = [(name, css) for name, css in vars(render).items() if name.endswith("_CSS") and isinstance(css, str)]
+    assert len(sheets) >= 2, sheets
+    for name, css in sheets:
+        for rule in css.split("}"):
+            head = rule.split("{")[0].strip()
+            for selector in filter(None, (s.strip() for s in head.split(","))):
+                assert not unsupported.search(selector), "{}: {}".format(name, selector)

vdiff/commands.py

@@ -107,12 +107,10 @@ def file_items(files):
         description = git.STATUS_NAMES.get(change.status, "")
         colour = STATUS_COLOURS.get(change.status)
         kind = (colour, change.status, description) if colour else sublime.KIND_AMBIGUOUS
-        details = description
+        details = ", ".join(filter(None, (description, "staged" if change.staged else "")))
-        if change.staged:
-            details += ", staged"
         if change.old_path:
             details += "  ← " + change.old_path
-        if change.insertions is None or change.deletions is None:
+        if None in (change.insertions, change.deletions):
             annotation = "binary"
         else:
             annotation = "-{} +{}".format(change.deletions, change.insertions)
@@ -283,6 +281,10 @@ def _act_close(view, window, st):
     view.close()
 
 
+def _act_help(view, window, st):
+    render.show_help(view, st)
+
+
 def _git_action(verb, done_verb, operation, confirm=None):
     """Action running `operation(repo, change)` on the shown working-tree file, then re-listing"""
 
@@ -362,6 +364,7 @@ ACTIONS = {
     "files": _act_files,
     "commits": _act_commits,
     "close": _act_close,
+    "help": _act_help,
     "stage": _git_action("stage", "staged", git.stage),
     "unstage": _git_action("unstage", "unstaged", git.unstage),
     "discard": _git_action("discard", "discarded", git.discard, confirm=_confirm_discard),

vdiff/git.py

@@ -45,10 +45,12 @@ class GitError(Exception):
 def run(repo, args, ok_codes=(0,)):
     """Run git in `repo`, return stdout as text"""
     cmd = [GIT_BINARY, "-c", "core.quotepath=off", "--no-pager"] + list(args)
-    proc = subprocess.run(cmd, cwd=repo, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+    proc = subprocess.run(
+        cmd, cwd=repo, stdout=subprocess.PIPE, stderr=subprocess.PIPE, encoding="utf-8", errors="replace"
+    )
     if proc.returncode not in ok_codes:
-        raise GitError(proc.stderr.decode("utf-8", "replace"), proc.returncode)
+        raise GitError(proc.stderr, proc.returncode)
-    return proc.stdout.decode("utf-8", "replace")
+    return proc.stdout
 
 
 def repo_root(path):
@@ -79,6 +81,11 @@ def base_ref(repo):
     return "HEAD" if has_head(repo) else EMPTY_TREE
 
 
+def _int_or_none(value):
+    """A numstat count: "-" marks a binary file"""
+    return None if value == "-" else int(value)
+
+
 def _parse_numstat(text):
     """`--numstat -z` output -> {path: (insertions, deletions, old_path)}"""
     result = {}
@@ -91,9 +98,9 @@ def _parse_numstat(text):
         old_path = None
         if path == "":
             # rename: counts, then old and new path as separate NUL fields
-            old_path = next(fields, None)
+            old_path = next(fields, "") or None
             path = next(fields, "")
-        result[path] = (None if ins == "-" else int(ins), None if dele == "-" else int(dele), old_path)
+        result[path] = (_int_or_none(ins), _int_or_none(dele), old_path)
     return result
 
 
@@ -146,7 +153,8 @@ def changed_files(repo):
     for entry in entries:
         kind = entry[0]
         if kind == "1":
-            xy, path = entry.split(" ", 8)[1], entry.split(" ", 8)[-1]
+            fields = entry.split(" ", 8)
+            xy, path = fields[1], fields[-1]
         elif kind == "u":
             xy, path = "UU", entry.split(" ", 10)[-1]
         elif kind == "?":

vdiff/listeners.py

@@ -17,7 +17,7 @@ class VdiffEventListener(sublime_plugin.EventListener):
 
     def on_post_save(self, view):
         """Review views showing the saved file re-render; the fingerprint decides which ones"""
-        for view_id, st in list(state.STATES.items()):
+        for view_id, st in state.tracked():
             review = sublime.View(view_id)
             if review.is_valid():
                 refresh_if_changed(review, st)

vdiff/parse.py

@@ -1,7 +1,8 @@
 """ Unified diff -> lines with old/new numbers, change blocks, word-level ranges; no sublime imports """
 import difflib
 import re
-from collections import namedtuple
+from collections import Counter, namedtuple
+from itertools import groupby
 
 # kind is " " (context), "-" (deleted) or "+" (inserted); the missing number is None
 Line = namedtuple("Line", "kind old_no new_no text")
@@ -51,31 +52,22 @@ def parse_unified(text):
     return Diff(lines, binary)
 
 
+def runs(items, key):
+    """(key value, start, end) for each maximal run of consecutive items with the same key"""
+    for value, group in groupby(enumerate(items), key=lambda pair: key(pair[1])):
+        indexes = [i for i, _ in group]
+        yield value, indexes[0], indexes[-1] + 1
+
+
 def change_blocks(lines):
     """[(start, end)) index ranges of consecutive changed lines"""
-    blocks = []
+    return [(start, end) for changed, start, end in runs(lines, lambda line: line.kind != " ") if changed]
-    start = None
-    for i, line in enumerate(lines):
-        if line.kind != " ":
-            if start is None:
-                start = i
-        elif start is not None:
-            blocks.append((start, i))
-            start = None
-    if start is not None:
-        blocks.append((start, len(lines)))
-    return blocks
 
 
 def counts(lines):
     """(insertions, deletions)"""
-    ins = dele = 0
+    tally = Counter(line.kind for line in lines)
-    for line in lines:
+    return tally["+"], tally["-"]
-        if line.kind == "+":
-            ins += 1
-        elif line.kind == "-":
-            dele += 1
-    return ins, dele
 
 
 def paired_lines(lines, block):

vdiff/render.py

@@ -3,6 +3,7 @@ import html
 import json
 import logging
 import os
+from collections import namedtuple
 
 import sublime
 import sublime_plugin
@@ -46,7 +47,39 @@ FALLBACK_RULES = [
     {"scope": SCOPE_INS_CHAR, "background": "hsla(114, 45%, 68%, 0.30)", "foreground_adjust": "l(+ 10%)"},
 ]
 
-KEY_HINTS = "f files   c commits   s stage   u unstage   d discard   o open   ] [ change   n p file   r refresh"
+# one source for the status-bar hints and the ? popup.
+# actions: the ACTIONS names the row covers; a single one makes the row a link. short: status-bar
+# wording, "" keeps the row out of it. The group flag is False for groups needing a working-tree file.
+Key = namedtuple("Key", "keys label short actions")
+
+KEY_GROUPS = [
+    ("Navigate", True, [
+        Key("] [", "next / previous change", "change", ("next_change", "prev_change")),
+        Key("n p", "next / previous file", "file", ("next_file", "prev_file")),
+        Key("o \u23ce", "open the real file at this line", "open", ("open",)),
+    ]),
+    ("Panels", True, [
+        Key("f", "changed files", "files", ("files",)),
+        Key("c", "recent commits", "commits", ("commits",)),
+    ]),
+    ("This file", False, [
+        Key("s", "stage", "stage", ("stage",)),
+        Key("u", "unstage", "unstage", ("unstage",)),
+        Key("d", "discard", "discard", ("discard",)),
+    ]),
+    ("View", True, [
+        Key("r", "refresh", "refresh", ("refresh",)),
+        Key("?", "this list", "keys", ("help",)),
+        Key("esc", "close the view", "", ("close",)),
+    ]),
+]
+
+KEYS = [key for _, _, group in KEY_GROUPS for key in group]
+
+KEY_HINTS = "   ".join("{} {}".format(k.keys, k.short) for k in KEYS if k.short)
+
+# the popup font is the editor's, so the key column is padded to a fixed width in characters
+_KEY_WIDTH = max(len(k.keys) for k in KEYS)
 
 HEADER_CSS = """
 body { margin: 0; padding: 0.5rem 0.8rem; background-color: color(var(--background) blend(var(--foreground) 93%)); }
@@ -64,6 +97,21 @@ a { text-decoration: none; padding: 0.15rem 0.5rem; margin-left: 0.3rem; border-
 .body { color: color(var(--foreground) alpha(0.6)); }
 """
 
+# minihtml stops applying a stylesheet at the first selector it cannot parse, and its documentation
+# covers properties only, not selectors: every rule here sticks to one bare element or one class,
+# the forms HEADER_CSS proves work. The first group's margin stands in for body's top padding, and
+# .kdim takes its colour from the .dim row around it.
+HELP_CSS = """
+body { margin: 0; padding: 0 0.8rem 0.6rem 0.8rem; }
+.group { margin-top: 0.6rem; color: var(--bluish); font-weight: bold; }
+.note { color: color(var(--foreground) alpha(0.5)); font-weight: normal; }
+.row { color: color(var(--foreground) alpha(0.85)); }
+.dim { color: color(var(--foreground) alpha(0.35)); }
+.k { color: var(--orangish); font-weight: bold; }
+.kdim { font-weight: bold; }
+a { text-decoration: none; color: color(var(--foreground) alpha(0.85)); }
+"""
+
 
 class VdiffReplaceCommand(sublime_plugin.TextCommand):
     """Replace the whole buffer; the only writer of a review view"""
@@ -212,16 +260,10 @@ def _row_starts(lines):
 def _line_regions(lines, starts):
     """Full-line regions of deleted and inserted rows, consecutive rows merged"""
     regions = {"-": [], "+": []}
-    run_kind = None
+    for kind, start, end in parse.runs(lines, lambda line: line.kind):
-    run_start = 0
+        if kind in regions:
-    for i in range(len(lines) + 1):
+            # starts has a sentinel at len(lines): the last line stops at the buffer end
-        kind = lines[i].kind if i < len(lines) else None
+            regions[kind].append(sublime.Region(starts[start], starts[end]))
-        if kind == run_kind:
-            continue
-        if run_kind in regions:
-            # min(): the last line's region stops at the buffer end instead of one past it
-            regions[run_kind].append(sublime.Region(starts[run_start], min(starts[i], starts[-1])))
-        run_kind, run_start = kind, i
     return regions["-"], regions["+"]
 
 
@@ -261,6 +303,7 @@ def header_html(st, ins, dele):
         actions.append('<a href="open">Open</a>')
     actions.append('<a href="files">Files</a>')
     actions.append('<a href="commits">Commits</a>')
+    actions.append('<a href="help" title="keys">?</a>')
 
     commit = ""
     if st.in_commit and st.commit:
@@ -278,16 +321,60 @@ def header_html(st, ins, dele):
     )
 
 
+def _run_action(view, href):
+    """Dispatch a header or popup link through the same command the keymap uses"""
+    view.run_command("vdiff_action", {"action": href})
+
+
 def _header(view, st, ins, dele):
     """One block phantom above the text; redrawn from scratch, so no PhantomSet state to keep"""
 
     def navigate(href):
-        view.run_command("vdiff_action", {"action": href})
+        _run_action(view, href)
 
     view.erase_phantoms(PHANTOM_KEY)
     view.add_phantom(PHANTOM_KEY, sublime.Region(0), header_html(st, ins, dele), sublime.LAYOUT_BLOCK, navigate)
 
 
+# --- key help popup ---------------------------------------------------------
+
+
+def help_html(st):
+    """The minihtml key list; rows that need a working-tree file are dimmed while a commit is shown"""
+    parts = []
+    for title, applies, group in KEY_GROUPS:
+        dim = st.in_commit and not applies
+        heading = html.escape(title)
+        if dim:
+            heading += ' <span class="note">uncommitted only</span>'
+        parts.append('<div class="group">{}</div>'.format(heading))
+        # one class per element: a compound selector would silently drop the rest of HELP_CSS
+        row_class, key_class = ("dim", "kdim") if dim else ("row", "k")
+        for key in group:
+            row = '<span class="{}">{}</span>{}{}'.format(
+                key_class, html.escape(key.keys), "&nbsp;" * (_KEY_WIDTH - len(key.keys) + 2), html.escape(key.label)
+            )
+            if len(key.actions) == 1 and not dim:
+                row = '<a href="{}">{}</a>'.format(key.actions[0], row)
+            parts.append('<div class="{}">{}</div>'.format(row_class, row))
+    return '<body id="vdiff-help"><style>{}</style>{}</body>'.format(HELP_CSS, "".join(parts))
+
+
+def show_help(view, st):
+    """Toggle the key list popup at the caret"""
+    if view.is_popup_visible():
+        view.hide_popup()
+        return
+
+    def navigate(href):
+        view.hide_popup()
+        _run_action(view, href)
+
+    # minihtml sizes follow the editor font, so fixed pixel bounds would clip at a large font size
+    em = view.em_width() or 8
+    view.show_popup(help_html(st), location=-1, max_width=int(em * 56), max_height=int(em * 44), on_navigate=navigate)
+
+
 # --- colour scheme fallback -------------------------------------------------
 
 _scheme_checked = set()

vdiff/state.py

@@ -1,5 +1,6 @@
 """ Per-view state of review views, keyed by view id; the essentials are mirrored into view settings for restore """
 from collections import namedtuple
+from dataclasses import dataclass, field
 
 # view setting marking a review view (keymap context "setting.vdiff_view")
 VIEW_FLAG = "vdiff_view"
@@ -18,25 +19,25 @@ def find_change(files, path):
     return -1
 
 
+@dataclass(eq=False)
 class ViewState:
     """Everything a review view needs to re-render itself and act on its file
 
     `sha` is None in working-tree mode and the reviewed commit's hash otherwise.
     """
 
-    def __init__(self, repo):
+    repo: str
-        self.repo = repo
+    sha: object = None  # None in working-tree mode, the reviewed commit's hash otherwise
-        self.sha = None
+    commit: object = None  # git.CommitInfo when sha is set
-        self.commit = None  # git.CommitInfo when sha is set
+    change: object = None  # git.FileChange currently shown
-        self.change = None  # git.FileChange currently shown
+    files: list = field(default_factory=list)  # git.FileChange list the user is browsing (next/prev file)
-        self.files = []  # git.FileChange list the user is browsing (next/prev file)
+    lines: list = field(default_factory=list)  # parse.Line list currently rendered
-        self.lines = []  # parse.Line list currently rendered
+    blocks: list = field(default_factory=list)  # parse.change_blocks(lines)
-        self.blocks = []  # parse.change_blocks(lines)
+    binary: bool = False
-        self.binary = False
+    token: int = 0  # bumped per show request; a stale async result is dropped
-        self.token = 0  # bumped per show request; a stale async result is dropped
+    rendered_token: int = -1  # token of the request that produced the current render
-        self.rendered_token = -1  # token of the request that produced the current render
+    highlight: int = 0  # bumped per quick-panel highlight, to debounce previews
-        self.highlight = 0  # bumped per quick-panel highlight, to debounce previews
+    fingerprint: object = None  # (file mtime, index mtime) at render time, working-tree mode
-        self.fingerprint = None  # (file mtime, index mtime) at render time, working-tree mode
 
     @property
     def in_commit(self):
@@ -77,6 +78,11 @@ def get(view):
     return STATES.get(view.id())
 
 
+def tracked():
+    """Snapshot of (view id, state) for every tracked review view"""
+    return list(STATES.items())
+
+
 def put(view, st):
     STATES[view.id()] = st
     return st

vdiff/utils.py

@@ -38,10 +38,10 @@ def run_async(work, done):
     def job():
         try:
             result = work()
-        except (git.GitError, OSError) as error:
+        except Exception as error:
-            LOG.warning("git failed: %s", error)
+            # the traceback helps when debugging; the status message is what the user sees
-            # `error` is unbound once the except block ends; the callback runs later
             message = str(error) or error.__class__.__name__
+            LOG.warning("vdiff worker failed: %s", message, exc_info=LOG.isEnabledFor(logging.DEBUG))
             sublime.set_timeout(lambda: status(message))
             return
         sublime.set_timeout(lambda: done(result))