Morgan Smith <morgan.j.sm...@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>