[BUG]: Carryover not working due to date sorting

jaidetree commented 4 years ago

Describe the bug When creating a new journal entry, sometimes tasks from the last entry are not carried over. This appears to be caused by the sorting of the date entries.

To Reproduce

Create an entry in the past in my case 20200618.org
Add a TODO heading with a timestamp under it
Create a new entry in the future. In my case 20200622.org

Expected behavior The TODO entry in 20200618.org is carried over to 20200622.org

Desktop

OS: OS X 10.15.5 (Catalina)
Emacs Version 27

Your Emacs Configuration

(after! org
  (setq
    org-directory                         "~/Dropbox/org"
    diary-file                            (concat org-directory "/diary")
    org-agenda-include-diary              nil
    org-agenda-file-regexp                "\\`[^.].*\\.org'\\|[0-9]+\\.org$"
    org-agenda-timegrid-use-ampm          t
    org-journal-dir                       (concat org-directory "/journal")
    org-journal-enable-agenda-integration nil
    org-journal-file-format               "%Y%m%d.org"
    org-journal-time-format               "%-l:%M%#p"
    org-journal-carryover-items           "TODO=\"TODO\"|TODO=\"STRT\"|TODO=\"HOLD\"")
  (appendq! org-agenda-files (list org-journal-dir)))

Research https://github.com/bastibe/org-journal/blob/master/org-journal.el#L1001-L1009

(setq dates (cl-loop
                  for date in dates
                  while (calendar-date-compare (list date) (list calendar-date))
                  collect date into result and count t into cnt
                  finally return (if result
                                        ;; Front
                                        `(,@result ,calendar-date)
                                      ;; Somewhere enbetween or end of dates
                                      `(,calendar-date ,@result ,@(nthcdr cnt dates)))))

It seems to be sorting the dates incorrectly. I added advice to that function and logged what the collection algorithm above would return:

Output

dates 
((6 16 2020)
 (6 18 2020)
 (6 11 2020)
 (6 12 2020)
 (6 13 2020)
 (6 14 2020)
 (6 15 2020)
 (6 16 2020))

Solution Strategies

Create a separate function for selecting\listing the dates, even if it uses the same sorting algorithm, it at least gives us options
Consider turning the dates back into ISO style strings like "20200616" since it can be sorted by string (at least in other languages using that format has worked for me)
If we at least create a separate function for collecting the dates, I can use the above in my own config

jaidetree commented 4 years ago

Ack! Forgot to mention I'm down for creating a PR to split out that date collection function as proposed. I just want the greenlight before I get started :smile:

casch-at commented 4 years ago

I PR would be highly appreciated. I thought already about re-factoring that function. Somehow this function/snippet looks ugly too :-), beside having obviously bugs.

Thanks!

casch-at commented 4 years ago

I moved this issue to the milestone 2.2.0, as the actual bug has been fixed with 75e241e.

bastibe / org-journal

[BUG]: Carryover not working due to date sorting #264