fix: confirm mismatched multi-card updates - #180
Open
kuator wants to merge 4 commits into
Open
Conversation
kuator
force-pushed
the
fix/confirm-mismatched-recent-notes
branch
from
August 12, 2026 07:16
4946241 to
4b76c04
Compare
Contributor
Author
|
A few times I updated unrelated note because I forgot, so I added handling a single-mismatched card as well |
Comment on lines
+706
to
+809
| local function test_update_last_note_confirms_unrelated_notes() | ||
| local now_ms = os.time() * 1000 | ||
| local note_ids = { now_ms - 5, now_ms - 4, now_ms - 3, now_ms - 2, now_ms - 1 } | ||
| local sentences = { "unrelated one", "unrelated two", "target", "<b>target</b>", "target" } | ||
| local updated = false | ||
| local notification = nil | ||
| local original_notify = h.notify | ||
| local original_add_key_binding = mp.add_forced_key_binding | ||
| local original_remove_key_binding = mp.remove_key_binding | ||
| local original_create_osd_overlay = mp.create_osd_overlay | ||
| local confirmation_bindings = {} | ||
| local confirmation_overlay = { update = function() return end, remove = function() return end } | ||
| local cleared_state = 0 | ||
| local refreshed_menu = 0 | ||
| h.notify = function(message) | ||
| notification = message | ||
| end | ||
| mp.add_forced_key_binding = function(key, _, fn) | ||
| confirmation_bindings[key] = fn | ||
| end | ||
| mp.remove_key_binding = function() | ||
| return | ||
| end | ||
| mp.create_osd_overlay = function() | ||
| return confirmation_overlay | ||
| end | ||
|
|
||
| local test_exporter = make_exporter().init( | ||
| { | ||
| get_last_note_ids = function() | ||
| return note_ids | ||
| end, | ||
| get_note_fields = function(note_id) | ||
| for i, id in ipairs(note_ids) do | ||
| if id == note_id then | ||
| return { SentKanji = sentences[i] } | ||
| end | ||
| end | ||
| end, | ||
| }, | ||
| { | ||
| get_cards = function() return 5 end, | ||
| clear_options = function() cleared_state = cleared_state + 1 end, | ||
| }, | ||
| { | ||
| clear = function() cleared_state = cleared_state + 1 end, | ||
| menu = { update = function() refreshed_menu = refreshed_menu + 1 end }, | ||
| }, | ||
| nil, | ||
| nil, | ||
| { | ||
| fail_if_not_ready = function() return end, | ||
| config = function() | ||
| return { | ||
| sentence_field = "SentKanji", | ||
| reload_config_before_card_creation = false, | ||
| } | ||
| end, | ||
| } | ||
| ) | ||
| test_exporter.update_notes = function() | ||
| updated = true | ||
| end | ||
|
|
||
| test_exporter.update_last_note(false) | ||
| h.assert_equals(updated, false) | ||
| h.assert_equals(h.is_substr(confirmation_overlay.data, "Only the newest 3 of 5 notes share SentKanji."), true) | ||
| h.assert_equals(h.is_substr(confirmation_overlay.data, "Update all 5 anyway?"), true) | ||
| h.assert_equals(type(confirmation_bindings.y), "function") | ||
| h.assert_equals(type(confirmation_bindings.n), "function") | ||
| h.assert_equals(type(confirmation_bindings.ENTER), "function") | ||
| h.assert_equals(type(confirmation_bindings.left), "function") | ||
| h.assert_equals(type(confirmation_bindings.right), "function") | ||
| h.assert_equals(h.is_substr(confirmation_overlay.data, OSD:new():blue("[n] No"):get_text()), true) | ||
|
|
||
| confirmation_bindings.left() | ||
| h.assert_equals(h.is_substr(confirmation_overlay.data, OSD:new():blue("[y] Yes"):get_text()), true) | ||
| confirmation_bindings.right() | ||
| h.assert_equals(h.is_substr(confirmation_overlay.data, OSD:new():blue("[n] No"):get_text()), true) | ||
| confirmation_bindings.left() | ||
| confirmation_bindings.ENTER() | ||
| h.assert_equals(updated, true) | ||
|
|
||
| updated = false | ||
| test_exporter.update_last_note(false) | ||
| confirmation_bindings.y() | ||
| h.assert_equals(updated, true) | ||
|
|
||
| updated = false | ||
| test_exporter.update_last_note(false) | ||
| confirmation_bindings.n() | ||
| h.assert_equals(updated, false) | ||
| h.assert_equals(notification, "Card update cancelled.") | ||
| h.assert_equals(cleared_state, 2) | ||
| h.assert_equals(refreshed_menu, 1) | ||
|
|
||
| sentences[1], sentences[2] = "target", "target" | ||
| test_exporter.update_last_note(false) | ||
| h.assert_equals(updated, true) | ||
| h.notify = original_notify | ||
| mp.add_forced_key_binding = original_add_key_binding | ||
| mp.remove_key_binding = original_remove_key_binding | ||
| mp.create_osd_overlay = original_create_osd_overlay | ||
| end |
Member
There was a problem hiding this comment.
This function is longer than 21 lines. Maybe it could be split into smaller logical chunks. The same applies to test_update_notes_confirms_unrelated_single_note.
Member
|
Have you tried mpv's https://github.com/mpv-player/mpv/blob/f4d13e1c2c91f3a56e589aef9cb44cbc02e26e47/DOCS/man/select.rst local input = require('mp.input')
local function confirm_yes_no(prompt, on_result)
local resolved = false
local function finish(result)
if resolved then return end -- guard against double-resolve (submit+closed)
resolved = true
on_result(result)
end
input.select {
prompt = prompt,
items = { "Yes", "No" },
default_item = 1, -- [1] "Yes" highlighted by default
-- submit(idx): 1-based index into `items`.
submit = function(idx)
finish(idx == 1)
end,
-- closed: fired on dismiss (e.g. ESC or when the prompt is superseded).
-- Treated as neither Yes nor No.
closed = function()
finish(nil)
end,
}
endUsage: confirm_yes_no("Update this note anyway?", function(confirmed)
if confirmed == true then
proceed_with_update()
else
clear_update_state()
h.notify("Card update cancelled.", "info", 2)
end
end) |
Contributor
Author
|
Sorry, I've been busy late, will try to addres your comments when I have more time |
kuator
force-pushed
the
fix/confirm-mismatched-recent-notes
branch
from
August 16, 2026 14:27
ea5a891 to
26b21f3
Compare
kuator
force-pushed
the
fix/confirm-mismatched-recent-notes
branch
from
August 16, 2026 16:19
26b21f3 to
988311a
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
y, or cancel withnor a ten-second timeoutWhy
If the card count is accidentally set too high,
update_last_notecurrently updates older unrelated notes as long as they are within the ten-minute window. This adds a safeguard without preventing intentional overwrites when the heuristic produces a false positive.Testing
luajit tests/run.luagit diff --check