Skip to content
New issue

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

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

Already on GitHub? # to your account

feat: support headline's cookie from TODOs #926

Merged
merged 6 commits into from
Mar 16, 2025

Conversation

thuyen
Copy link
Contributor

@thuyen thuyen commented Mar 9, 2025

Summary

Add function update_todo_cookie to update a headline's cookie from a TODO state change.

Related Issues

Closes #305

Changes

Screenshot from 2025-03-15 21-25-54

Checklist

I confirm that I have:

  • Followed the
    Conventional Commits
    specification
    (e.g., feat: add new feature, fix: correct bug,
    docs: update documentation).
  • My PR title also follows the conventional commits specification.
  • Updated relevant documentation, if necessary.
  • Thoroughly tested my changes.
  • Added tests (if applicable) and verified existing tests pass with
    make test.
  • Checked for breaking changes and documented them, if any.

@thuyen thuyen changed the title feat: update headline cookies for TODOs feat: update cookies for TODOs Mar 9, 2025
@thuyen thuyen changed the title feat: update cookies for TODOs feat: support counting children TODOs in a headline's cookie Mar 9, 2025
@thuyen thuyen force-pushed the todo_cookie branch 4 times, most recently from 5885d42 to 8cd451f Compare March 10, 2025 00:03
@kristijanhusak
Copy link
Member

Lets rebase and then I'll take a look. Thanks!

@thuyen
Copy link
Contributor Author

thuyen commented Mar 12, 2025

Lets rebase and then I'll take a look. Thanks!

Done!

@kristijanhusak
Copy link
Member

Generally looks good, and works as expected as long as only sub-todos are used.
The only issue is when both checkboxes and sub-todos are used. For example, having this content:

* Test [/]
- [ ] foo
- [ ] bar
** TODO Test 1
** TODO Test 2
** TODO Test 3

Toggling either sub-todo or a checkbox, ends up adding [1/5] to the top level headline.
In emacs, it seems that checkbox list has a priority over sub-todos.
For example, given the content above, if you change the sub-todo to DONE, it will update cookie to [1/3], but if you recalculate the cookie value with C-c #, it will actually take checkboxes state and update the cookie back to [0/2].
If you end up having one checkbox checked and cookie value of [1/2], switching a sub-todo to done will again update cookie to [1/3]. I'm assuming Emacs orgmode is not handling this conflict since it's highly unlikely that anyone is using it like this.
Nontheless, I'd like to follow the same logic. If there are checkboxes in the headline body, prioritize that over sub-tasks.

@thuyen thuyen changed the title feat: support counting children TODOs in a headline's cookie feat: support headline's cookie from TODOs Mar 16, 2025
@thuyen
Copy link
Contributor Author

thuyen commented Mar 16, 2025

Generally looks good, and works as expected as long as only sub-todos are used. The only issue is when both checkboxes and sub-todos are used. For example, having this content:

* Test [/]
- [ ] foo
- [ ] bar
** TODO Test 1
** TODO Test 2
** TODO Test 3

Toggling either sub-todo or a checkbox, ends up adding [1/5] to the top level headline. In emacs, it seems that checkbox list has a priority over sub-todos. For example, given the content above, if you change the sub-todo to DONE, it will update cookie to [1/3], but if you recalculate the cookie value with C-c #, it will actually take checkboxes state and update the cookie back to [0/2]. If you end up having one checkbox checked and cookie value of [1/2], switching a sub-todo to done will again update cookie to [1/3]. I'm assuming Emacs orgmode is not handling this conflict since it's highly unlikely that anyone is using it like this. Nontheless, I'd like to follow the same logic. If there are checkboxes in the headline body, prioritize that over sub-tasks.

I reverted update_cookie to just handle checked boxes; and added update_todo_cookie to handle TODO state change separately. At least for the example above, it works as you would expect.

Copy link
Member

@kristijanhusak kristijanhusak left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@kristijanhusak kristijanhusak merged commit abf8890 into nvim-orgmode:master Mar 16, 2025
17 checks passed
@thuyen thuyen deleted the todo_cookie branch March 16, 2025 15:35
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support fo [/] and [%] cookies for TODOs and DONEs
2 participants