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.
> +(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.
> + (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.
--
Ihor Radchenko // yantar92,
Org mode maintainer,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>
