From 7eece407772d7c5cfba93ba914439094f0d9fbf2 Mon Sep 17 00:00:00 2001 From: Craig Jennings Date: Tue, 26 May 2026 19:58:21 -0500 Subject: refactor: dedupe presenters, group defcustoms, and fill in docstrings A cleanup pass over org-drill internals, squashed from the refactor/wave3-cleanup branch. No behavior change. Each step kept the existing tests green and added its own. I shared two duplicated helpers across the language card getters: org-drill--read-property-string and org-drill--face-from-alist. I factored the cloze body-scan out of the two multicloze presenters into org-drill--cloze-body-bounds, org-drill--count-cloze-matches, and org-drill--hide-cloze-by-index, so each presenter just picks which indices to hide. I pulled the presenter resolution and the four-way result classification out of org-drill-entry-f into org-drill--resolve-presenter and org-drill--classify-presentation-result, untangling the pivot of every drill iteration. I split the 37 defcustoms (and the three cloze faces) into four customize sub-groups (display, algorithm, session, leech) so customize-group org-drill is navigable. There's no leitner group because the Leitner settings are defvars. I documented the 22 defuns that had no docstring, rewrote the corrupted org-drill-presentation-prompt-in-mini-buffer docstring, and switched eleven docstrings to the imperative "Return" (issue #2). --- tests/test-org-drill-custom-groups.el | 37 +++++++++++++++++++ tests/test-org-drill-entry-f.el | 42 ++++++++++++++++++++++ tests/test-org-drill-explain-and-language-cards.el | 28 +++++++++++++++ tests/test-org-drill-multicloze-hiding.el | 18 ++++++++++ 4 files changed, 125 insertions(+) create mode 100644 tests/test-org-drill-custom-groups.el (limited to 'tests') diff --git a/tests/test-org-drill-custom-groups.el b/tests/test-org-drill-custom-groups.el new file mode 100644 index 0000000..fe1e22c --- /dev/null +++ b/tests/test-org-drill-custom-groups.el @@ -0,0 +1,37 @@ +;;; test-org-drill-custom-groups.el --- Customize sub-group structure -*- lexical-binding: t; -*- + +;;; Commentary: +;; The defcustoms are split across four sub-groups under the top-level +;; org-drill group so M-x customize-group org-drill is navigable instead of +;; dumping 37 options in one flat list. (There is no leitner sub-group: the +;; Leitner settings are defvars, not defcustoms.) + +;;; Code: + +(require 'ert) +(require 'org-drill) + +(defconst test-org-drill-subgroups + '(org-drill-display org-drill-algorithm org-drill-session org-drill-leech) + "The customize sub-groups org-drill defines.") + +(ert-deftest test-org-drill-subgroups-exist-and-nest-under-org-drill () + "Each sub-group is defined and is a child of the org-drill group." + (dolist (g test-org-drill-subgroups) + (should (get g 'group-documentation)) + (should (assq g (get 'org-drill 'custom-group))))) + +(ert-deftest test-org-drill-defcustoms-land-in-expected-subgroups () + "A representative defcustom from each sub-group is a member of it." + (should (assq 'org-drill-spaced-repetition-algorithm + (get 'org-drill-algorithm 'custom-group))) + (should (assq 'org-drill-leech-method + (get 'org-drill-leech 'custom-group))) + (should (assq 'org-drill-scope + (get 'org-drill-session 'custom-group))) + (should (assq 'org-drill-use-visible-cloze-face-p + (get 'org-drill-display 'custom-group)))) + +(provide 'test-org-drill-custom-groups) + +;;; test-org-drill-custom-groups.el ends here diff --git a/tests/test-org-drill-entry-f.el b/tests/test-org-drill-entry-f.el index e99a171..1beed34 100644 --- a/tests/test-org-drill-entry-f.el +++ b/tests/test-org-drill-entry-f.el @@ -97,6 +97,48 @@ the complete-func (e.g., reschedule) is invoked with the session." (lambda (_) (setq complete-called t))) (should complete-called))))) +;;;; org-drill--resolve-presenter + +(ert-deftest test-org-drill-resolve-presenter-bare-symbol () + "A bare presenter symbol pairs with the default answer function." + (let ((org-drill-card-type-alist '(("simple" . my-present)))) + (should (equal '(my-present . org-drill-present-default-answer) + (org-drill--resolve-presenter "simple"))))) + +(ert-deftest test-org-drill-resolve-presenter-list-with-answer () + "A list entry uses its first element as presenter and second as answer." + (let ((org-drill-card-type-alist '(("two" my-present my-answer)))) + (should (equal '(my-present . my-answer) + (org-drill--resolve-presenter "two"))))) + +(ert-deftest test-org-drill-resolve-presenter-list-without-answer () + "A single-element list entry falls back to the default answer function." + (let ((org-drill-card-type-alist '(("one" my-present)))) + (should (equal '(my-present . org-drill-present-default-answer) + (org-drill--resolve-presenter "one"))))) + +(ert-deftest test-org-drill-resolve-presenter-unknown-is-nil () + "An unknown card type resolves to a nil presenter (which entry-f skips)." + (let ((org-drill-card-type-alist '(("simple" . my-present)))) + (should (equal '(nil . org-drill-present-default-answer) + (org-drill--resolve-presenter "no-such-type"))))) + +;;;; org-drill--classify-presentation-result + +(ert-deftest test-org-drill-classify-result-nil-is-quit () + (should (eq 'quit (org-drill--classify-presentation-result nil)))) + +(ert-deftest test-org-drill-classify-result-edit () + (should (eq 'edit (org-drill--classify-presentation-result 'edit)))) + +(ert-deftest test-org-drill-classify-result-skip () + (should (eq 'skip (org-drill--classify-presentation-result 'skip)))) + +(ert-deftest test-org-drill-classify-result-other-is-answer () + "Any other non-nil value means proceed to the answer." + (should (eq 'answer (org-drill--classify-presentation-result t))) + (should (eq 'answer (org-drill--classify-presentation-result 5)))) + (provide 'test-org-drill-entry-f) ;;; test-org-drill-entry-f.el ends here diff --git a/tests/test-org-drill-explain-and-language-cards.el b/tests/test-org-drill-explain-and-language-cards.el index 301705d..9454a0d 100644 --- a/tests/test-org-drill-explain-and-language-cards.el +++ b/tests/test-org-drill-explain-and-language-cards.el @@ -171,6 +171,34 @@ on whether the buffer ends with a newline." (face (get-text-property 0 'face (nth 0 info)))) (should (equal "red" (plist-get face :foreground)))))) +;;;; org-drill--read-property-string + +(ert-deftest test-org-drill-read-property-string-strips-one-read () + "A Lisp-readable property string is read down to its datum." + (should (equal "hablar" (org-drill--read-property-string "\"hablar\"")))) + +(ert-deftest test-org-drill-read-property-string-nil-is-nil () + "A nil property (absent) returns nil rather than erroring." + (should (null (org-drill--read-property-string nil)))) + +;;;; org-drill--face-from-alist + +(ert-deftest test-org-drill-face-from-alist-hit-returns-colour () + "A key present in the alist returns its mapped colour." + (should (equal "tomato" + (org-drill--face-from-alist "present" org-drill-verb-tense-alist "x")))) + +(ert-deftest test-org-drill-face-from-alist-is-case-insensitive () + "Lookup ignores case, matching the assoc-string call sites." + (should (equal "tomato" + (org-drill--face-from-alist "PRESENT" org-drill-verb-tense-alist "x")))) + +(ert-deftest test-org-drill-face-from-alist-miss-returns-default () + "A key absent from the alist falls back to the supplied default." + (should (equal "fallback" + (org-drill--face-from-alist "no-such-tense" + org-drill-verb-tense-alist "fallback")))) + (provide 'test-org-drill-explain-and-language-cards) ;;; test-org-drill-explain-and-language-cards.el ends here diff --git a/tests/test-org-drill-multicloze-hiding.el b/tests/test-org-drill-multicloze-hiding.el index f67e083..b225008 100644 --- a/tests/test-org-drill-multicloze-hiding.el +++ b/tests/test-org-drill-multicloze-hiding.el @@ -158,6 +158,24 @@ Verified by checking the resulting overlay's bounds match the (org-drill-present-multicloze-hide-nth (org-drill-session) -1)) (should (= 1 overlays-during-prompt)))) +;;;; org-drill--count-cloze-matches (extracted scan helper) + +(ert-deftest test-org-drill-count-cloze-matches-counts-body-clozes () + "Counts each cloze region between the body bounds." + (with-cloze-card "* Question :drill: +[A] [B] [C] [D] +" + (let ((bounds (org-drill--cloze-body-bounds))) + (should (= 4 (org-drill--count-cloze-matches (car bounds) (cdr bounds))))))) + +(ert-deftest test-org-drill-count-cloze-matches-zero-when-no-cloze () + "A body with no cloze syntax counts zero." + (with-cloze-card "* Question :drill: +No cloze syntax here. +" + (let ((bounds (org-drill--cloze-body-bounds))) + (should (= 0 (org-drill--count-cloze-matches (car bounds) (cdr bounds))))))) + (provide 'test-org-drill-multicloze-hiding) ;;; test-org-drill-multicloze-hiding.el ends here -- cgit v1.2.3