Hi Morgan!
Actually, I realized I was using the LOGGING property wrong. From the documentation I got the impression that you had to use TODO(!) and TODO(@) when specifying the behavior for the TODO -> DONE transition, but that's wrong, you should use DONE(!) and DONE(@). That's not because the documentation is misleading, just that using the TODO keyword is what I tried first and it worked for me (it says moved to TODO from DONE instead of moved to DONE from TODO, but I never realized since logging the timestamp and the note was all I cared about). If you use DONE(!) and DONE(@), everthing works as expected. There is still a bug when you use TODO(!) and TODO(@), but I guess is minor and more complex to reason about. Should a repeated task with both TODO(!) and DONE(!) log both state changes for example? Currently it doesn't. But between that and the fact that I already know how to do what I actually wanted, I'm no longer interested in fixing the bug. If you also agree it's a minor thing, feel free to close the bug on my side. Thanks for your time! Morgan Smith <[email protected]> writes: > Ignacio Casso <[email protected]> writes: > >> I'm not sure how it should be fixed, the code looks too spaguetti >> already and the fixes I can think of would only add up to that. If you >> don't have a problem with that I can submit a patch with the simplest >> fix I can think of, but I think this code would benefit from a >> refactor from someone who really understands it. > > We're always looking for help so if you're going to offer we're going to > accept. > > That does indeed look like some complicated code that I am not familiar > with. I am however familiar with the tests for this so I whipped up > some test cases for you to use and attached them. > > As you can see, the test case with > "(let ((org-todo-keywords '((sequence "TODO(!)" "DONE"))))" > passes but the one with the property ":LOGGING: TODO(!)" fails. There > is definitely a bug here. > > Confirmed. > > If you're using the build system make sure you pull in the latest > changes first. I just pushed a fix so that tests pass when setting > "BTEST_RE='test-org/org-todo-prefix'". > > No worries if you give up attempting this yourself. Just report back > here. I know this stuff can get pretty frustrating. > > Good luck! > > [2. text/x-patch; > 0001-Testing-Add-more-tests-in-test-org-org-todo-prefix.patch]...
