Re: [PATCH] Org Habit fix + new feature

[Ihor Radchenko <yantar92@gmail.com>]

Morgan Smith <Morgan.J.Smith@outlook.com> writes:

> ...  The current logic simply looks at the previous '(+
> org-habit-preceding-days org-habit-following-days)' log records which by
> default would be 28.  First of all, why are we looking into the future
> at all?  I don't think the habits graph currently supports looking at it
> from the perspective of a different day and I think marking things as
> complete in the future is pretty odd.

Well. Ideally, the habit graph should use current agenda day. Not the
actual current date.

> Second of all, if we use org
> wrong, then we will start loosing days at the beginning of the graph if
> we have more then 28 log records in our period.

Agree.

> Now my patch calculates the first day of the graph and simply looks at
> all log records before that date.  This is more robust if we want to use
> org wrong. Also it's more intuitive I think.  In many cases I think it
> will also be a performance boost since then we likely won't loop the
> full 28 times.  Furthermore, this method would support looking at the
> habits graph from the perspective of a different day (which blindly
> looping 28 times does not).
> This patch does not do a good job at adding support for repetitions.
> The graph and logic still works in days, not repetitions.  It simply
> makes the current code more robust.

Agree.

>> This logic will fail for non-default combinations of org-log-into-drawer
>> + org-clock-into-drawer + org-log-states-order-reversed.
>
> Well shoot.  That's a bummer.  So why are we using a regex here anyways?
> It feels not super robust.  Don't we have an AST we could use instead?
> Also even if we do want to use regex, pulling out log records and clock
> records seems like a pretty common thing to do that should be in a
> core library function right?

Because the last serious change in org-habit was 8 years ago (de51e1aef)
and the main implementation dates back to 13 years ago. org-element has
been introduced 10 years ago. It was simply not a thing when the
original org-habit had been developed.

Similar history goes over clock and log parsing. Nobody bothered to
update them for the new element API.

And notes format is only regexp-based. There is no fixed AST element for
notes. Just hard-coded conventions.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
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>