emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* [BUG] Changing TODO states sometimes modifies the scheduling of the next heading
@ 2011-04-03  6:46 Tom
  2011-04-03  8:35 ` Tom
  0 siblings, 1 reply; 10+ messages in thread
From: Tom @ 2011-04-03  6:46 UTC (permalink / raw)
  To: emacs-orgmode

I have a heading where the scheduling changed misteriuosly. It is
set to repeat weekly and I found it jumped to the next week
without me touching it at all. I set up a background watcher and
after a week it detected when the unwanted change happens.

I could not yet create a simple org file to reproduce the
problem, but I found the problematic part in the code. It happens
when I change the TODO state of a heading with repeating
scheduling. When org modifies the scheduling of the heading
according to the repeat period it also modifies the scheduling of
the heading under it.

Here's the buggy part of the code:


(defun org-auto-repeat-maybe (done-word)
...
    (while (re-search-forward
              re (save-excursion (outline-next-heading) (point)) t)


http://repo.or.cz/w/org-mode.git/blob/HEAD:/lisp/org.el#l11546


The problem is the bound for the search is determined within the
while loop. The problem occurs when the scheduling of the heading
is changed and cursor is put on the beginning of the next line
after that. If that next line happens to be the next heading line
then the (outline-next-heading) call in the re-search-forward
skips over the next heading, setting the bound of the search to
the end of it, so the search includes the contents of the next
heading too, so its scheduling is also modified behind the user's
back.

The solution is simple: determine the bound of the search (the
end of the heading) before the while loop and use that value in
re-search-forward, instead of computing it within the loop.

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

* Re: [BUG] Changing TODO states sometimes modifies the scheduling of the next heading
  2011-04-03  6:46 [BUG] Changing TODO states sometimes modifies the scheduling of the next heading Tom
@ 2011-04-03  8:35 ` Tom
  2011-04-03 13:37   ` Matt Lundin
  0 siblings, 1 reply; 10+ messages in thread
From: Tom @ 2011-04-03  8:35 UTC (permalink / raw)
  To: emacs-orgmode

Tom <adatgyujto <at> gmail.com> writes:
> 
> I could not yet create a simple org file to reproduce the
> problem


Here's an org file which demonstrates the bug with org 7.5:

    * TODO test1
    SCHEDULED: <2011-04-02 Szo +1d>
    * TODO test2 
    SCHEDULED: <2011-04-03 H .+1w>


If you (setq org-log-repeat nil)  and press C-c C-t on test1
then the scheduling of test2 is modified too.

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

* Re: [BUG] Changing TODO states sometimes modifies the scheduling of the next heading
  2011-04-03  8:35 ` Tom
@ 2011-04-03 13:37   ` Matt Lundin
  2011-04-03 14:09     ` [BUG] Changing TODO states sometimes modifies the schedulingof " Tom
  0 siblings, 1 reply; 10+ messages in thread
From: Matt Lundin @ 2011-04-03 13:37 UTC (permalink / raw)
  To: Tom; +Cc: emacs-orgmode

Tom <adatgyujto@gmail.com> writes:

> Tom <adatgyujto <at> gmail.com> writes:
>> 
>> I could not yet create a simple org file to reproduce the
>> problem
>
>
> Here's an org file which demonstrates the bug with org 7.5:
>
>     * TODO test1
>     SCHEDULED: <2011-04-02 Szo +1d>
>     * TODO test2 
>     SCHEDULED: <2011-04-03 H .+1w>
>
>
> If you (setq org-log-repeat nil)  and press C-c C-t on test1
> then the scheduling of test2 is modified too.

I cannot reproduce this. 

--8<---------------cut here---------------start------------->8---
* TODO test1
  SCHEDULED: <2011-04-02 Sat +1d>
* TODO test2 
  SCHEDULED: <2011-04-03 Sun .+1w>
--8<---------------cut here---------------end--------------->8---

When I set org-log-repeat to nil and mark the first task DONE, I get the
following results:

--8<---------------cut here---------------start------------->8---
* TODO test1
  SCHEDULED: <2011-04-03 Sun +1d>
* TODO test2 
  SCHEDULED: <2011-04-03 Sun .+1w>
--8<---------------cut here---------------end--------------->8---

Best,
Matt

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

* Re: [BUG] Changing TODO states sometimes modifies the schedulingof the next heading
  2011-04-03 13:37   ` Matt Lundin
@ 2011-04-03 14:09     ` Tom
  2011-04-03 16:42       ` Matt Lundin
  0 siblings, 1 reply; 10+ messages in thread
From: Tom @ 2011-04-03 14:09 UTC (permalink / raw)
  To: emacs-orgmode

Matt Lundin <mdl <at> imapmail.org> writes:
> 
> I cannot reproduce this. 
> 

Did you start emacs without any initialization? I started it with
-Q and loaded org 7.5 manually to avoid affecting the test with 
my own org customizations.

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

* Re: [BUG] Changing TODO states sometimes modifies the schedulingof the next heading
  2011-04-03 14:09     ` [BUG] Changing TODO states sometimes modifies the schedulingof " Tom
