org-journal icon indicating copy to clipboard operation
org-journal copied to clipboard

Org 9.6 and carryover weird behaviour

Open mrvdb opened this issue 2 years ago • 20 comments

I've recently upgraded my org package to 9.6 from 9.5.5 and org-journal carryover functionality started to behave erratically. Reverting back org solves the issue, so my hunch was some changed function behaviour that needed adaptation in org-journal. So far I haven't been able to figure it out.

My minimal case which shows the problem:

  • install org 9.6 and org-journal (git head) with straight (no extra config)
  • relevant config for org-journal in use:
(setq org-journal-dir "/home/mrb/dat/org/journal/"
        org-agenda-file-regexp "^[0-9]+\\.org"
        org-journal-file-format  "%Y%m%d.org"
        org-journal-date-format "%A, %e-%m-%Y"
        org-journal-date-prefix "#+TITLE: "
        org-journal-time-format "[%R] "
        org-journal-time-prefix "* "
        org-journal-carryover-items "+carryover|+TODO=\"TODO\""
        org-journal-enable-agenda-integration t
        org-journal-carryover-delete-empty-journal 'always
        org-journal-enable-cache t))

Testfile 20221216.org (previous day):

#+TITLE: Friday, 16-12-2022
* Main                                                            :carryover:
** TODO Testing if this comes through.

With this config, I ran org-journal-new-entry

Contents of 20221217.org:

#+TITLE: Saturday, 17-12-2022
* Main                                                            :carryover:
** TODO Testing if this comes through.
* Main                                                            :carryover:
** TODO Testing if this comes through.

and an error message gets logged: org-journal-delete-old-carryover: Args out of range: #<buffer 20221216.org>, 29, 146"

Depending on the test file the carryover items which get in the new file is different. I have not been able to spot a clear pattern yet. It seems to double the main headline in any case with different items below it.

mrvdb avatar Dec 17 '22 15:12 mrvdb

How strange. Does it happen with the default org-journal-carryover-items as well, or is this specific to the carryover-tag matcher? (perhaps it now matches once for carryover, and once for TODO)

bastibe avatar Dec 19 '22 10:12 bastibe

It is indeed the org-journal-carryover-items value that causes problems. Using the default it seems to work as expected, that is, it carries over TODO items and leaves the rest behind.

mrvdb avatar Dec 19 '22 10:12 mrvdb

Forgot to ask, can you reproduce the error?

mrvdb avatar Dec 19 '22 10:12 mrvdb

I currently don't have much time to spare for open source development, and didn't try to reproduce the issue, sorry.

It is probably relatively easy to filter down the carryover results, such that we only ever copy each block once. If you'd like to try implementing this as a pull request, I'd be grateful for the help!

bastibe avatar Dec 19 '22 16:12 bastibe

Fair enough.

mrvdb avatar Dec 19 '22 16:12 mrvdb

I was unable to reproduce this error. I wrote this as an ert test case, which passes. I'm not seeing the duplicated entry. Have I missed anything?

