From bacc90cd24fe146d1e4dd190fc64362caaba79ac Mon Sep 17 00:00:00 2001 From: sudacode Date: Mon, 27 Apr 2026 20:14:09 -0700 Subject: [PATCH] fix: accept modified digits for multi-line sentence mining --- ...p-digits-for-multi-line-sentence-mining.md | 57 ++++++++ changes/309-multi-mine-modified-digits.md | 4 + package.json | 2 +- plugin/subminer/session_bindings.lua | 39 +++-- scripts/test-plugin-session-bindings.lua | 134 ++++++++++++++++++ src/renderer/handlers/keyboard.test.ts | 26 ++++ src/renderer/handlers/keyboard.ts | 8 +- 7 files changed, 259 insertions(+), 11 deletions(-) create mode 100644 backlog/tasks/task-309 - Accept-modified-follow-up-digits-for-multi-line-sentence-mining.md create mode 100644 changes/309-multi-mine-modified-digits.md create mode 100644 scripts/test-plugin-session-bindings.lua diff --git a/backlog/tasks/task-309 - Accept-modified-follow-up-digits-for-multi-line-sentence-mining.md b/backlog/tasks/task-309 - Accept-modified-follow-up-digits-for-multi-line-sentence-mining.md new file mode 100644 index 00000000..078d151d --- /dev/null +++ b/backlog/tasks/task-309 - Accept-modified-follow-up-digits-for-multi-line-sentence-mining.md @@ -0,0 +1,57 @@ +--- +id: TASK-309 +title: Accept modified follow-up digits for multi-line sentence mining +status: Done +assignee: + - '@codex' +created_date: '2026-04-27 20:06' +updated_date: '2026-04-27 20:15' +labels: + - bug + - linux + - shortcuts +dependencies: [] +priority: high +--- + +## Description + + + +On Linux, `Ctrl+Shift+S` starts multi-line sentence-card mining, but the follow-up digit is not accepted and the prompt times out. Restore reliable digit capture for the multi-mine flow, including the common case where the original shortcut modifiers are still held briefly while pressing the digit. + + + +## Acceptance Criteria + + + +- [x] #1 `Ctrl+Shift+S` followed by a number-row digit creates a counted `mineSentenceMultiple` request instead of timing out. +- [x] #2 Follow-up digit capture works when the user has not fully released `Ctrl`/`Shift` after the starter shortcut. +- [x] #3 Regression coverage includes renderer session bindings and mpv plugin numeric selection. + + +## Implementation Notes + + + +Backlog MCP unavailable in this session, so this task is tracked via repo-local backlog files. + +Implemented renderer digit extraction from `KeyboardEvent.code` for pending numeric selection, so shifted number-row events such as `Ctrl+Shift+Digit3` still dispatch count `3`. Updated the mpv plugin session-binding numeric selector to register bare digits plus the starter shortcut modifier combinations, so plugin-owned `Ctrl+Shift+S` can accept a follow-up digit before the modifiers are fully released. + +Verification: + +- `bun test src/renderer/handlers/keyboard.test.ts src/core/services/overlay-shortcut-handler.test.ts src/core/services/overlay-window.test.ts` +- `bun run test:plugin:src` +- `bun run changelog:lint` +- `bun x prettier --check src/renderer/handlers/keyboard.ts src/renderer/handlers/keyboard.test.ts package.json 'changes/309-multi-mine-modified-digits.md' 'backlog/tasks/task-309 - Accept-modified-follow-up-digits-for-multi-line-sentence-mining.md'` + + + +## Final Summary + + + +Restored multi-line sentence-card digit capture for the case where `Ctrl`/`Shift` are still held after `Ctrl+Shift+S`. The renderer now accepts digits by physical `Digit1`-`Digit9`/`Numpad1`-`Numpad9` code during pending numeric selection, and the mpv plugin registers the matching modified digit bindings for session-binding numeric prompts. + + diff --git a/changes/309-multi-mine-modified-digits.md b/changes/309-multi-mine-modified-digits.md new file mode 100644 index 00000000..98ce24ca --- /dev/null +++ b/changes/309-multi-mine-modified-digits.md @@ -0,0 +1,4 @@ +type: fixed +area: shortcuts + +- Accept follow-up number-row digits for multi-line subtitle mining even when the original shortcut modifiers are still held. diff --git a/package.json b/package.json index aafafe65..5bbc5387 100644 --- a/package.json +++ b/package.json @@ -45,7 +45,7 @@ "test:config:src": "bun test src/config/config.test.ts src/config/path-resolution.test.ts src/config/resolve/anki-connect.test.ts src/config/resolve/integrations.test.ts src/config/resolve/subtitle-style.test.ts src/config/resolve/jellyfin.test.ts src/config/definitions/domain-registry.test.ts src/generate-config-example.test.ts src/verify-config-example.test.ts", "test:config:dist": "bun test dist/config/config.test.js dist/config/path-resolution.test.js dist/config/resolve/anki-connect.test.js dist/config/resolve/integrations.test.js dist/config/resolve/subtitle-style.test.js dist/config/resolve/jellyfin.test.js dist/config/definitions/domain-registry.test.js dist/generate-config-example.test.js dist/verify-config-example.test.js", "test:config:smoke:dist": "bun test dist/config/path-resolution.test.js", - "test:plugin:src": "lua scripts/test-plugin-lua-compat.lua && lua scripts/test-plugin-start-gate.lua && lua scripts/test-plugin-binary-windows.lua", + "test:plugin:src": "lua scripts/test-plugin-lua-compat.lua && lua scripts/test-plugin-start-gate.lua && lua scripts/test-plugin-session-bindings.lua && lua scripts/test-plugin-binary-windows.lua", "test:launcher:smoke:src": "bun test launcher/smoke.e2e.test.ts", "test:launcher:src": "bun test launcher/config.test.ts launcher/config-domain-parsers.test.ts launcher/config/cli-parser-builder.test.ts launcher/config/args-normalizer.test.ts launcher/mpv.test.ts launcher/picker.test.ts launcher/parse-args.test.ts launcher/main.test.ts launcher/commands/command-modules.test.ts launcher/smoke.e2e.test.ts && bun run test:plugin:src", "test:core:src": "bun test src/cli/args.test.ts src/cli/help.test.ts src/shared/setup-state.test.ts src/core/services/cli-command.test.ts src/core/services/field-grouping-overlay.test.ts src/core/services/numeric-shortcut-session.test.ts src/core/services/secondary-subtitle.test.ts src/core/services/mpv-render-metrics.test.ts src/core/services/overlay-content-measurement.test.ts src/core/services/mpv-control.test.ts src/core/services/mpv.test.ts src/core/services/runtime-options-ipc.test.ts src/core/services/runtime-config.test.ts src/core/services/yomitan-extension-paths.test.ts src/core/services/config-hot-reload.test.ts src/core/services/discord-presence.test.ts src/core/services/tokenizer.test.ts src/core/services/tokenizer/annotation-stage.test.ts src/core/services/tokenizer/parser-selection-stage.test.ts src/core/services/tokenizer/parser-enrichment-stage.test.ts src/core/services/subsync.test.ts src/core/services/overlay-bridge.test.ts src/core/services/overlay-shortcut-handler.test.ts src/core/services/stats-window.test.ts src/main/runtime/stats-server-routing.test.ts src/core/services/mining.test.ts src/core/services/anki-jimaku.test.ts src/core/services/jimaku-download-path.test.ts src/core/services/jellyfin.test.ts src/core/services/jellyfin-remote.test.ts src/core/services/immersion-tracker-service.test.ts src/core/services/overlay-runtime-init.test.ts src/core/services/app-ready.test.ts src/core/services/startup-bootstrap.test.ts src/core/services/subtitle-processing-controller.test.ts src/core/services/anilist/anilist-update-queue.test.ts src/core/services/anilist/rate-limiter.test.ts src/core/services/jlpt-token-filter.test.ts src/core/services/subtitle-position.test.ts src/core/utils/shortcut-config.test.ts src/main/runtime/first-run-setup-plugin.test.ts src/main/runtime/first-run-setup-service.test.ts src/main/runtime/first-run-setup-window.test.ts src/main/runtime/tray-runtime.test.ts src/main/runtime/tray-main-actions.test.ts src/main/runtime/tray-main-deps.test.ts src/main/runtime/tray-runtime-handlers.test.ts src/main/runtime/cli-command-context-main-deps.test.ts src/main/runtime/app-ready-main-deps.test.ts src/renderer/error-recovery.test.ts src/renderer/subtitle-render.test.ts src/renderer/handlers/mouse.test.ts src/renderer/handlers/keyboard.test.ts src/renderer/modals/jimaku.test.ts src/subsync/utils.test.ts src/main/anilist-url-guard.test.ts src/window-trackers/hyprland-tracker.test.ts src/window-trackers/x11-tracker.test.ts src/window-trackers/windows-helper.test.ts src/window-trackers/windows-tracker.test.ts launcher/config.test.ts launcher/config-domain-parsers.test.ts launcher/config/cli-parser-builder.test.ts launcher/config/args-normalizer.test.ts launcher/parse-args.test.ts launcher/main.test.ts launcher/commands/command-modules.test.ts launcher/setup-gate.test.ts stats/src/lib/api-client.test.ts", diff --git a/plugin/subminer/session_bindings.lua b/plugin/subminer/session_bindings.lua index 718a1041..ef30528f 100644 --- a/plugin/subminer/session_bindings.lua +++ b/plugin/subminer/session_bindings.lua @@ -225,16 +225,39 @@ function M.create(ctx) end end - local function start_numeric_selection(action_id, timeout_ms) + local function build_modifier_prefixes(modifiers) + local prefixes = { "" } + if type(modifiers) ~= "table" then + return prefixes + end + + for _, modifier in ipairs(modifiers) do + local mapped = MODIFIER_MAP[modifier] + if mapped then + local existing_count = #prefixes + for index = 1, existing_count do + prefixes[#prefixes + 1] = prefixes[index] .. mapped .. "+" + end + end + end + return prefixes + end + + local function start_numeric_selection(action_id, timeout_ms, starter_modifiers) clear_numeric_selection(false) + local modifier_prefixes = build_modifier_prefixes(starter_modifiers) for digit = 1, 9 do local digit_string = tostring(digit) - local name = "subminer-session-digit-" .. digit_string - state.session_numeric_binding_names[#state.session_numeric_binding_names + 1] = name - mp.add_forced_key_binding(digit_string, name, function() - clear_numeric_selection(false) - invoke_cli_action(action_id, { count = digit }) - end) + for _, prefix in ipairs(modifier_prefixes) do + local key_name = prefix .. digit_string + local modifier_name = prefix:gsub("[^%w]", "-") + local name = "subminer-session-digit-" .. modifier_name .. digit_string + state.session_numeric_binding_names[#state.session_numeric_binding_names + 1] = name + mp.add_forced_key_binding(key_name, name, function() + clear_numeric_selection(false) + invoke_cli_action(action_id, { count = digit }) + end) + end end state.session_numeric_binding_names[#state.session_numeric_binding_names + 1] = @@ -272,7 +295,7 @@ function M.create(ctx) end if binding.actionId == "copySubtitleMultiple" or binding.actionId == "mineSentenceMultiple" then - start_numeric_selection(binding.actionId, numeric_selection_timeout_ms) + start_numeric_selection(binding.actionId, numeric_selection_timeout_ms, binding.key.modifiers) return end diff --git a/scripts/test-plugin-session-bindings.lua b/scripts/test-plugin-session-bindings.lua new file mode 100644 index 00000000..da433a52 --- /dev/null +++ b/scripts/test-plugin-session-bindings.lua @@ -0,0 +1,134 @@ +package.path = "plugin/subminer/?.lua;" .. package.path + +local session_bindings = require("session_bindings") + +local function assert_true(condition, message) + if condition then + return + end + error(message) +end + +local artifact_path = ".tmp/test-plugin-session-bindings.json" +os.execute("mkdir -p .tmp") +local handle = assert(io.open(artifact_path, "w")) +handle:write("__SESSION_BINDINGS__") +handle:close() + +local recorded = { + bindings = {}, + removed = {}, + async_calls = {}, + osd = {}, +} + +local mp = {} + +function mp.add_forced_key_binding(keys, name, fn) + recorded.bindings[#recorded.bindings + 1] = { + keys = keys, + name = name, + fn = fn, + } +end + +function mp.remove_key_binding(name) + recorded.removed[#recorded.removed + 1] = name +end + +function mp.add_timeout(seconds, callback) + return { + seconds = seconds, + callback = callback, + killed = false, + kill = function(self) + self.killed = true + end, + } +end + +function mp.osd_message(message) + recorded.osd[#recorded.osd + 1] = message +end + +local ctx = { + mp = mp, + utils = { + parse_json = function(raw) + if raw ~= "__SESSION_BINDINGS__" then + return nil, "unexpected artifact" + end + return { + numericSelectionTimeoutMs = 3000, + bindings = { + { + key = { + code = "KeyS", + modifiers = { "ctrl", "shift" }, + }, + actionType = "session-action", + actionId = "mineSentenceMultiple", + }, + }, + }, nil + end, + }, + state = { + binary_path = "/tmp/subminer", + session_binding_names = {}, + session_numeric_binding_names = {}, + session_numeric_selection = nil, + }, + process = { + check_binary_available = function() + return true + end, + run_binary_command_async = function(args) + recorded.async_calls[#recorded.async_calls + 1] = args + end, + }, + environment = { + resolve_session_bindings_artifact_path = function() + return artifact_path + end, + }, + log = { + subminer_log = function() end, + show_osd = function(message) + recorded.osd[#recorded.osd + 1] = message + end, + }, +} + +local bindings = session_bindings.create(ctx) +assert_true(bindings.register_bindings(), "session bindings should register") + +local starter = nil +for _, binding in ipairs(recorded.bindings) do + if binding.keys == "Ctrl+Shift+s" then + starter = binding + break + end +end +assert_true(starter ~= nil, "multi-mine starter binding should be registered") + +starter.fn() + +local modified_digit = nil +for _, binding in ipairs(recorded.bindings) do + if binding.keys == "Ctrl+Shift+3" then + modified_digit = binding + break + end +end +assert_true(modified_digit ~= nil, "numeric selection should bind Ctrl+Shift+3") + +modified_digit.fn() + +local call = recorded.async_calls[#recorded.async_calls] +assert_true(call ~= nil, "modified digit should invoke CLI action") +assert_true(call[1] == "/tmp/subminer", "CLI action should use configured binary") +assert_true(call[2] == "--mine-sentence-count", "CLI action should mine sentence count") +assert_true(call[3] == "3", "CLI action should pass selected count") + +print("plugin session binding regression tests: OK") diff --git a/src/renderer/handlers/keyboard.test.ts b/src/renderer/handlers/keyboard.test.ts index 13addd2b..2cf1aeaf 100644 --- a/src/renderer/handlers/keyboard.test.ts +++ b/src/renderer/handlers/keyboard.test.ts @@ -1168,6 +1168,32 @@ test('session binding: copy subtitle multiple captures follow-up digit locally', } }); +test('session binding: mine sentence multiple captures modified follow-up digit locally', async () => { + const { handlers, testGlobals } = createKeyboardHandlerHarness(); + + try { + await handlers.setupMpvInputForwarding(); + handlers.updateSessionBindings([ + { + sourcePath: 'shortcuts.mineSentenceMultiple', + originalKey: 'Ctrl+Shift+S', + key: { code: 'KeyS', modifiers: ['ctrl', 'shift'] }, + actionType: 'session-action', + actionId: 'mineSentenceMultiple', + }, + ] as never); + + testGlobals.dispatchKeydown({ key: 'S', code: 'KeyS', ctrlKey: true, shiftKey: true }); + testGlobals.dispatchKeydown({ key: '#', code: 'Digit3', ctrlKey: true, shiftKey: true }); + + assert.deepEqual(testGlobals.sessionActions, [ + { actionId: 'mineSentenceMultiple', payload: { count: 3 } }, + ]); + } finally { + testGlobals.restore(); + } +}); + test('keyboard mode: h moves left when popup is closed', async () => { const { ctx, handlers, testGlobals } = createKeyboardHandlerHarness(); diff --git a/src/renderer/handlers/keyboard.ts b/src/renderer/handlers/keyboard.ts index 4558a0d7..be3e640b 100644 --- a/src/renderer/handlers/keyboard.ts +++ b/src/renderer/handlers/keyboard.ts @@ -176,13 +176,17 @@ export function createKeyboardHandlers( return true; } - if (!/^[1-9]$/.test(e.key) || e.ctrlKey || e.metaKey || e.altKey || e.shiftKey) { + const digit = /^[1-9]$/.test(e.key) + ? e.key + : (e.code.match(/^(?:Digit|Numpad)([1-9])$/)?.[1] ?? null); + + if (!digit) { e.preventDefault(); return true; } e.preventDefault(); - const count = Number(e.key); + const count = Number(digit); const actionId = pendingNumericSelection.actionId; cancelPendingNumericSelection(false); void window.electronAPI.dispatchSessionAction(actionId, { count });