Regression in `org-todo'

Regression in `org-todo'

[Moritz Ulrich]

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

Commit 6cbf1f417222321a47848a7368427ba8a22fe3a5 renames variable
`last-state' to `org-last-state' in org.el. This change isn't
reflected in function `org-todo'.

That causes the following regression: Toggling a TODO state to DONE on
a headline via C-c C-t (`org-todo') results in: "Symbol's value as
variable is void: org-last-state".
This is reproducible in latest HEAD and even in tag 'release_7.8.07'.

M-x emacs-version: GNU Emacs 24.0.94.1 (i386-apple-darwin11.3.0, NS
apple-appkit-1138.32) of 2012-03-12 on moritz-macbook.local

The attached patch contains a simple (two line) fix.


Cheers,
Moritz Ulrich

-- 
Moritz Ulrich

[-- Attachment #2: fix-org-last-state.patch --]
[-- Type: application/octet-stream, Size: 865 bytes --]

diff --git a/lisp/org.el b/lisp/org.el
index c418c67..169e17d 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -11313,7 +11313,7 @@ For calling through lisp, arg is also interpreted in the following way:
 		 (interpret (nth 1 ass))
 		 (done-word (nth 3 ass))
 		 (final-done-word (nth 4 ass))
-		 (last-state (or this ""))
+		 (org-last-state (or this ""))
 		 (completion-ignore-case t)
 		 (member (member this org-todo-keywords-1))
 		 (tail (cdr member))
@@ -11379,7 +11379,7 @@ For calling through lisp, arg is also interpreted in the following way:
 			  (car tail))))
 		 (state (or
 			 (run-hook-with-args-until-success
-			  'org-todo-get-default-hook state last-state)
+			  'org-todo-get-default-hook state org-last-state)
 			 state))
 		 (next (if state (concat " " state " ") " "))
 		 (change-plist (list :type 'todo-state-change :from this :to state

Re: [URGENT] Regression in `org-todo'

[Moritz Ulrich]

Marking it URGENT as the release for emacs 24 seems to be affected too.

On Sun, Apr 1, 2012 at 17:01, Moritz Ulrich
<ulrich.moritz@googlemail.com> wrote:
> Commit 6cbf1f417222321a47848a7368427ba8a22fe3a5 renames variable
> `last-state' to `org-last-state' in org.el. This change isn't
> reflected in function `org-todo'.
>
> That causes the following regression: Toggling a TODO state to DONE on
> a headline via C-c C-t (`org-todo') results in: "Symbol's value as
> variable is void: org-last-state".
> This is reproducible in latest HEAD and even in tag 'release_7.8.07'.
>
> M-x emacs-version: GNU Emacs 24.0.94.1 (i386-apple-darwin11.3.0, NS
> apple-appkit-1138.32) of 2012-03-12 on moritz-macbook.local
>
> The attached patch contains a simple (two line) fix.
>
>
> Cheers,
> Moritz Ulrich
>
> --
> Moritz Ulrich



-- 
Moritz Ulrich

Re: [URGENT] Regression in `org-todo'

[Bastien]

Moritz Ulrich <ulrich.moritz@googlemail.com> writes:

> Marking it URGENT as the release for emacs 24 seems to be affected
> too.

Fixed, thanks.

-- 
 Bastien

Re: [URGENT] Regression in `org-todo'

[Bastien]

Bastien <bzg@altern.org> writes:

> Moritz Ulrich <ulrich.moritz@googlemail.com> writes:
>
>> Marking it URGENT as the release for emacs 24 seems to be affected
>> too.
>
> Fixed, thanks.

Fixed in Emacs too -- this will be fixed in the next minor release
of Org, this is in hotfix/master in the meantime.

Thanks,

-- 
 Bastien

Re: Regression in `org-todo'

[Michael Strey]

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

Hi Moritz & Bastien,

This bug fix seems not to be complete.  At least switching the todo
state in repeated tasks and in conjunction with org-checklist didn't
work for me as it used to before the last update.  PFA a patch that
solved the problem for me.

ATTENTION: Please check this patch very carefully.  This is my first
contribution to org-mode and my first patch ever.  My lisp knowledge is
rudimentary and I do not really understand the code.

Thanks to all developers for your work!

Best regards
Michael Strey


On Sun, Apr 01, 2012 at 05:01:12PM +0200, Moritz Ulrich wrote:
> Commit 6cbf1f417222321a47848a7368427ba8a22fe3a5 renames variable
> `last-state' to `org-last-state' in org.el. This change isn't
> reflected in function `org-todo'.
> 
> That causes the following regression: Toggling a TODO state to DONE on
> a headline via C-c C-t (`org-todo') results in: "Symbol's value as
> variable is void: org-last-state".
> This is reproducible in latest HEAD and even in tag 'release_7.8.07'.
> 
> M-x emacs-version: GNU Emacs 24.0.94.1 (i386-apple-darwin11.3.0, NS
> apple-appkit-1138.32) of 2012-03-12 on moritz-macbook.local
> 
> The attached patch contains a simple (two line) fix.

[-- Attachment #2: 0001-s-state-org-state.patch --]
[-- Type: text/plain, Size: 3998 bytes --]

From 554b1ece898c9aaff47e729d6fa746ec21b5eefd Mon Sep 17 00:00:00 2001
From: Michael Strey <mstrey@strey.biz>
Date: Mon, 2 Apr 2012 16:21:34 +0200
Subject: [PATCH] s/state/org-state/

---
 contrib/lisp/org-checklist.el |    2 +-
 lisp/org.el                   |   14 +++++++-------
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/contrib/lisp/org-checklist.el b/contrib/lisp/org-checklist.el
index a974874..03c7547 100644
--- a/contrib/lisp/org-checklist.el
+++ b/contrib/lisp/org-checklist.el
@@ -129,7 +129,7 @@ of checkbox items"
 			 (a2ps-buffer)))))))))
 
 (defun org-checklist ()
-  (when (member state org-done-keywords)
+  (when (member org-state org-done-keywords)
     (org-make-checklist-export)
     (org-reset-checkbox-state-maybe)))
 
diff --git a/lisp/org.el b/lisp/org.el
index 9183743..bf30497 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -2429,7 +2429,7 @@ When nil, only the date will be recorded."
 
 (defcustom org-log-note-headings
   '((done .  "CLOSING NOTE %t")
-    (state . "State %-12s from %-12S %t")
+    (org-state . "State %-12s from %-12S %t")
     (note .  "Note taken on %t")
     (reschedule .  "Rescheduled from %S on %t")
     (delschedule .  "Not scheduled, was %S on %t")
@@ -2458,7 +2458,7 @@ agenda log mode depends on the format of these entries."
 	  (cons (const :tag "Heading when closing an item" done) string)
 	  (cons (const :tag
 		       "Heading when changing todo state (todo sequence only)"
-		       state) string)
+		       org-state) string)
 	  (cons (const :tag "Heading when just taking a note" note) string)
 	  (cons (const :tag "Heading when clocking out" clock-out) string)
 	  (cons (const :tag "Heading when an item is no longer scheduled" delschedule) string)
