From 77eb4f046cc90d535ba30ec8a408f52cb85da63f Mon Sep 17 00:00:00 2001 From: Craig Jennings Date: Tue, 5 May 2026 03:43:25 -0500 Subject: fix: include child subtree in entry-empty-p search bound (upstream #13) kqr (2019-07-22) reported that drill entries whose answer lives inside a child sub-heading were silently skipped. Their example: a question in the heading text and the answer under `** The Answer`. The function returned t (empty) for such entries, so they never got presented during drill sessions. The cause is `(outline-next-heading)` in `org-drill-entry-empty-p`. That primitive lands on the first heading at any level, including children. So the search range was metadata-end up to the child's heading line, which excluded the child's body. Bodies that lived in child sub-headings never got searched. I switched the bound to `(org-end-of-subtree t t)`, which covers the whole subtree of the current heading and degrades gracefully at the last heading in the buffer. The reporter suggested `outline-forward-same-level`, but that primitive errors at the last sibling, which would be its own regression. `org-end-of-subtree` is the canonical Emacs idiom for this kind of bound and handles end-of-buffer correctly. I added `tests/test-org-drill-entry-empty-p.el` with 6 ERT tests across Normal, Boundary (kqr's exact fixture), and edge categories. The two regression tests fail at HEAD before the fix and pass after. One semantic note worth flagging: any subtree content now counts as non-empty, including bare child headings with no body of their own. The bug report is silent on that case and I expect it to be rare in practice. If anyone reports the new behavior as a regression, the fix would be to filter heading lines out of the graphical-character search. --- org-drill.el | 10 ++- tests/test-org-drill-entry-empty-p.el | 135 ++++++++++++++++++++++++++++++++++ 2 files changed, 143 insertions(+), 2 deletions(-) create mode 100644 tests/test-org-drill-entry-empty-p.el diff --git a/org-drill.el b/org-drill.el index 07de25d..c3a73eb 100644 --- a/org-drill.el +++ b/org-drill.el @@ -2157,11 +2157,17 @@ Note: does not actually alter the item." ;; This version is about 5x faster than the old version, above. (defun org-drill-entry-empty-p () - "Return non-nil if the current entry is empty." + "Return non-nil if the current entry is empty. + +The search bound covers the whole subtree, so an entry whose answer +lives inside a child sub-heading is correctly reported as non-empty +\(upstream issue #13). `org-end-of-subtree' is used in place of +`outline-next-heading' because the latter lands on the first child +heading, which truncates the search range before the child's body." (save-excursion (org-back-to-heading t) (let ((lim (save-excursion - (outline-next-heading) (point)))) + (org-end-of-subtree t t) (point)))) (if (fboundp 'org-end-of-meta-data-and-drawers) (org-end-of-meta-data-and-drawers) ; function removed Feb 2015 (org-end-of-meta-data t)) diff --git a/tests/test-org-drill-entry-empty-p.el b/tests/test-org-drill-entry-empty-p.el new file mode 100644 index 0000000..047c36f --- /dev/null +++ b/tests/test-org-drill-entry-empty-p.el @@ -0,0 +1,135 @@ +;;; test-org-drill-entry-empty-p.el --- Tests for org-drill-entry-empty-p -*- lexical-binding: t; -*- + +;;; Commentary: +;; Regression tests for `org-drill-entry-empty-p'. +;; +;; Upstream issue #13 (kqr, 2019-07-22) reported that drill entries +;; whose body lives inside a child sub-heading were being skipped as +;; empty. Example: a question stored in the heading text, with the +;; answer inside `** The Answer'. +;; +;; Root cause: the function used `(outline-next-heading)' to compute +;; the search-end bound. `outline-next-heading' lands on the next +;; heading at any level, including a child — so the search range was +;; metadata-end up to the child heading's start, which excluded the +;; child's body text. The function returned t (empty) on entries +;; that clearly had content. +;; +;; Fix: use `org-end-of-subtree' for the bound. That covers the +;; whole subtree (including children) and degrades gracefully at the +;; last heading in the buffer (where `outline-forward-same-level' +;; would have errored). + +;;; Code: + +(require 'ert) +(require 'org) +(require 'org-drill) + +;;;; Helpers + +(defmacro with-org-fixture (content &rest body) + "Run BODY in a temp org-mode buffer containing CONTENT, point at start." + (declare (indent 1)) + `(with-temp-buffer + (let ((org-startup-folded nil)) + (insert ,content) + (org-mode) + (goto-char (point-min)) + ,@body))) + +;;;; Normal cases + +(ert-deftest test-org-drill-entry-empty-p-normal-empty-entry-returns-t () + "An entry with only metadata and no body is empty." + (with-org-fixture "* Question :drill: +:PROPERTIES: +:ID: abc +:END: +" + (should (org-drill-entry-empty-p)))) + +(ert-deftest test-org-drill-entry-empty-p-normal-direct-body-returns-nil () + "An entry with body text directly under the heading is not empty." + (with-org-fixture "* Question :drill: +:PROPERTIES: +:ID: abc +:END: + +The answer lives right here. +" + (should-not (org-drill-entry-empty-p)))) + +;;;; Boundary / regression — issue #13 + +(ert-deftest test-org-drill-entry-empty-p-regression-body-in-child-not-empty () + "Issue #13: an entry with its answer inside a child sub-heading is not empty. + +This is the exact shape kqr reported. The question lives in the +heading text and the answer lives inside `** The Answer'. Pre-fix, +this returned t because `outline-next-heading' set the search bound +to the child's start, excluding the child's body." + (with-org-fixture "* Entry question? :drill: +SCHEDULED: <2019-04-05 Fri> +:PROPERTIES: +:ID: def +:END: + +** The Answer + +Some text +" + (should-not (org-drill-entry-empty-p)))) + +(ert-deftest test-org-drill-entry-empty-p-boundary-last-entry-in-buffer () + "An entry that's the last in the buffer is handled without error. + +`outline-forward-same-level' would error at the last heading; the fix +uses `org-end-of-subtree' which handles end-of-buffer gracefully." + (with-org-fixture "* Final question :drill: +:PROPERTIES: +:ID: jkl +:END: + +The answer. +" + (should-not (org-drill-entry-empty-p)))) + +(ert-deftest test-org-drill-entry-empty-p-boundary-followed-by-sibling () + "An empty entry followed by a non-empty sibling stays empty. + +Sanity check that the wider search bound doesn't bleed into siblings." + (with-org-fixture "* First :drill: +:PROPERTIES: +:ID: mno +:END: + +* Second :drill: +:PROPERTIES: +:ID: pqr +:END: + +This one has body. +" + (should (org-drill-entry-empty-p)))) + +;;;; Error cases + +(ert-deftest test-org-drill-entry-empty-p-normal-deeply-nested-content () + "Content several levels deep under the entry is still found." + (with-org-fixture "* Question :drill: +:PROPERTIES: +:ID: stu +:END: + +** Section A +*** Subsection +**** Sub-subsection + +Deeply nested answer. +" + (should-not (org-drill-entry-empty-p)))) + +(provide 'test-org-drill-entry-empty-p) + +;;; test-org-drill-entry-empty-p.el ends here -- cgit v1.2.3