From 18551641b5537f6dbab000405a00f506f8d75695 Mon Sep 17 00:00:00 2001 From: Vaclav Tvrdik Date: Thu, 24 Sep 2026 17:13:29 +0200 Subject: [PATCH] "n" and "p" keys cycle over files --- README.md | 2 +- tests/test_commands.py | 74 ++++++++++++++++++++++++++++++++++++++++++ tests/test_render.py | 33 +++++++++++++++++++ vdiff/commands.py | 19 ++++++++--- vdiff/render.py | 26 ++++++++++++++- 5 files changed, 147 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 2cae327..9936d16 100644 --- a/README.md +++ b/README.md @@ -22,7 +22,7 @@ Keys inside the review view: | `C` or `m` | commit the staged files, typing the message in an input panel | | `o` or Enter | open the real file at this line | | `]` / `[` | next / previous change | -| `n` / `p` | next / previous file of the current list | +| `n` / `p` | next / previous file of the current list (wraps at the ends) | | `j` / `k` | move down / up one line (vim keys) | | `r` | refresh | | `?` | show this key list in a popup | diff --git a/tests/test_commands.py b/tests/test_commands.py index 8972d8f..36b1b65 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -329,3 +329,77 @@ def test_esc_rewinds_the_file_navigation_counts_from(loading): st.restore(kept) commands.ACTIONS["next_file"](view, window, st) assert requested_path(queue) == "f4.py" + + +# --- cycling past the ends -------------------------------------------------- + + +@pytest.fixture +def rings(monkeypatch): + """Record visual bells instead of drawing them""" + bells = [] + monkeypatch.setattr(commands.render, "flash", lambda view: bells.append(view)) + return bells + + +def at_file(files, path): + """A loaded working-tree view sitting on `path`""" + view, window = sublime.View(), sublime.Window() + st = state.ViewState("/repo") + st.files = files + st.change = st.requested = next(c for c in files if c.path == path) + return view, window, st + + +def test_next_file_wraps_past_the_last(loading, rings): + queue, _ = loading + files = numbered() + view, window, st = at_file(files, "f4.py") + + commands.ACTIONS["next_file"](view, window, st) + assert requested_path(queue) == "f0.py" and len(rings) == 1 + + +def test_prev_file_wraps_before_the_first(loading, rings): + queue, _ = loading + files = numbered() + view, window, st = at_file(files, "f0.py") + + commands.ACTIONS["prev_file"](view, window, st) + assert requested_path(queue) == "f4.py" and len(rings) == 1 + + +def test_stepping_inside_the_list_does_not_ring(loading, rings): + queue, _ = loading + files = numbered() + view, window, st = at_file(files, "f2.py") + + commands.ACTIONS["next_file"](view, window, st) + commands.ACTIONS["prev_file"](view, window, st) + assert requested_path(queue) == "f2.py" and rings == [] + + +def test_single_file_list_rings_without_refetching(loading, rings): + queue, _ = loading + files = numbered(1) + view, window, st = at_file(files, "f0.py") + + commands.ACTIONS["next_file"](view, window, st) + commands.ACTIONS["prev_file"](view, window, st) + # it rings both times, but never re-runs git for the file already on screen + assert len(rings) == 2 and queue == [] + + +def test_first_step_without_a_shown_file_picks_an_end(loading, rings): + queue, _ = loading + files = numbered() + view, window, st = commit_panel(files) # opening a commit leaves nothing shown + assert st.index() == -1 + + commands.ACTIONS["next_file"](view, window, st) + assert requested_path(queue) == "f0.py" + st.requested = None + commands.ACTIONS["prev_file"](view, window, st) + assert requested_path(queue) == "f4.py" + # neither is a wrap: there was no position to run past + assert rings == [] diff --git a/tests/test_render.py b/tests/test_render.py index 7579d8e..fc79673 100644 --- a/tests/test_render.py +++ b/tests/test_render.py @@ -270,3 +270,36 @@ def test_summary_is_omitted_without_a_file_list(): st = make_state() render.render(view, st) assert 'class="summary"' not in header(view) + + +def test_flash_tints_the_buffer_then_clears(monkeypatch): + view = sublime.View("some text") + timers = [] + monkeypatch.setattr(render.sublime, "set_timeout", lambda fn, ms=0: timers.append((fn, ms))) + + render.flash(view) + regions, scope, _, flags = view.regions[render.BELL_KEY] + assert regions == [sublime.Region(0, len(view.text))] and flags == sublime.DRAW_NO_OUTLINE + assert scope and timers[0][1] == render.BELL_MS + + timers[0][0]() + assert render.BELL_KEY not in view.regions + + +def test_flash_survives_a_re_render(monkeypatch): + """The wrap that rings also re-renders; _replace must not wipe the bell before it is seen""" + assert render.BELL_KEY not in render.REGION_KEYS + monkeypatch.setattr(render.sublime, "set_timeout", lambda fn, ms=0: None) # hold the bell up + view = sublime.View() + render.init_view(view) + render.flash(view) + render.render(view, make_state()) + assert render.BELL_KEY in view.regions + + +def test_bell_scope_falls_back_when_the_scheme_lacks_it(monkeypatch): + view = sublime.View() + assert render._bell_scope(view) == render.SCOPE_BELL + monkeypatch.setattr(view, "style_for_scope", lambda scope: {"foreground": "#fff"}) + # an invisible bell is a silent failure, so fall back to a scope the plugin guarantees + assert render._bell_scope(view) == render.SCOPE_INS diff --git a/vdiff/commands.py b/vdiff/commands.py index 7f4c935..9a640be 100644 --- a/vdiff/commands.py +++ b/vdiff/commands.py @@ -359,11 +359,20 @@ def _step_file(delta): if not st.files: status("no file list, press f or c") return - index = st.index() + delta - if not 0 <= index < len(st.files): - status("no more files") - return - show_file(view, st, st.files[index]) + count = len(st.files) + position = st.index() + if position < 0: + # nothing shown yet: n starts at the first file, p at the last. Not a wrap. + index, wrapped = (0 if delta > 0 else count - 1), False + else: + index = position + delta + wrapped = not 0 <= index < count + index %= count # -1 is the last file, count is the first + if wrapped: + render.flash(view) + if index != position: + # a one-file list wraps onto itself: ring, but do not re-run git for the same file + show_file(view, st, st.files[index]) return act diff --git a/vdiff/render.py b/vdiff/render.py index 1b1fdf7..6b0e574 100644 --- a/vdiff/render.py +++ b/vdiff/render.py @@ -26,6 +26,11 @@ REGION_KEYS = (KEY_DEL, KEY_INS, KEY_DEL_GUTTER, KEY_INS_GUTTER, KEY_DEL_CHAR, K PHANTOM_KEY = "vdiff.header" STATUS_KEY = "vdiff" +# visual bell: a whole-buffer tint, erased on a timer. Deliberately not in REGION_KEYS, so the +# re-render a wrap triggers does not wipe it before it has been seen. +BELL_KEY = "vdiff.bell" +BELL_MS = 90 + # a block phantom anchored at point 0 is drawn below row 0, so row 0 stays empty and the file starts at row 1. # Only index_point / caret_index know about it. _HEADER_ROWS = 1 @@ -38,6 +43,8 @@ SCOPE_INS_CHAR = "diff.inserted.char" # gutter dots use the scope's foreground SCOPE_DEL_MARK = "markup.deleted" SCOPE_INS_MARK = "markup.inserted" +# ST4 defines region.ish in every bundled scheme for exactly this kind of transient marking +SCOPE_BELL = "region.yellowish" # rules written into a colour-scheme override when the active scheme lacks the scopes above FALLBACK_RULES = [ @@ -55,7 +62,7 @@ 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("n p", "next / previous file (wraps)", "file", ("next_file", "prev_file")), Key("o \u23ce", "open the real file at this line", "open", ("open",)), Key("j k", "down / up one line (vim)", "", ("line_down", "line_up")), ]), @@ -176,6 +183,23 @@ def _mtime(path): return None +def flash(view): + """Visual bell: tint the buffer for a moment, for feedback that has nothing to say in words""" + view.add_regions(BELL_KEY, [sublime.Region(0, view.size())], _bell_scope(view), flags=sublime.DRAW_NO_OUTLINE) + # unconditional erase: overlapping flashes may cut one short, but none can leave the tint stuck + sublime.set_timeout(lambda: view.is_valid() and view.erase_regions(BELL_KEY), BELL_MS) + + +def _bell_scope(view): + """region.yellowish, or a scope the plugin guarantees a background for when a scheme lacks it""" + try: + if view.style_for_scope(SCOPE_BELL).get("background"): + return SCOPE_BELL + except Exception: + LOG.debug("style_for_scope failed for %s", SCOPE_BELL, exc_info=True) + return SCOPE_INS + + def render_message(view, st, message): """Show a one-line notice instead of a file (nothing to review, binary, ...)""" st.change = None