@ 2011-04-03 16:42       ` Matt Lundin
  2011-04-03 17:49         ` Tom
  0 siblings, 1 reply; 10+ messages in thread
From: Matt Lundin @ 2011-04-03 16:42 UTC (permalink / raw)
  To: Tom; +Cc: emacs-orgmode

Tom <adatgyujto@gmail.com> writes:

> Matt Lundin <mdl <at> imapmail.org> writes:
>> 
>> I cannot reproduce this. 
>> 
>
> Did you start emacs without any initialization? I started it with
> -Q and loaded org 7.5 manually to avoid affecting the test with 
> my own org customizations.

I still cannot reproduce it.

Steps followed: 

1. /usr/bin/emacs -Q

2. (setq org-log-repeat nil)

3. (add-to-list 'load-path "~/org-mode/lisp/")

4. M-x org-reload

5. M-x org-version => Org-mode version 7.5 (release_7.5.144.g174e)

6. C-c C-t on first headline.

Results:

--8<---------------cut here---------------start------------->8---
* TODO test1
  SCHEDULED: <2011-04-03 Sun +1d>
* TODO test2 
  SCHEDULED: <2011-04-03 Sun .+1w>
--8<---------------cut here---------------end--------------->8---

Best,
Matt

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

* Re: [BUG] Changing TODO states sometimes modifies the schedulingof the next heading
  2011-04-03 16:42       ` Matt Lundin
@ 2011-04-03 17:49         ` Tom
  2011-04-04 20:25           ` Tom
  0 siblings, 1 reply; 10+ messages in thread
From: Tom @ 2011-04-03 17:49 UTC (permalink / raw)
  To: emacs-orgmode

Matt Lundin <mdl <at> imapmail.org> writes:

> 
> I still cannot reproduce it.
>

I tried your test file which was different from the test file
I suggested in the second mail and I couldn't reproduce the problem
with it either.

So I looked into this and turns out the problem occurs only if
after the state change the new day abbreviation in the timestamp
is shorter than the previous one.

In your case Sat switches to Sun, both 3 characters length.
No problem.

But in my case Szo changes to V which is two characters shorter:


--8<---------------cut here---------------start------------->8---
* TODO test1
SCHEDULED: <2011-04-02 Szo +1d>
* TODO test2 
SCHEDULED: <2011-04-03 V .+1w>
--8<---------------cut here---------------end--------------->8---


Why is it a problem?

Because org-timestamp-change starts with storing the current
cursor position which is the end of the timestamp:

15400 (defun org-timestamp-change (n &optional what updown)
15401   "Change the date in the time stamp at point.
15402 The date will be changed by N times WHAT.  WHAT can be `day', `month',
15403 `year', `minute', `second'.  If WHAT is not given, the cursor position
15404 in the timestamp determines what will be changed."
15405   (let ((pos (point))
...

http://repo.or.cz/w/org-mode.git/blob/HEAD:/lisp/org.el#l15405



and later it simply restores the position with:

15468       (goto-char pos)

http://repo.or.cz/w/org-mode.git/blob/HEAD:/lisp/org.el#l15468


The problem is in my case the new day abbreviation is two char
shorter, so the whole line is shorter, therefore the goto-char
puts the cursor in the next line (instead of at the end of the
timestamp) which triggers the bound problem I described in the
first mail:

http://article.gmane.org/gmane.emacs.orgmode/40502



Bottom line: the problem does not occur in the English locale,
because there all day abbreviations are 3 chars long, so the
above described simple way of restoring the cursor position
always works. But this is not true for all locales, so org
shouldn't rely on that.

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

* Re: [BUG] Changing TODO states sometimes modifies the schedulingof the next heading
  2011-04-03 17:49         ` Tom
@ 2011-04-04 20:25           ` Tom
  2011-07-20  8:28             ` Nicolas Goaziou
  0 siblings, 1 reply; 10+ messages in thread
From: Tom @ 2011-04-04 20:25 UTC (permalink / raw)
  To: emacs-orgmode

Tom <adatgyujto <at> gmail.com> writes:
> 
> Bottom line: the problem does not occur in the English locale,
> because there all day abbreviations are 3 chars long, so the
> above described simple way of restoring the cursor position
> always works. But this is not true for all locales, so org
> shouldn't rely on that.
> 
> 


I created a temporary fix for the problem with advice until it
is fixed in the code properly.


Here it is if someone needs it:


(defadvice org-todo (around my-org-todo activate)
  (save-restriction
    (narrow-to-region (save-excursion (org-back-to-heading t) (point))
                      (save-excursion (outline-next-heading) (point)))
    ad-do-it))

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

* Re: [BUG] Changing TODO states sometimes modifies the schedulingof the next heading
  2011-04-04 20:25           ` Tom
@ 2011-07-20  8:28             ` Nicolas Goaziou
  2011-07-21 19:25               ` Tom
  0 siblings, 1 reply; 10+ messages in thread
From: Nicolas Goaziou @ 2011-07-20  8:28 UTC (permalink / raw)
  To: Tom; +Cc: emacs-orgmode

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

Hello,

Tom <adatgyujto@gmail.com> writes:

> Bottom line: the problem does not occur in the English locale,
> because there all day abbreviations are 3 chars long, so the
> above described simple way of restoring the cursor position
> always works. But this is not true for all locales, so org
> shouldn't rely on that.

Would you mind telling me if the following patch fixes your problem?
If so, I'll apply it to the code base.

Regards,

-- 
Nicolas Goaziou

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-bug-with-TODO-states-changes-modifying-schedulin.patch --]
[-- Type: text/x-patch, Size: 1438 bytes --]

From 6ab4222325f304d89bb161085956bc3c2d1d7617 Mon Sep 17 00:00:00 2001
From: Nicolas Goaziou <n.goaziou@gmail.com>
Date: Wed, 20 Jul 2011 10:18:31 +0200
Subject: [PATCH] Fix bug with TODO states changes modifying scheduling of
 next headline

* lisp/org.el (org-timestamp-change): some locales don't use the same
  length for date abbreviations. Set a marker at origin in case length
  of new timestamp is different.

Thanks to Tom for analyzing this.
---
 lisp/org.el |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index fee13b7..c08ab75 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -15567,7 +15567,7 @@ With prefix ARG, change that many days."
 The date will be changed by N times WHAT.  WHAT can be `day', `month',
 `year', `minute', `second'.  If WHAT is not given, the cursor position
 in the timestamp determines what will be changed."
-  (let ((pos (point))
+  (let ((pos (copy-marker (point)))
 	with-hm inactive
 	(dm (max (nth 1 org-time-stamp-rounding-minutes) 1))
 	org-ts-what
@@ -15631,6 +15631,7 @@ in the timestamp determines what will be changed."
 	    (org-insert-time-stamp time with-hm inactive nil nil extra))
       (org-clock-update-time-maybe)
       (goto-char pos)
+      (move-marker pos nil)
       ;; Try to recenter the calendar window, if any
       (if (and org-calendar-follow-timestamp-change
 	       (get-buffer-window "*Calendar*" t)
-- 
1.7.6


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

* Re: [BUG] Changing TODO states sometimes modifies the schedulingof the next heading
  2011-07-20  8:28             ` Nicolas Goaziou
@ 2011-07-21 19:25               ` Tom
  2011-07-22  7:31                 ` Bastien
  0 siblings, 1 reply; 10+ messages in thread
From: Tom @ 2011-07-21 19:25 UTC (permalink / raw)
  To: emacs-orgmode

Nicolas Goaziou <n.goaziou <at> gmail.com> writes:
 
> Would you mind telling me if the following patch fixes your problem?
> If so, I'll apply it to the code base.
> 

Yes, it seems to work with the test file I mentioned previously in
the thread.

I'm a bit suprised it took so long until someone fixed the problem
though it was a major bug (changing scheduling of items unintentionally),
the problem was well analyzed and the solution was simple.

Anyway, thanks for the fix. Now I can remove my defadvice workaround
from my setup as it apparently is no longer needed.

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

* Re: [BUG] Changing TODO states sometimes modifies the schedulingof the next heading
  2011-07-21 19:25               ` Tom
@ 2011-07-22  7:31                 ` Bastien
  0 siblings, 0 replies; 10+ messages in thread
From: Bastien @ 2011-07-22  7:31 UTC (permalink / raw)
  To: Tom; +Cc: emacs-orgmode

Hi Tom,

Tom <adatgyujto@gmail.com> writes:

> I'm a bit suprised it took so long until someone fixed the problem
> though it was a major bug (changing scheduling of items unintentionally),
> the problem was well analyzed and the solution was simple.

Apparently not.  

And the problem only hit people using locales other than english, 
maybe that explains why there was not that many complaints.

Anyway, we will provide a solution for this.

Thanks for your patience,

-- 
 Bastien

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

end of thread, other threads:[~2011-07-22  7:30 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-04-03  6:46 [BUG] Changing TODO states sometimes modifies the scheduling of the next heading Tom
2011-04-03  8:35 ` Tom
2011-04-03 13:37   ` Matt Lundin
2011-04-03 14:09     ` [BUG] Changing TODO states sometimes modifies the schedulingof " Tom
2011-04-03 16:42       ` Matt Lundin
2011-04-03 17:49         ` Tom
2011-04-04 20:25           ` Tom
2011-07-20  8:28             ` Nicolas Goaziou
2011-07-21 19:25               ` Tom
2011-07-22  7:31                 ` Bastien

Code repositories for project(s) associated with this 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).