Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Added the ability to create a closing entry when TODOS are marked done as per logdone #984

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

vHugoObject
Copy link

@vHugoObject vHugoObject commented Jul 13, 2024

In reference to issue #980. There is now an option to create a closing entry when TODOS are marked done. This is disabled by default. This can be enabled in the Organice settings, for a specific file using "#+STARTUP: logdone" or for a specific header by adding ":LOGGING: logdone" as a drawer property. The default behavior when enabled is to add the closing entry to the header. However, this setting will add the closing entry to your logbook drawer if you have org-log-into-drawer enabled in the Organice settings as well. The documentation has been updated to reflect this new addition.

Fixes #980

- Added logDone to the advanceTodoState action
- Revised the integration tests I added for advanceTodoState
- Added new unit tests for advanceTodoState
- Added LogDone, which uses the newly added functions addLogNote or addLogBookEntry to add a closing entry.
to use org-log-into-drawer setting with logdone
@vHugoObject vHugoObject changed the title Added the ability to create a closing entry when TODOS are marked done as per logdone feat: Added the ability to create a closing entry when TODOS are marked done as per logdone Aug 22, 2024
@vHugoObject
Copy link
Author

vHugoObject commented Sep 12, 2024

Can someone review? @schoettl @munen

Copy link
Collaborator

@schoettl schoettl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work! Nice that you also added many tests.

Unfortunately I wasn't able yet to run and test it locally. Since I last ran yarn install/start I've switched OS and now I get a lot of errors with node dependencies.

Maybe @munen has time to run a quick test?

Also, maybe we should upgrade to a newer node version...

@vHugoObject
Copy link
Author

@schoettl Did you use nvm?

@schoettl
Copy link
Collaborator

@schoettl Did you use nvm?

I tried but couldn't even install it on NixOS :/ I'll try again later this week.

@schoettl
Copy link
Collaborator

Hi @vHugoObject, I got my testing environment right and started playing. I like this feature, good work!

For reference:

I found a few bugs that you might want to fix:

  1. The CLOSED: […] word should be in the same line as SCHEDULED: or DEADLINE:
  2. The CLOSED: should be removed when the header's TODO state is changed to something other than a DONE state.
  3. If CLOSED: exists, another CLOSED: is added when I close a header again. The timestamp should be updated instead.
  4. The CLOSED: is currently placed below a :PROPERTIES: drawer (see screenshot below). It should be above it, right below the headline.
  5. The settings either :LOGGING: in a drawer or #+STARTUP: should be case-insensitive. E.g. :logging: logdone should have the same effect.

I recommend that you look into how SCHEDULED and DEADLINE are implemented, it should be pretty similar regarding parsing, output, and storing.

grafik

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Closing TODOs does not log as per logdone
2 participants