@@ -11437,10 +11437,10 @@ For calling through lisp, arg is also interpreted in the following way:
 		;; It is now done, and it was not done before
 		(org-add-planning-info 'closed (org-current-effective-time))
 		(if (and (not dolog) (eq 'note org-log-done))
-		    (org-add-log-setup 'done state this 'findpos 'note)))
+		    (org-add-log-setup 'done org-state this 'findpos 'note)))
 	      (when (and org-state dolog)
 		;; This is a non-nil state, and we need to log it
-		(org-add-log-setup 'state org-state this 'findpos dolog)))
+		(org-add-log-setup 'org-state org-state this 'findpos dolog)))
 	    ;; Fixup tag positioning
 	    (org-todo-trigger-tag-changes org-state)
 	    (and org-auto-align-tags (not org-setting-tags) (org-set-tags nil t))
@@ -11938,7 +11938,7 @@ This function is run automatically after each state change to a DONE state."
 	 (org-todo-log-states nil)
 	 re type n what ts time to-state)
     (when repeat
-      (if (eq org-log-repeat t) (setq org-log-repeat 'state))
+      (if (eq org-log-repeat t) (setq org-log-repeat 'org-state))
       (setq to-state (or (org-entry-get nil "REPEAT_TO_STATE")
 			 org-todo-repeat-to-state))
       (unless (and to-state (member to-state org-todo-keywords-1))
@@ -11955,7 +11955,7 @@ This function is run automatically after each state change to a DONE state."
 		;; make sure we take a note, not only a time stamp
 		(setq org-log-note-how 'note))
 	  ;; Set up for taking a record
-	  (org-add-log-setup 'state (or done-word (car org-done-keywords))
+	  (org-add-log-setup 'org-state (or done-word (car org-done-keywords))
 			     org-last-state
 			     'findpos org-log-repeat)))
       (org-back-to-heading t)
@@ -12377,7 +12377,7 @@ EXTRA is additional text that will be inserted into the notes buffer."
 		    (cond
 		     ((eq org-log-note-purpose 'clock-out) "stopped clock")
 		     ((eq org-log-note-purpose 'done)  "closed todo item")
-		     ((eq org-log-note-purpose 'state)
+		     ((eq org-log-note-purpose 'org-state)
 		      (format "state change from \"%s\" to \"%s\""
 			      (or org-log-note-previous-state "")
 			      (or org-log-note-state "")))
-- 
1.7.9.4

Re: Regression in `org-todo'

[Bastien]

Hi Michael,

Michael Strey <mstrey@strey.biz> writes:

> This bug fix seems not to be complete.  At least switching the todo
> state in repeated tasks and in conjunction with org-checklist didn't
> work for me as it used to before the last update.  PFA a patch that
> solved the problem for me.

Thanks for reporting this -- this has already been solved in this
commit:

http://orgmode.org/w/?p=org-mode.git;a=commit;h=9d01ca1bf357917de0c496e86405e24c65b5761c

> ATTENTION: Please check this patch very carefully.  This is my first
> contribution to org-mode and my first patch ever.  My lisp knowledge is
> rudimentary and I do not really understand the code.

The rest of the patch isn't relevant as it changes a symbol's name
(s/state/org-state) for which we don't need the value.  

Please test against latest HEAD and tell me if your problem is still
here, it shouldn't.

Thanks!

-- 
 Bastien

Re: Regression in `org-todo'

[Michael Strey]

Hi Bastien,

On Mon, Apr 02, 2012 at 05:03:04PM +0200, Bastien wrote:
> ...
> Please test against latest HEAD and tell me if your problem is still
> here, it shouldn't.
 
I confirm that the problem is solved.  Thank you!

It's interesting to see how a bug in a module from the contrib path can
cause strange behaviour of org-mode's core functions.

Best regards
Michael Strey

Re: Regression in `org-todo'

[Bastien]

Hi Michael,

Michael Strey <mstrey@strey.biz> writes:

> It's interesting to see how a bug in a module from the contrib path can
> cause strange behaviour of org-mode's core functions.

Indeed.  Perhaps org-checklist.el should be part of core.

Opinions?

-- 
 Bastien

Re: Regression in `org-todo'

[Nick Dokos]

Bastien <bzg@altern.org> wrote:

> Hi Michael,
> 
> Michael Strey <mstrey@strey.biz> writes:
> 
> > It's interesting to see how a bug in a module from the contrib path can
> > cause strange behaviour of org-mode's core functions.
> 
> Indeed.  Perhaps org-checklist.el should be part of core.
> 
> Opinions?
> 

There might be other reasons to include org-checklist.el in the core,
but as explained in my email to Michael, this is not one of them.

Nick

Re: Regression in `org-todo'

[Nick Dokos]

Michael Strey <mstrey@strey.biz> wrote:

> Hi Bastien,
> 
> On Mon, Apr 02, 2012 at 05:03:04PM +0200, Bastien wrote:
> > ...
> > Please test against latest HEAD and tell me if your problem is still
> > here, it shouldn't.
>  
> I confirm that the problem is solved.  Thank you!
> 
> It's interesting to see how a bug in a module from the contrib path can
> cause strange behaviour of org-mode's core functions.
> 

There is no distinction between contrib and core. Once a module is
loaded (wherever it comes from), it is part of emacs and can cause all
kinds of damage: not just to org, but to the whole running emacs. It's
the price one pays for the convenience of dynamically loading code.  You
can avoid the danger by not loading modules, but then emacs becomes much
less interesting: after all, the "e" stands for "extensible".

And it's not just emacs: if you load a module in linux, it can muck
around with kernel memory and crash your system in no time at all.

The whole microkernel approach was an attempt to have that cake and eat
it too: safely loading modules (they run in a separate address space and
can't touch core kernel memory). Nobody has found a way to make it as
fast as a monolithic kernel though (although people have argued forever
about it: the Tanenbaum-Torvalds debate centered exactly around that
point - after you subtract the flaming).

Nick

Re: Regression in `org-todo'

[Bastien]

Hi Nick,

Nick Dokos <nicholas.dokos@hp.com> writes:

> Michael Strey <mstrey@strey.biz> wrote:
>
>> Hi Bastien,
>> 
>> On Mon, Apr 02, 2012 at 05:03:04PM +0200, Bastien wrote:
>> > ...
>> > Please test against latest HEAD and tell me if your problem is still
>> > here, it shouldn't.
>>  
>> I confirm that the problem is solved.  Thank you!
>> 
>> It's interesting to see how a bug in a module from the contrib path can
>> cause strange behaviour of org-mode's core functions.
>> 
>
> There is no distinction between contrib and core. Once a module is
> loaded (wherever it comes from), it is part of emacs and can cause all
> kinds of damage: not just to org, but to the whole running emacs. It's
> the price one pays for the convenience of dynamically loading code.  You
> can avoid the danger by not loading modules, but then emacs becomes much
> less interesting: after all, the "e" stands for "extensible".

If the features provided by org-checklist.el are useful to many people, 
including them in core seems quite natural.  I don't use it myself, I'm
just wondering how people do.  But I agree with you here that we should
not move things into core inconsiderately.

-- 
 Bastien

Re: Regression in `org-todo'

[Bernt Hansen]

Bastien <bzg@altern.org> writes:

> If the features provided by org-checklist.el are useful to many people, 
> including them in core seems quite natural.  I don't use it myself, I'm
> just wondering how people do.  But I agree with you here that we should
> not move things into core inconsiderately.

Hi Bastien,

I use org-checklist for the RESET_CHECK_BOXES functionality on repeating
tasks.

I have lots of repeating tasks with checkboxes to mark what needs to be
completed and the RESET_CHECK_BOXES property clears all of the boxes for
the next repeating event.

My weekly review is one of these types of tasks.  I just walk down the
list and check off the stuff as I do it, and when I'm done I mark the
task done.  All the boxes are ready to be checked again the next time it
comes up.

Regards,
Bernt