added j / k for vim-style line movement
j and k move the caret one line, delegated to sublime's own `move` command so they behave exactly like the arrow keys. They dispatch through the existing ACTIONS registry, so the keymap, the status-bar hints and the ? popup all stay in sync through the single KEY_GROUPS table. Unlike the other letter keys, j / k keep no command_mode context: the view is read-only, so leaving them live while Vintage is in command mode can only ever move the caret. That asymmetry was previously documented only in a comment, which was wrong (it claimed j / k were the only unguarded bindings, but escape has always carried its own context instead). It is now pinned by test_command_mode_guard_is_deliberate, so normalising the keymap fails the suite with the reason attached. Also tidies two long lines in commands.py. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -23,6 +23,10 @@
|
|||||||
"context": [{"key": "setting.vdiff_view"}, {"key": "setting.command_mode", "operand": false}] },
|
"context": [{"key": "setting.vdiff_view"}, {"key": "setting.command_mode", "operand": false}] },
|
||||||
{ "keys": ["p"], "command": "vdiff_action", "args": {"action": "prev_file"},
|
{ "keys": ["p"], "command": "vdiff_action", "args": {"action": "prev_file"},
|
||||||
"context": [{"key": "setting.vdiff_view"}, {"key": "setting.command_mode", "operand": false}] },
|
"context": [{"key": "setting.vdiff_view"}, {"key": "setting.command_mode", "operand": false}] },
|
||||||
|
{ "keys": ["j"], "command": "vdiff_action", "args": {"action": "line_down"},
|
||||||
|
"context": [{"key": "setting.vdiff_view"}] },
|
||||||
|
{ "keys": ["k"], "command": "vdiff_action", "args": {"action": "line_up"},
|
||||||
|
"context": [{"key": "setting.vdiff_view"}] },
|
||||||
{ "keys": ["?"], "command": "vdiff_action", "args": {"action": "help"},
|
{ "keys": ["?"], "command": "vdiff_action", "args": {"action": "help"},
|
||||||
"context": [{"key": "setting.vdiff_view"}, {"key": "setting.command_mode", "operand": false}] },
|
"context": [{"key": "setting.vdiff_view"}, {"key": "setting.command_mode", "operand": false}] },
|
||||||
{ "keys": ["escape"], "command": "vdiff_action", "args": {"action": "close"},
|
{ "keys": ["escape"], "command": "vdiff_action", "args": {"action": "close"},
|
||||||
|
|||||||
@@ -22,6 +22,7 @@ Keys inside the review view:
|
|||||||
| `o` or Enter | open the real file at this line |
|
| `o` or Enter | open the real file at this line |
|
||||||
| `]` / `[` | next / previous change |
|
| `]` / `[` | next / previous change |
|
||||||
| `n` / `p` | next / previous file of the current list |
|
| `n` / `p` | next / previous file of the current list |
|
||||||
|
| `j` / `k` | move down / up one line (vim keys) |
|
||||||
| `r` | refresh |
|
| `r` | refresh |
|
||||||
| `?` | show this key list in a popup |
|
| `?` | show this key list in a popup |
|
||||||
| Esc | close the view |
|
| Esc | close the view |
|
||||||
|
|||||||
@@ -135,3 +135,13 @@ def test_run_async_reports_generic_error(monkeypatch):
|
|||||||
|
|
||||||
commands.run_async(boom, lambda r: messages.append("done"))
|
commands.run_async(boom, lambda r: messages.append("done"))
|
||||||
assert messages == ["vDiff: boom"]
|
assert messages == ["vDiff: boom"]
|
||||||
|
|
||||||
|
|
||||||
|
def test_j_and_k_move_the_caret_one_line():
|
||||||
|
view = sublime.View()
|
||||||
|
commands.ACTIONS["line_down"](view, None, None)
|
||||||
|
commands.ACTIONS["line_up"](view, None, None)
|
||||||
|
assert view.commands == [
|
||||||
|
("move", {"by": "lines", "forward": True}),
|
||||||
|
("move", {"by": "lines", "forward": False}),
|
||||||
|
]
|
||||||
|
|||||||
@@ -134,7 +134,7 @@ def test_help_popup_lists_every_key():
|
|||||||
assert view.is_popup_visible()
|
assert view.is_popup_visible()
|
||||||
for group in ("Navigate", "Panels", "This file", "View"):
|
for group in ("Navigate", "Panels", "This file", "View"):
|
||||||
assert ">{}<".format(group) in html
|
assert ">{}<".format(group) in html
|
||||||
for keys in ("] [", "n p", "o ⏎", "f", "c", "s", "u", "d", "r", "?", "esc"):
|
for keys in ("] [", "n p", "o ⏎", "j k", "f", "c", "s", "u", "d", "r", "?", "esc"):
|
||||||
assert ">{}<".format(keys) in html
|
assert ">{}<".format(keys) in html
|
||||||
assert "next / previous change" in html and "open the real file at this line" 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
|
# working-tree mode: nothing dimmed, every single-action row is a link
|
||||||
@@ -176,11 +176,15 @@ def test_help_popup_toggles_and_navigates():
|
|||||||
DISPLAY_KEYS = {"enter": "⏎", "escape": "esc"}
|
DISPLAY_KEYS = {"enter": "⏎", "escape": "esc"}
|
||||||
|
|
||||||
|
|
||||||
def test_keymap_matches_help_table():
|
def keymap_bindings():
|
||||||
"""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")
|
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:
|
with open(path, encoding="utf-8") as f:
|
||||||
bindings = json.load(f)
|
return json.load(f)
|
||||||
|
|
||||||
|
|
||||||
|
def test_keymap_matches_help_table():
|
||||||
|
"""The keymap, ACTIONS and the popup agree on both the actions and the keys that reach them"""
|
||||||
|
bindings = keymap_bindings()
|
||||||
|
|
||||||
assert all(b["command"] == "vdiff_action" for b in bindings)
|
assert all(b["command"] == "vdiff_action" for b in bindings)
|
||||||
bound = {b["args"]["action"] for b in bindings}
|
bound = {b["args"]["action"] for b in bindings}
|
||||||
@@ -192,6 +196,20 @@ def test_keymap_matches_help_table():
|
|||||||
assert bound_keys == {token for k in render.KEYS for token in k.keys.split()}
|
assert bound_keys == {token for k in render.KEYS for token in k.keys.split()}
|
||||||
|
|
||||||
|
|
||||||
|
# j / k stay live in Vintage command mode on purpose: the view is read-only, so the worst they can do
|
||||||
|
# is move the caret, and a vim user should not have to leave command mode to scroll a diff.
|
||||||
|
# escape carries its own context (panel / overlay / popup / selection guards) instead.
|
||||||
|
UNGUARDED_ACTIONS = {"line_down", "line_up", "close"}
|
||||||
|
|
||||||
|
|
||||||
|
def test_command_mode_guard_is_deliberate():
|
||||||
|
"""Every binding yields to Vintage except the ones that documented a reason not to"""
|
||||||
|
guard = {"key": "setting.command_mode", "operand": False}
|
||||||
|
for binding in keymap_bindings():
|
||||||
|
unguarded = binding["args"]["action"] in UNGUARDED_ACTIONS
|
||||||
|
assert (guard in binding["context"]) != unguarded, binding["keys"]
|
||||||
|
|
||||||
|
|
||||||
def test_minihtml_stylesheets_reject_unsupported_selectors():
|
def test_minihtml_stylesheets_reject_unsupported_selectors():
|
||||||
"""minihtml drops the rest of a stylesheet at a selector it cannot parse; these are the forms
|
"""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"""
|
that silently broke HELP_CSS, so no stylesheet constant may use them"""
|
||||||
|
|||||||
@@ -230,8 +230,7 @@ class VdiffCommitsCommand(sublime_plugin.WindowCommand):
|
|||||||
|
|
||||||
def show_commits(self, view, st, commits, selected):
|
def show_commits(self, view, st, commits, selected):
|
||||||
items = [
|
items = [
|
||||||
sublime.QuickPanelItem(c.subject or "(no subject)", "{} · {}".format(c.author, c.date), c.short, COMMIT_KIND)
|
sublime.QuickPanelItem(c.subject or "(no subject)", "{} · {}".format(c.author, c.date), c.short, COMMIT_KIND) for c in commits
|
||||||
for c in commits
|
|
||||||
]
|
]
|
||||||
for i, c in enumerate(commits):
|
for i, c in enumerate(commits):
|
||||||
if c.sha == st.sha:
|
if c.sha == st.sha:
|
||||||
@@ -306,7 +305,9 @@ def _confirm_discard(change):
|
|||||||
if change.status == "?":
|
if change.status == "?":
|
||||||
message = "Delete the untracked file {}?\n\nThis cannot be undone.".format(change.path)
|
message = "Delete the untracked file {}?\n\nThis cannot be undone.".format(change.path)
|
||||||
else:
|
else:
|
||||||
message = "Discard all changes to {}?\n\nThe file goes back to its last committed version. This cannot be undone.".format(change.path)
|
message = "Discard all changes to {}?\n\nThe file goes back to its last committed version. This cannot be undone.".format(
|
||||||
|
change.path
|
||||||
|
)
|
||||||
return sublime.ok_cancel_dialog(message, "Discard")
|
return sublime.ok_cancel_dialog(message, "Discard")
|
||||||
|
|
||||||
|
|
||||||
@@ -360,6 +361,13 @@ def _step_change(delta):
|
|||||||
return act
|
return act
|
||||||
|
|
||||||
|
|
||||||
|
def _move_line(forward):
|
||||||
|
def act(view, window, st):
|
||||||
|
view.run_command("move", {"by": "lines", "forward": forward})
|
||||||
|
|
||||||
|
return act
|
||||||
|
|
||||||
|
|
||||||
ACTIONS = {
|
ACTIONS = {
|
||||||
"files": _act_files,
|
"files": _act_files,
|
||||||
"commits": _act_commits,
|
"commits": _act_commits,
|
||||||
@@ -374,6 +382,8 @@ ACTIONS = {
|
|||||||
"prev_file": _step_file(-1),
|
"prev_file": _step_file(-1),
|
||||||
"next_change": _step_change(1),
|
"next_change": _step_change(1),
|
||||||
"prev_change": _step_change(-1),
|
"prev_change": _step_change(-1),
|
||||||
|
"line_down": _move_line(True),
|
||||||
|
"line_up": _move_line(False),
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -57,6 +57,7 @@ KEY_GROUPS = [
|
|||||||
Key("] [", "next / previous change", "change", ("next_change", "prev_change")),
|
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", "file", ("next_file", "prev_file")),
|
||||||
Key("o \u23ce", "open the real file at this line", "open", ("open",)),
|
Key("o \u23ce", "open the real file at this line", "open", ("open",)),
|
||||||
|
Key("j k", "down / up one line (vim)", "", ("line_down", "line_up")),
|
||||||
]),
|
]),
|
||||||
("Panels", True, [
|
("Panels", True, [
|
||||||
Key("f", "changed files", "files", ("files",)),
|
Key("f", "changed files", "files", ("files",)),
|
||||||
|
|||||||
Reference in New Issue
Block a user