(ert-deftest org-journal-carryover-args-out-of-range-test ()
  "Carrying over TODO items for daily files, with specified date format."

  (org-journal-test-macro
   (let (
         (org-journal-file-type 'daily)
         (org-journal-date-prefix "#+TITLE: ")
         (org-journal-time-prefix "* ")
         (org-journal-file-format  "%Y%m%d.org")
         (org-journal-date-format "%A, %e-%m-%Y")
         (org-journal-time-format "[%R] ")
         (org-journal-carryover-items "+carryover|+TODO=\"TODO\"")
         (org-journal-carryover-delete-empty-journal 'always)
         (org-journal-enable-cache t)
         )

     (copy-directory
      (directory-file-name "tests/journals/daily/carryover2")
      (file-name-as-directory org-journal-dir-test) nil nil t)

     (org-journal-new-entry t)

     (message (buffer-string))

     (should
      (cl-search
       (format-time-string org-journal-date-format)
       (buffer-substring-no-properties (point-min) (point-max))
       )
      )
     )))

jmay avatar Dec 19 '22 21:12 jmay

@mrvdb I take back my "cannot reproduce" comment. Was using org 9.5.5. When I ensured that 9.6 was installed and being used by ERT, I'm getting several errors from the test suite, not just the carryover case. Not sure what is going on.

Command line to run the test suite:

emacs -q -batch -l ert --eval "(package-initialize)" -l org-journal.el -l tests/org-journal-test.el -f ert-run-tests-batch-and-exit

jmay avatar Dec 20 '22 00:12 jmay

ERT results:

3 unexpected results:
   FAILED  org-journal-carryover-delete-empty-journal-test
   FAILED  org-journal-carryover-items-test
   FAILED  org-journal-scheduled-weekly-test

The common error in my ert results, at least for the carryover related tests seems to be:

"Calling ‘org-fold-core-region’ with missing SPEC"

mrvdb avatar Dec 20 '22 09:12 mrvdb

It might be extremely useful to incorporate these ERT tests into the Github CI.

bastibe avatar Dec 20 '22 19:12 bastibe

Yes, confirming that I can reliably get those same test failures. This is definitely caused by new behavior in org 9.6, some missing values that are expected by functions in org-fold-core.el (if you're curious, poke around with injecting a drawer value into org-fold-core--specs). I'm trying to figure out a safe backward-compatible workaround until the core issue can be resolved.

jmay avatar Dec 22 '22 00:12 jmay

How can I help get this resolved?

mrvdb avatar Jan 11 '23 17:01 mrvdb

We'd need someone to contribute a pull request that fixes the issue. If you'd like to take a stab at that, we'd be happy to review and merge it!

bastibe avatar Jan 12 '23 08:01 bastibe

Some extra info:

git bisecting (orgmode repo) with my journal files showed

3c4290e668b15c64e6d48b1926291987742476e8 is the first bad commit
commit 3c4290e668b15c64e6d48b1926291987742476e8
Author: Ihor Radchenko <[email protected]>
Date:   Sun Oct 17 00:00:01 2021 +0800

    org.el/org-scan-tags: Make use of fast `org-element-cache-map'

 lisp/org.el | 304 ++++++++++++++++++++++++++++++++++++++----------------------
 1 file changed, 195 insertions(+), 109 deletions(-)

Using that info, a workaround for the issue is to set org-element-use-cache to nil (temporarily).

mrvdb avatar Jan 15 '23 10:01 mrvdb

Great work! Thank you for figuring this out!

Would you like to contribute the workaround to the README? Probably somewhere near the top, so as to warn users of this issue in Org 9.6.

bastibe avatar Jan 16 '23 07:01 bastibe

[Bastian Bechtold]:

Would you like to contribute the workaround to the README? Probably somewhere near the top, so as to warn users of this issue in Org 9.6.

That's probably a good idea, I'm also checking if it is an option to wrap the 'carryover'; disabling the cache or invalidating some parts of it.

I can imagine the carryover should be a cache-invalidating operation, but I am not familiar enough with the org code to say for sure.

mrvdb avatar Jan 16 '23 07:01 mrvdb

We could maybe call org-element-cache-reset after the carryover (possibly on both the old and the new file). It might also be possible to call org-element-cache-refresh on just the changed areas, but that's probably more fragile.

bastibe avatar Jan 16 '23 09:01 bastibe

thanks for updating the readme, saved me a couple hours of debugging, and assuring my self I was sure it worked before.

Andsbf avatar Sep 04 '23 20:09 Andsbf

Is this issue still present with org 9.7?

pglpm avatar Jun 02 '24 06:06 pglpm

Since disabling the cache it has been a workable situation for me. Sporadically I get a repeat of the #TITLE line from the previous day (once a month or so), but could live with that workaround.

I have upgraded to Org mode version 9.8-pre (release_9.7.2-13-gc426f4) and re-enabled the cache (setq org-element-use-cache t) So far so good.

mrvdb avatar Jun 04 '24 11:06 mrvdb

I'm considering this issue resolved from my point of view. ERT tests all succeed, I can't reproduce the problems anymore using org 9.7 and later and the cache setting is not needed anymore from what I can see.

mrvdb avatar Jun 09 '24 17:06 mrvdb