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 updating a headline's cookie from multiple lists #925

Merged
merged 2 commits into from
Mar 12, 2025

Conversation

thuyen
Copy link
Contributor

@thuyen thuyen commented Mar 8, 2025

Summary

On master if a headline has multiple lists, the cookie is only updated for the current list. In emacs org, the cookie reflects information about all lists under the headline. This PR just makes it so.

Changes

Screenshot from 2025-03-08 06-55-34

On master the headline would show either 2/3 or 1/1 depending where the last update happens.

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.

@seflue
Copy link
Contributor

seflue commented Mar 9, 2025

While you're on it, have you considered to also implement #305? I am really missing that feature and already thought about implementing it myself, but never found the time to wrap my head around the cookie code. Looking at your code, it seems, that it has gotten simpler over the last two years due to some refactorings, so it might be worth a shot. 🙂

@thuyen
Copy link
Contributor Author

thuyen commented Mar 9, 2025

While you're on it, have you considered to also implement #305? I am really missing that feature and already thought about implementing it myself, but never found the time to wrap my head around the cookie code. Looking at your code, it seems, that it has gotten simpler over the last two years due to some refactorings, so it might be worth a shot. 🙂

Let's see if #926 works.

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.

Thanks!

@kristijanhusak kristijanhusak merged commit 22094c4 into nvim-orgmode:master Mar 12, 2025
17 checks passed
@thuyen thuyen deleted the multilist 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.

3 participants