Re: bug in "simple dependencies" handling (?)

From: Carsten Dominik <carsten.dominik@gmail.com>
To: Daniel <dh@obyz.de>
Cc: emacs-orgmode@gnu.org
Subject: Re: bug in "simple dependencies" handling (?)
Date: Fri, 3 Apr 2009 18:58:29 +0200	[thread overview]
Message-ID: <77533135-0A74-4654-814A-38D75715D5DE@gmail.com> (raw)
In-Reply-To: <49D3C858.7040301@obyz.de>

[-- Attachment #1.1: Type: text/plain, Size: 12891 bytes --]

Hi Daniel,

thanks for your continuing efforts.  I have now implemented
this change, with some speed optimizations.

I was worried that this could get slow, and the main occasion
would be if you have an entry in a very long chain of
siblings that have to be done ordered.  I believe that I
have made this a lot faster now.

Thanks!

- Carsten

On Apr 1, 2009, at 10:02 PM, Daniel wrote:

> Hello Carsten,
>
> you're right. I've changed the code suitable (at least I hope I  
> have ;).
>
> Do you think it's possible to replace the original hook with my
> altered one, or do you think the additional checks are to expensive?
> As far as I see there's no noticeable difference.
>
> Nevertheless, here's the code in case someone is interested:
> (altered lines are marked with semicolons)
>
>
> (defun org-block-todo-from-children-or-siblings (change-plist)
> "Block turning an entry into a TODO, using the hierarchy.
> This checks whether the current task should be blocked from state
> changes.  Such blocking occurs when:
>
> 1. The task has children which are not all in a completed state.
>
> 2. A task has a parent with the property :ORDERED:, and there
>    are siblings prior to the current task with incomplete
>    status."
> (catch 'dont-block
>   ;; If this is not a todo state change, or if this entry is already  
> DONE,
>   ;; do not block
>   (when (or (not (eq (plist-get change-plist :type) 'todo-state- 
> change))
>         (member (plist-get change-plist :from)
>             (cons 'done org-done-keywords))
>         (member (plist-get change-plist :to)
>             (cons 'todo org-not-done-keywords)))
>     (throw 'dont-block t))
>   ;; If this task has children, and any are undone, it's blocked
>   (save-excursion
>     (org-back-to-heading t)
>     (let ((this-level (funcall outline-level)))
>   (outline-next-heading)
>   (let ((child-level (funcall outline-level)))
>     (while (and (not (eobp))
>             (> child-level this-level))
>       ;; this todo has children, check whether they are all
>       ;; completed
>       (if (and (not (org-entry-is-done-p))
>            (org-entry-is-todo-p))
>       (throw 'dont-block nil))
>       (outline-next-heading)
>       (setq child-level (funcall outline-level))))))
>   ;; Otherwise, if the task's parent has the :ORDERED: property, and
>   ;; any previous siblings are undone, it's blocked
> ;;;;;;;;;;;;;;;;;;;;;;
> (setq while_cond t);;;
> ;;;;;;;;;;;;;;;;;;;;;;
> (save-excursion (save-match-data
>   ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
>   (ignore-errors (while while_cond;;;
>   ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
>   (save-excursion
>     (org-back-to-heading t)
>     (when (save-excursion (ignore-errors
>       (org-up-heading-all 1)
>       (org-entry-get (point) "ORDERED")))
>   (let* ((this-level (funcall outline-level))
>          (current-level this-level))
>     (while (and (not (bobp))
>             (>= current-level this-level))
>       (outline-previous-heading)
>       (setq current-level (funcall outline-level))
>       (if (= current-level this-level)
>       ;; This is a younger sibling, check if it is completed
>       (if (and (not (org-entry-is-done-p))
>            (org-entry-is-todo-p))
>           (throw 'dont-block nil)))))))
>   (org-back-to-heading t)
>   (org-up-heading-all 1)
>   ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
>   (when (not (org-entry-is-todo-p))
>     (setq while_cond nil))))))
>   ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
>   t))
>
>
> Best regards,
> Daniel
>
>
> Carsten Dominik wrote:
>> Hi Daniel,
>>
>> looks that your modification do the right thing - almost.
>> It does not check if the parent itself is a TODO, and I
>> think this would be necessary as well.  Because it would
>> allow to have do-able subtasks in the list without
>> too much blocking.
>>
>> So "write hopping list" would not be blocked in this case:
>>
>> * organize party
>> :PROPERTIES:
>> :ORDERED:  t
>> :END:
>> ** TODO send invitations
>> *** TODO send invitation to Paul
>> *** TODO send invitation to Nicole
>> *** ect.
>> ** Buy meals and drinks
>> :PROPERTIES:
>> :ORDERED:  t
>> :END:
>> *** TODO write shopping list
>> *** TODO get money  from my bank account
>> *** TODO buy food
>> *** TODO buy drinks
>>
>> Would you agree?
>>
>> - Carsten
>>
>> On Mar 28, 2009, at 3:52 AM, Daniel wrote:
>>
>>> Oh my god! I think I've found a good solution :)
>>>
>>> Can you please tell me, whether it's crap or not?
>>>
>>> Only 4 lines differ from the original
>>> org-block-todo-from-children-or-siblings hook. I've marked
>>> the lines with "comment lines", they are before and after
>>> the ORDERED-property check, at the end of the function.
>>>
>>>
>>> (defun org-block-todo-from-children-or-siblings-or-parent (change- 
>>> plist)
>>> "Block turning an entry into a TODO, using the hierarchy.
>>> This checks whether the current task should be blocked from state
>>> changes.  Such blocking occurs when:
>>>
>>> 1. The task has children which are not all in a completed state.
>>>
>>> 2. A task has a parent with the property :ORDERED:, and there
>>>   are siblings prior to the current task with incomplete
>>>   status."
>>> (catch 'dont-block
>>>  ;; If this is not a todo state change, or if this entry is  
>>> already DONE,
>>>  ;; do not block
>>>  (when (or (not (eq (plist-get change-plist :type) 'todo-state- 
>>> change))
>>>        (member (plist-get change-plist :from)
>>>            (cons 'done org-done-keywords))
>>>        (member (plist-get change-plist :to)
>>>            (cons 'todo org-not-done-keywords)))
>>>    (throw 'dont-block t))
>>>  ;; If this task has children, and any are undone, it's blocked
>>>  (save-excursion
>>>    (org-back-to-heading t)
>>>    (let ((this-level (funcall outline-level)))
>>>  (outline-next-heading)
>>>  (let ((child-level (funcall outline-level)))
>>>    (while (and (not (eobp))
>>>            (> child-level this-level))
>>>      ;; this todo has children, check whether they are all
>>>      ;; completed
>>>      (if (and (not (org-entry-is-done-p))
>>>           (org-entry-is-todo-p))
>>>      (throw 'dont-block nil))
>>>      (outline-next-heading)
>>>      (setq child-level (funcall outline-level))))))
>>>  ;; Otherwise, if the task's parent has the :ORDERED: property, and
>>>  ;; any previous siblings are undone, it's blocked
>>> ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
>>> (save-excursion (save-match-data
>>>  (ignore-errors (while t
>>> ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
>>>  (save-excursion
>>>    (org-back-to-heading t)
>>>    (when (save-excursion
>>>        (ignore-errors
>>>      (org-up-heading-all 1)
>>>      (org-entry-get (point) "ORDERED")))
>>>  (let* ((this-level (funcall outline-level))
>>>         (current-level this-level))
>>>    (while (and (not (bobp))
>>>            (>= current-level this-level))
>>>      (outline-previous-heading)
>>>      (setq current-level (funcall outline-level))
>>>      (if (= current-level this-level)
>>>      ;; This is a younger sibling, check if it is completed
>>>      (if (and (not (org-entry-is-done-p))
>>>           (org-entry-is-todo-p))
>>>          (throw 'dont-block nil)))))))
>>>  ;;;;;;;;;;;;;;;;;;;;;;;;;;
>>>  (org-back-to-heading t)
>>>  (org-up-heading-all 1)))))
>>>  ;;;;;;;;;;;;;;;;;;;;;;;;;;
>>>  t))
>>>
>>> (add-hook 'org-blocker-hook 'org-block-todo-from-children-or- 
>>> siblings-or-parent)
>>>
>>>
>>>
>>> Carsten Dominik wrote:
>>>> Hi Daniel,
>>>>
>>>> one problem might have been a bug I just fixed.
>>>>
>>>> Another problem is that the way you wrote your code, a child
>>>> on an ordered sequence will block the parent, and the parent
>>>> will block the child.
>>>>
>>>> I'd like to come around an fix this, just not clear yet how, and
>>>> how to do it efficiently.
>>>>
>>>> - Carsten
>>>>
>>>>
>>>> On Mar 27, 2009, at 1:24 AM, Daniel wrote:
>>>>
>>>>> Hello Carsten,
>>>>>
>>>>> thanks for your reply.
>>>>>
>>>>>> Hi Daniel,
>>>>>>
>>>>>> yes, this could be seen as a bug.  However, the implementation
>>>>>> does not work by scanning the entire buffer and marking tasks
>>>>>> that should be blocked.  Rather, it goes to each task and then
>>>>>> scans around to see locally what the dependencies are.
>>>>>>
>>>>>> In this case it looks only at the parent, not at the
>>>>>> grand parent.
>>>>> Wouldn't it be enough to check whether the parent is blocked.
>>>>> Wouldn't that generate a blocking-chain?
>>>>>
>>>>>
>>>>>> However, the todo
>>>>>> dependencies are simple hook functions, and an interested
>>>>>> programmer could relatively easily extend them, I believe.
>>>>> I've tried to write a custom org-blocker-hook but it doesn't work,
>>>>> unfortunately.
>>>>>
>>>>> Can you (or someone else) tell me please what's wrong with my  
>>>>> code?
>>>>>
>>>>> (defun org-block-todo-with-blocked-parent (change-plist)
>>>>> ;; check whether we are in a endless loop:
>>>>> (if (plist-get change-plist :org-block-todo-with-blocked-parent)
>>>>> ;; We are in a endless loop: don't block (return t)
>>>>> t
>>>>> ;; We are not in a endless loop: go to the parent heading
>>>>> (save-excursion
>>>>>   (org-back-to-heading t)
>>>>>   (ignore-errors (org-up-heading-all 1))
>>>>>   ;; generate a fake change-plist with a flag to indicate a  
>>>>> endless loop
>>>>>   (setq fake-change-plist
>>>>>     (list
>>>>>       :type 'todo-state-change
>>>>>       :from "DONE"
>>>>>       :to "TODO"
>>>>>       :position 0
>>>>>       :org-block-todo-with-blocked-parent t))
>>>>>   ;; check whether the parent heading should be blocked and  
>>>>> return the result
>>>>>   (save-match-data
>>>>>     (run-hook-with-args-until-failure 'org-blocker-hook fake- 
>>>>> change-plist)))))
>>>>>
>>>>> (add-hook 'org-blocker-hook 'org-block-todo-with-blocked-parent)
>>>>>
>>>>>
>>>>>> I don't expect to change this because it would make the
>>>>>> mechanism a lot more complex and slower.
>>>>> I think it's essential to provide this eventuality. For example
>>>>> this tree:
>>>>>
>>>>> * organize party
>>>>> :PROPERTIES:
>>>>> :ORDERED:  t
>>>>> :END:
>>>>> ** TODO send invitations
>>>>> *** TODO send invitation to Paul
>>>>> *** TODO send invitation to Nicole
>>>>> *** ect.
>>>>> ** TODO buy meals and drinks
>>>>> :PROPERTIES:
>>>>> :ORDERED:  t
>>>>> :END:
>>>>> *** TODO write shopping list
>>>>> *** TODO get money  from my bank account
>>>>> *** TODO buy food
>>>>> *** TODO buy drinks
>>>>>
>>>>> with this tree, only "send invitation to Paul" and "send  
>>>>> invitation to Nicole"
>>>>> should be on my agenda. But "write shopping list" is also an my  
>>>>> agenda
>>>>> list (although it shouldn't: I can't write a shopping list if I  
>>>>> don't know how
>>>>> many people will come to my party).
>>>>>
>>>>>
>>>>> best regards,
>>>>> Daniel
>>>>>
>>>>>
>>>>>> - Carsten
>>>>>>
>>>>>> On Mar 24, 2009, at 8:13 PM, Daniel Hochheimer wrote:
>>>>>>
>>>>>>> Hello,
>>>>>>>
>>>>>>> first of all, please excuse my poorly english.
>>>>>>>
>>>>>>> It seems there is a bug in the handling of simple dependencies.
>>>>>>> I think an example tree is the best solution, to show you the  
>>>>>>> bug:
>>>>>>>
>>>>>>> * Projects
>>>>>>> #+CATEGORY: Projects
>>>>>>> *** TODO foo bar project
>>>>>>> :PROPERTIES:
>>>>>>> :ORDERED:  t
>>>>>>> :END:
>>>>>>> ***** TODO foo subproject        :FooSubproject:
>>>>>>> ******* TODO Task 1
>>>>>>> ***** TODO bar subproject        :BarSubproject:
>>>>>>> ******* TODO Task 1
>>>>>>>
>>>>>>> This is in my .emacs file:
>>>>>>> (setq org-enforce-todo-dependencies t)
>>>>>>> (setq org-agenda-dim-blocked-tasks 'invisible)
>>>>>>> (setq org-odd-levels-only t)
>>>>>>>
>>>>>>> the expected global todo agenda view imho is:
>>>>>>>
>>>>>>> Projects: Task 1       :FooSubproject:
>>>>>>>
>>>>>>> but actual it is unfortunately:
>>>>>>>
>>>>>>> Projects: Task 1       :FooSubproject:
>>>>>>> Projects: Task 1       :BarSubproject:
>>>>>>>
>>>>>>>
>>>>>>> Imho "Task 1" from "bar subproject" should not be visible,
>>>>>>> because "bar subproject " is blocked because of the
>>>>>>> ORDERED property (therefore it's childs should be blocked, too)
>>>>>>>
>>>>>>>
>>>>>>> Is it easy / possible to fix this bug? My whole GTD system is
>>>>>>> heavily based on such project / subproject-Constructs. But with
>>>>>>> this bug my global todo agenda view is unfortunately "polluted"
>>>>>>> a little bit with tasks from projects that shouldn't be active.
>>>>>>>
>>>>>>> Best regards,
>>>>>>> Daniel
>>>>>>>
>>>>>>> PS: many thanks to the developer of this great emacs mode, I  
>>>>>>> really
>>>>>>> enjoy org-mode. I started using emacs only because of the great
>>>>>>> abilities of org-mode.
>>>>>>>
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> Emacs-orgmode mailing list
>>>>>>> Remember: use `Reply All' to send replies to the list.
>>>>>>> Emacs-orgmode@gnu.org
>>>>>>> http://lists.gnu.org/mailman/listinfo/emacs-orgmode
>>>>>>
>>>>
>>>
>>
>

[-- Attachment #1.2: Type: text/html, Size: 50619 bytes --]

[-- Attachment #2: Type: text/plain, Size: 204 bytes --]

_______________________________________________
Emacs-orgmode mailing list
Remember: use `Reply All' to send replies to the list.
Emacs-orgmode@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-orgmode