-
-
Notifications
You must be signed in to change notification settings - Fork 139
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(core): support org-enforce-todo-dependencies #741
base: master
Are you sure you want to change the base?
feat(core): support org-enforce-todo-dependencies #741
Conversation
Many thanks for the contribution. 👍 Old issues don't mean, there is no interest in a working implementation. It just means, that we need more active contributors. 😄 From a very quick overlook your code looks good - I will try it out as soon as I can.
I wasn't even aware of this, but Orgmode is soo big ... you always learn something new ... 😅 |
add warning for blocked tasks
08c4490
to
7413717
Compare
Haha, glad to hear that! I am migrating my org from emacs to nvim, and will try to implement some daily functions into nvim-orgmode. |
Hi there, I really dont want to be pushy. Thanks for your great work here. |
@PannenetsF I finally found the time to play around with your contribution. First I was not sure if I misunderstood how the feature is supposed to work, but what was immediately noticeable is that changing the TODO state of a parent headline is completely blocked. And it is actually blocked on pressing "t", so if I want to set it to "Progress" and press "tp" I get a warning and it pastes my current register. On the other hand it does not change the state of the parent headline, when I set a child into "Progress", so I am not able to fix inconsistent states. I then actually took the time to see, how the feature behaves in Emacs. There you can change the state of the parent headline as long as it is not a state considered as "closed" (a state defined on the right hand side of the "|" in the configuration list). To get this merged, we should mimic the behavior of Emacs org mode. |
|
||
local M = {} | ||
-- @param headline OrgApiHeadline | ||
-- local M.vis_head = function (headline, indent) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove dead code.
function OrgMappings:_has_unfinished_children(headline) | ||
for _, h in ipairs(headline:get_child_headlines()) do | ||
local was_done = h:is_done() | ||
if not was_done then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this local variable? It is not used later on, isn't it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if not was_done then | |
if not h:is_done() then | |
return true | |
end |
if not was_done then | ||
return true | ||
end | ||
if OrgMappings:_has_unfinished_children(h) then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I stumbled over this recursion and after some moments I recognized, that not every headline is a todo. I think this is worth a comment.
return false | ||
end | ||
|
||
describe('Todo mappings unfer force dependency', function() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo?
}, vim.api.nvim_buf_get_lines(0, 2, 4, false)) | ||
end) | ||
|
||
it('should change todo state of a headline backward (org_todo_prev)', function() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate test name? Can you reflect the difference to the former test in the name?
@@ -0,0 +1,329 @@ | |||
local config = require('orgmode.config') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I appreciate, that you wrote tests. I saw that some of them are actually testing multiple things. I'm my experience this can make a regression harder to track, although it's much better than having no test. Would you mind to split the tests up a little bit?
Hi Seflue, thanks for your review. I would read the original Emacs orgmode for better mimic behavior. I am kind of busy these weeks. Will try to fetch time to do that |
Hi guys, I just impl the option for
org-enforce-todo-dependencies
, which will prevent parent TODOs from toggling when it has some unfinished children.I am not sure if you are still interested in it, as the related issue #274 was updated 2 years ago.
Here comes the reference.
https://orgmode.org/manual/TODO-dependencies.html
:)