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: Wed, 01 Apr 2009 22:02:32 +0200	[thread overview]
Message-ID: <49D3C858.7040301@obyz.de> (raw)
In-Reply-To: <6699EFD4-DDCF-463F-89D2-8614F10963FA@gmail.com>

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
>>>>>
>>>
>>
>