emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* BUG: TODO statistics in parent heading prevent evaluation of TODOs with TRIGGER property
@ 2017-12-10 21:25 Adrian Bradd
  2017-12-10 21:53 ` Nicolas Goaziou
  0 siblings, 1 reply; 6+ messages in thread
From: Adrian Bradd @ 2017-12-10 21:25 UTC (permalink / raw)
  To: emacs-orgmode


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

Hi,

Please see the patch attached.

When completing a TODO with a TRIGGER property that has statistics in the
parent headline the trigger would not evaluate because the :position
property in `change-plist' may now refer to the line above the original
TODO.

I have used a marker to avoid the issue with the point moving due to the
addition of characters
​ in the parent headline​
. Not sure if this is the best way to solve the problem.

Cheers,

Adrian

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

[-- Attachment #2: 0001-lisp-org.el-Use-marker-for-change-plist-position-pro.patch --]
[-- Type: application/octet-stream, Size: 1593 bytes --]

From fc8a47cc6eb98e312c594bb29834c119fda1361e Mon Sep 17 00:00:00 2001
From: Adrian Bradd <adrian.bradd@gmail.com>
Date: Sun, 10 Dec 2017 16:07:57 -0500
Subject: [PATCH] lisp/org.el: Use marker for `change-plist' position property

* org.el (org-todo): Use marker `change-plist' position property to
  permit triggering through org-depend.el with parent heading
  statistics

When todo statistics in parent headings were updated it shifted the point
defining the :position property in `change-plist' causing headlines
with TRIGGER properties to fail to evaluate.

TINYCHANGE
---
 lisp/org.el | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lisp/org.el b/lisp/org.el
index 4b4ce40..03de5ab 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -12379,6 +12379,7 @@ When called through ELisp, arg is also interpreted in the following way:
 	      (looking-at "\\(?: *\\|[ \t]*$\\)"))
 	  (let* ((match-data (match-data))
 		 (startpos (point-at-bol))
+		 (startmark (mark-marker))
 		 (logging (save-match-data (org-entry-get nil "LOGGING" t t)))
 		 (org-log-done org-log-done)
 		 (org-log-repeat org-log-repeat)
@@ -12461,7 +12462,7 @@ When called through ELisp, arg is also interpreted in the following way:
 			     org-state))
 		 (next (if org-state (concat " " org-state " ") " "))
 		 (change-plist (list :type 'todo-state-change :from this :to org-state
-				     :position startpos))
+				     :position (set-marker startmark startpos)))
 		 dolog now-done-p)
 	    (when org-blocker-hook
 	      (let (org-blocked-by-checkboxes block-reason)
-- 
2.10.1 (Apple Git-78)


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: BUG: TODO statistics in parent heading prevent evaluation of TODOs with TRIGGER property
  2017-12-10 21:25 BUG: TODO statistics in parent heading prevent evaluation of TODOs with TRIGGER property Adrian Bradd
@ 2017-12-10 21:53 ` Nicolas Goaziou
  2017-12-10 22:50   ` Adrian Bradd
  0 siblings, 1 reply; 6+ messages in thread
From: Nicolas Goaziou @ 2017-12-10 21:53 UTC (permalink / raw)
  To: Adrian Bradd; +Cc: emacs-orgmode

Hello,

Adrian Bradd <adrian.bradd@gmail.com> writes:

> Please see the patch attached.
>
> When completing a TODO with a TRIGGER property that has statistics in the
> parent headline the trigger would not evaluate because the :position
> property in `change-plist' may now refer to the line above the original
> TODO.
>
> I have used a marker to avoid the issue with the point moving due to the
> addition of characters
> ​ in the parent headline​
> . Not sure if this is the best way to solve the problem.

IIUC, point is moved between `startpos' and `change-plist' bindings? Do
you know when that happens? Would you have an ECM for the issue?

Thank you.

Regards,

-- 
Nicolas Goaziou

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: BUG: TODO statistics in parent heading prevent evaluation of TODOs with TRIGGER property
  2017-12-10 21:53 ` Nicolas Goaziou
@ 2017-12-10 22:50   ` Adrian Bradd
  2017-12-10 22:51     ` Adrian Bradd
  0 siblings, 1 reply; 6+ messages in thread
From: Adrian Bradd @ 2017-12-10 22:50 UTC (permalink / raw)
  To: Nicolas Goaziou; +Cc: emacs-orgmode

[-- Attachment #1: Type: text/plain, Size: 2049 bytes --]

Hello,

ECM:

* Top-Heading with process indicator [/]

** TODO Here I invoke org-todo to DONE
:PROPERTIES:
:TRIGGER:  2021-12-03-target(TODO)
:END:

** This should be changed to TODO
:PROPERTIES:
:ID: 2021-12-03-target
:END:

If you run org-todo on the "Here I invoke org-todo to DONE" headline the
headline will change to DONE, but the trigger will not update the "This
should be changed to TODO" headline. There is further discussion in another
thread where the user reported the issue [1].

The issue is Line 12534 in org.el:

(when org-provide-todo-statistics
  (org-update-parent-todo-statistics))

which traverses the tree and updates the todo progress statistics. If the
statistic is [/], as in the ECM above, or [%] it will add 1 or more
characters which is enough to push the :position property up to the line
above. I wasn't sure how to deal with this as it seems
`org-update-parent-todo-statistics' could update more than one parent
heading and the number of additional characters isn't clear without some
feedback from `org-update-parent-todo-statistics'.

Cheers,

Adrian

[1] https://lists.gnu.org/archive/html/emacs-orgmode/2017-12/msg00058.html

On 10 December 2017 at 16:53, Nicolas Goaziou <mail@nicolasgoaziou.fr>
wrote:

> Hello,
>
> Adrian Bradd <adrian.bradd@gmail.com> writes:
>
> > Please see the patch attached.
> >
> > When completing a TODO with a TRIGGER property that has statistics in the
> > parent headline the trigger would not evaluate because the :position
> > property in `change-plist' may now refer to the line above the original
> > TODO.
> >
> > I have used a marker to avoid the issue with the point moving due to the
> > addition of characters
> > ​ in the parent headline​
> > . Not sure if this is the best way to solve the problem.
>
> IIUC, point is moved between `startpos' and `change-plist' bindings? Do
> you know when that happens? Would you have an ECM for the issue?
>
> Thank you.
>
> Regards,
>
> --
> Nicolas Goaziou
>

[-- Attachment #2: Type: text/html, Size: 3799 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: BUG: TODO statistics in parent heading prevent evaluation of TODOs with TRIGGER property
  2017-12-10 22:50   ` Adrian Bradd
@ 2017-12-10 22:51     ` Adrian Bradd
  2017-12-26  3:55       ` Adrian Bradd
  0 siblings, 1 reply; 6+ messages in thread
From: Adrian Bradd @ 2017-12-10 22:51 UTC (permalink / raw)
  To: Nicolas Goaziou; +Cc: emacs-orgmode

[-- Attachment #1: Type: text/plain, Size: 2312 bytes --]

I should probably add that this will require org-depend.el to be loaded.

On 10 December 2017 at 17:50, Adrian Bradd <adrian.bradd@gmail.com> wrote:

> Hello,
>
> ECM:
>
> * Top-Heading with process indicator [/]
>
> ** TODO Here I invoke org-todo to DONE
> :PROPERTIES:
> :TRIGGER:  2021-12-03-target(TODO)
> :END:
>
> ** This should be changed to TODO
> :PROPERTIES:
> :ID: 2021-12-03-target
> :END:
>
> If you run org-todo on the "Here I invoke org-todo to DONE" headline the
> headline will change to DONE, but the trigger will not update the "This
> should be changed to TODO" headline. There is further discussion in another
> thread where the user reported the issue [1].
>
> The issue is Line 12534 in org.el:
>
> (when org-provide-todo-statistics
>   (org-update-parent-todo-statistics))
>
> which traverses the tree and updates the todo progress statistics. If the
> statistic is [/], as in the ECM above, or [%] it will add 1 or more
> characters which is enough to push the :position property up to the line
> above. I wasn't sure how to deal with this as it seems
> `org-update-parent-todo-statistics' could update more than one parent
> heading and the number of additional characters isn't clear without some
> feedback from `org-update-parent-todo-statistics'.
>
> Cheers,
>
> Adrian
>
> [1] https://lists.gnu.org/archive/html/emacs-orgmode/2017-12/msg00058.html
>
> On 10 December 2017 at 16:53, Nicolas Goaziou <mail@nicolasgoaziou.fr>
> wrote:
>
>> Hello,
>>
>> Adrian Bradd <adrian.bradd@gmail.com> writes:
>>
>> > Please see the patch attached.
>> >
>> > When completing a TODO with a TRIGGER property that has statistics in
>> the
>> > parent headline the trigger would not evaluate because the :position
>> > property in `change-plist' may now refer to the line above the original
>> > TODO.
>> >
>> > I have used a marker to avoid the issue with the point moving due to the
>> > addition of characters
>> > ​ in the parent headline​
>> > . Not sure if this is the best way to solve the problem.
>>
>> IIUC, point is moved between `startpos' and `change-plist' bindings? Do
>> you know when that happens? Would you have an ECM for the issue?
>>
>> Thank you.
>>
>> Regards,
>>
>> --
>> Nicolas Goaziou
>>
>
>

[-- Attachment #2: Type: text/html, Size: 4424 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: BUG: TODO statistics in parent heading prevent evaluation of TODOs with TRIGGER property
  2017-12-10 22:51     ` Adrian Bradd
@ 2017-12-26  3:55       ` Adrian Bradd
  2017-12-27 22:59         ` Nicolas Goaziou
  0 siblings, 1 reply; 6+ messages in thread
From: Adrian Bradd @ 2017-12-26  3:55 UTC (permalink / raw)
  To: Nicolas Goaziou; +Cc: emacs-orgmode


Just wanted to bump this.

Let me know if there is a preferred/better way to attack this issue and
I can give it a shot.

Cheers,

Adrian

Adrian Bradd <adrian.bradd@gmail.com> writes:

> I should probably add that this will require org-depend.el to be loaded.
>
> On 10 December 2017 at 17:50, Adrian Bradd <adrian.bradd@gmail.com> wrote:
>
>> Hello,
>>
>> ECM:
>>
>> * Top-Heading with process indicator [/]
>>
>> ** TODO Here I invoke org-todo to DONE
>> :PROPERTIES:
>> :TRIGGER:  2021-12-03-target(TODO)
>> :END:
>>
>> ** This should be changed to TODO
>> :PROPERTIES:
>> :ID: 2021-12-03-target
>> :END:
>>
>> If you run org-todo on the "Here I invoke org-todo to DONE" headline the
>> headline will change to DONE, but the trigger will not update the "This
>> should be changed to TODO" headline. There is further discussion in another
>> thread where the user reported the issue [1].
>>
>> The issue is Line 12534 in org.el:
>>
>> (when org-provide-todo-statistics
>>   (org-update-parent-todo-statistics))
>>
>> which traverses the tree and updates the todo progress statistics. If the
>> statistic is [/], as in the ECM above, or [%] it will add 1 or more
>> characters which is enough to push the :position property up to the line
>> above. I wasn't sure how to deal with this as it seems
>> `org-update-parent-todo-statistics' could update more than one parent
>> heading and the number of additional characters isn't clear without some
>> feedback from `org-update-parent-todo-statistics'.
>>
>> Cheers,
>>
>> Adrian
>>
>> [1] https://lists.gnu.org/archive/html/emacs-orgmode/2017-12/msg00058.html
>>
>> On 10 December 2017 at 16:53, Nicolas Goaziou <mail@nicolasgoaziou.fr>
>> wrote:
>>
>>> Hello,
>>>
>>> Adrian Bradd <adrian.bradd@gmail.com> writes:
>>>
>>> > Please see the patch attached.
>>> >
>>> > When completing a TODO with a TRIGGER property that has statistics in
>>> the
>>> > parent headline the trigger would not evaluate because the :position
>>> > property in `change-plist' may now refer to the line above the original
>>> > TODO.
>>> >
>>> > I have used a marker to avoid the issue with the point moving due to the
>>> > addition of characters
>>> > ​ in the parent headline​
>>> > . Not sure if this is the best way to solve the problem.
>>>
>>> IIUC, point is moved between `startpos' and `change-plist' bindings? Do
>>> you know when that happens? Would you have an ECM for the issue?
>>>
>>> Thank you.
>>>
>>> Regards,
>>>
>>> --
>>> Nicolas Goaziou
>>>
>>
>>


-- 
Adrian

Bioelectronic Systems Lab,
Columbia University

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: BUG: TODO statistics in parent heading prevent evaluation of TODOs with TRIGGER property
  2017-12-26  3:55       ` Adrian Bradd
@ 2017-12-27 22:59         ` Nicolas Goaziou
  0 siblings, 0 replies; 6+ messages in thread
From: Nicolas Goaziou @ 2017-12-27 22:59 UTC (permalink / raw)
  To: Adrian Bradd; +Cc: emacs-orgmode

Hello,

Adrian Bradd <adrian.bradd@gmail.com> writes:

> Just wanted to bump this.
>
> Let me know if there is a preferred/better way to attack this issue and
> I can give it a shot.

Oops, this was falling through the cracks.

I applied your initial patch. Thank you.

Regards,

-- 
Nicolas Goaziou

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-12-27 22:59 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-10 21:25 BUG: TODO statistics in parent heading prevent evaluation of TODOs with TRIGGER property Adrian Bradd
2017-12-10 21:53 ` Nicolas Goaziou
2017-12-10 22:50   ` Adrian Bradd
2017-12-10 22:51     ` Adrian Bradd
2017-12-26  3:55       ` Adrian Bradd
2017-12-27 22:59         ` Nicolas Goaziou

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).