Ihor Radchenko <[email protected]> writes:
> Morgan Smith <[email protected]> writes:
>
>> Ihor Radchenko <[email protected]> writes:
>>>> I've attached a patch that fixes the issue. Let me know what you think.
>>>> ...
>>>> + (let ((todo (org-element-at-point (or epom
>>>> (org-entry-beginning-position)))))
>>>
>>> This defeats the point of using element API. You can just use
>>> org-element-lineage.
>>
>> Does the regex search near the end of org-habit-parse-todo not also
>> defeat the point of using element API? I'm not sure exactly what you
>> mean by "defeats the point".
>
> Hmm. I guess I mixed what I think and the original description of the
> patch. In the long term, I envision that we should use org-element API
> instead of moving point around or regexp matching.
> So, org-entry-beginning-position would also work fine.
> Although, it would still boil down to org-element-lineage(-map), via the
> call chain.
I just looked at the definition of `org-back-to-heading' (called by
`org-entry-beginning-position') for the first time and it seems to be
exactly what I wrote using `org-element-lineage' except it handles the
case of being at the end of an inline task so it is superior. So I will
go back to using `org-entry-beginning-position'. It is unfortunate that
it returns the point instead of the element but that seems minor.
If you still have icky feelings about me touching the point, feel free
to write (or delegate the writing of) a helper function that returns the
element of the local heading. I think such a function would indeed be
quite useful and well loved.
I do 100% agree with your long term vision. Unfortunately, I think the
best way to get rid of the regex is the same way I did it in
org-clock-sum. Which then got reverted due to parser problems.
I was forced to add this to my config after the org-clock-sum changes
got reverted. Yes this is very off topic but I will continue to be sad
about these very hard to fix problems in our parser. I'm not blaming
anyone, I'm just being sad.
#+begin_src elisp
;; Prevent hang when clocking in
(add-function :around (symbol-function 'org-clock-in)
(lambda (fun &rest args)
(cl-letf (((symbol-function #'org-clock-sum-current-item)
(lambda (&rest _) 0)))
(apply fun args))))
#+end_src
>> +(defun org-test-habit-agenda-string (repeater-type-string
>> + repeater-deadline?
>> + &optional
>> + headline-depth
>> + inlinetask-p)
>
> If we want to be strictly accurate, inlinetask-p + headline-depth
> combination may go wrong if org-inlinetask-min-level is less than
> headline-depth passed.
Fixed this by removing headline-depth completely. Unnecessary complexity
>> + (let ((todo (if epom
>> + (org-element-at-point epom)
>> + (or (org-element-lineage (org-element-at-point) '(headline
>> inlinetask) t)
>> + (user-error "Before first headline at position %d in
>> buffer %s"
>> + (point) (current-buffer))))))
>
> LGTM.
>
> I notice another issue with the applied patch.
>
> (limit (if reversed
> (org-element-contents-end todo)
> (org-element-contents-begin todo)))
>
> This is not an equivalent of the original
>
> (end (org-entry-end-position))
> ...
> (limit (if reversed end (point)))
>
> org-entry-end-position return the beginning of the first child or the
> beginning of next heading.
> org-element-contentss-end will go pass all the children and return the
> end of the whole subtree.
Oh dear! I switched out a function for a similar sounding one and
didn't realize they actually did different things.
I came up with this solution but it doesn't seem too elegant to me
#+begin_src elisp
(or (save-excursion (and (org-goto-first-child todo) (point)))
(org-element-contents-end todo))
#+end_src
`org-goto-first-child' here feels very similar to what we where just
talking about with `org-entry-beginning-position'. It utilizes the
org-element API internally but then it moves the point instead of
returning an element.
I think maybe we need more helper functions that embody your vision
>From 290ab2212aa912d8322d31c4095d246255a78a2d Mon Sep 17 00:00:00 2001
From: Morgan Smith <[email protected]>
Date: Wed, 15 Jul 2026 10:49:19 -0400
Subject: [PATCH 1/2] Testing: org-habit: Run tests on inlinetasks as well
* testing/lisp/test-org-habit.el
(org-test-habit-agenda-string): Add functionality to generate inline
tasks.
(org-test-habit): Also run tests on inlinetasks.
---
testing/lisp/test-org-habit.el | 24 +++++++++++++++++++-----
1 file changed, 19 insertions(+), 5 deletions(-)
diff --git a/testing/lisp/test-org-habit.el b/testing/lisp/test-org-habit.el
index 20f9a4522..790877154 100644
--- a/testing/lisp/test-org-habit.el
+++ b/testing/lisp/test-org-habit.el
@@ -22,6 +22,7 @@
(require 'org-test "../testing/org-test")
(require 'org-agenda)
(require 'org-habit)
+(require 'org-inlinetask)
(eval-when-compile
(require 'test-org-agenda "../testing/lisp/test-org-agenda"))
(eval-when-compile
@@ -39,12 +40,18 @@ org-test-habit-no-fluff-agenda
(org-agenda-todo-keyword-format "")
(org-agenda-prefix-format "")))))
-(defun org-test-habit-agenda-string (repeater-type-string repeater-deadline?)
+(defun org-test-habit-agenda-string (repeater-type-string
+ repeater-deadline?
+ &optional
+ inlinetask-p)
"Return an org habit test string.
REPEATER-TYPE-STRING is used as the repeater type (ex. \".+\").
When REPEATER-DEADLINE? is non-nil, add a repeater deadline.
Order is determined by `org-log-states-order-reversed'."
(concat
+ (if inlinetask-p
+ (make-string (1- org-inlinetask-min-level) ?*)
+ "")
"* TODO Shave
SCHEDULED: <2009-10-17 Sat " repeater-type-string "2d"
(if repeater-deadline?
@@ -80,7 +87,11 @@ org-test-habit-agenda-string
- CLOSING NOTE [2009-10-10 Sat] \\
this style occurs when `org-log-done' is `note'.
- State \"DONE\" from \"TODO\" [2009-10-12 Mon]
-- State \"DONE\" from \"TODO\" [2009-10-15 Thu]")))
+- State \"DONE\" from \"TODO\" [2009-10-15 Thu]")
+ (if inlinetask-p
+ (concat
+ "\n" (make-string org-inlinetask-min-level ?*) " end")
+ "")))
(defmacro org-test-habit (&rest body)
"Run BODY multiple times for testing habits.
@@ -95,9 +106,12 @@ org-test-habit
(dolist (org-log-states-order-reversed '(t nil))
(dolist (repeater-deadline? '(nil t))
(dolist (repeater-type-string '(".+" "+" "++"))
- (org-test-agenda-with-agenda
- (org-test-habit-agenda-string repeater-type-string repeater-deadline?)
- ,@body))))))
+ (dolist (inlinetask-p '(nil t))
+ (org-test-agenda-with-agenda
+ (org-test-habit-agenda-string repeater-type-string
+ repeater-deadline?
+ inlinetask-p)
+ ,@body)))))))
(ert-deftest test-org-habit/simple-habit ()
"Test the agenda view for a simple habit."
base-commit: d27f01a19901a6fb08a676ffaafe75ce7c0b400b
--
2.54.0
>From 7e5324fcea5e51494b91c24b2af8d8c43180718d Mon Sep 17 00:00:00 2001
From: Morgan Smith <[email protected]>
Date: Wed, 15 Jul 2026 10:55:11 -0400
Subject: [PATCH 2/2] org-habit: Fix recent regressions
The commit e55fdd953 introduced a few regressions.
If point was not at a heading then the function would fail.
If the habit had subheadings, those would be searched for state
changes as well.
* lisp/org-habit.el (org-habit-parse-todo): Don't assume point is at a
heading and then check the element type for good measure.
Properly limit the search region to the heading contents ignoring
subheadings.
---
lisp/org-habit.el | 22 +++++++++++++---------
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/lisp/org-habit.el b/lisp/org-habit.el
index bf52e50e8..d88650f5f 100644
--- a/lisp/org-habit.el
+++ b/lisp/org-habit.el
@@ -197,7 +197,8 @@ org-habit-parse-todo
5: Repeater type as a string
This list represents a \"habit\" for the rest of this module."
- (let ((todo (org-element-at-point epom)))
+ (let ((todo (org-element-at-point (or epom (org-entry-beginning-position)))))
+ (cl-assert (org-element-type-p todo '(headline inlinetask)))
(cl-assert (org-is-habit-p todo))
(let* ((habit-entry (org-no-properties (org-element-property :title todo)))
(scheduled
@@ -229,12 +230,15 @@ org-habit-parse-todo
(let* ((maxdays (+ org-habit-preceding-days org-habit-following-days))
(reversed org-log-states-order-reversed)
(search (if reversed 're-search-forward 're-search-backward))
- (start (if reversed
- (org-element-contents-begin todo)
- (org-element-contents-end todo)))
- (limit (if reversed
- (org-element-contents-end todo)
- (org-element-contents-begin todo)))
+ (region-start (org-element-contents-begin todo))
+ (region-end (or (save-excursion (and (org-goto-first-child todo) (point)))
+ (org-element-contents-end todo)))
+ (search-start (if reversed
+ region-start
+ region-end))
+ (search-limit (if reversed
+ region-end
+ region-start))
(count 0)
(re (format
"^[ \t]*-[ \t]+\\(?:State \"%s\".*%s%s\\)"
@@ -254,8 +258,8 @@ org-habit-parse-todo
("%u" . ".*?")
("%U" . ".*?")))))))))
(save-excursion
- (goto-char start)
- (while (and (< count maxdays) (funcall search re limit t))
+ (goto-char search-start)
+ (while (and (< count maxdays) (funcall search re search-limit t))
(push (time-to-days
(org-time-string-to-time
(or (match-string-no-properties 1)
--
2.54.0
