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

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

Hello Carsten,

thanks a million for optimizing and including
this blocking-behavior into org-mode! :)

It seems to work like a charm.

Best regards,
Daniel

Carsten Dominik wrote:
> 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 <mailto:Emacs-orgmode@gnu.org>
>>>>>>>> http://lists.gnu.org/mailman/listinfo/emacs-orgmode
>>>>>>>
>>>>>
>>>>
>>>
>>
>