emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* Bug: Duplicate logbook entry for repeated tasks [9.4.6 (9.4.6-gab9f2a @ /home/gustavo/.emacs.d/elpa/org-9.4.6/)]
@ 2021-06-14 13:39 Gustavo Barros
  2021-06-14 18:01 ` Bhavin Gandhi
  0 siblings, 1 reply; 18+ messages in thread
From: Gustavo Barros @ 2021-06-14 13:39 UTC (permalink / raw)
  To: emacs-orgmode

Hi All,

The marking of repeated tasks as "done" is currently resulting in 
duplicate entries in the "LOGBOOK" drawer, which is not expected.  I 
don't know exactly when this came to be, but it does not happen in the 
current built-in version (9.4.4), while it does in the latest release 
(9.4.6).

An ECM to reproduce the issue is the following.

Start 'emacs -Q' and do an initial setup:

#+begin_src emacs-lisp
(add-to-list 'load-path "~/.emacs.d/elpa/org-9.4.6")
(setq org-log-into-drawer t)
(setq org-log-done 'time)
(setq org-todo-keywords
      '((sequence "TODO(t)" "SOMEDAY(s)" "|" "DONE(d!)")
        (sequence "NEXT(n)" "WAIT(w@/!)" "|" "CANCELED(c@/!)")))
#+end_src

Visit file =test.org= with contents:
#+begin_src org
,* TODO Foo
SCHEDULED: <2021-06-13 Sun +1d>
#+end_src

Now call "C-c C-t" (`org-todo'), and call the key "d" for "DONE", as per 
the above settings.  The resulting buffer is:

#+begin_src org
,* TODO Foo
SCHEDULED: <2021-06-15 Tue +1d>
:PROPERTIES:
:LAST_REPEAT: [2021-06-14 Mon 10:27]
:END:
:LOGBOOK:
- State "DONE"       from "TODO"       [2021-06-14 Mon 10:27]
- State "DONE"       from "TODO"       [2021-06-14 Mon 10:27]
:END:
#+end_src


Best regards,
Gustavo.



Emacs  : GNU Emacs 27.2 (build 1, x86_64-pc-linux-gnu, GTK+ Version 
3.24.20, cairo version 1.16.0)
 of 2021-03-25
Package: Org mode version 9.4.6 (9.4.6-gab9f2a @ 
/home/gustavo/.emacs.d/elpa/org-9.4.6/)

current state:
==============
(setq
 org-src-mode-hook '(org-src-babel-configure-edit-buffer
		     org-src-mode-configure-edit-buffer)
 org-link-shell-confirm-function 'yes-or-no-p
 org-metadown-hook '(org-babel-pop-to-session-maybe)
 org-clock-out-hook '(org-clock-remove-empty-clock-drawer)
 org-mode-hook '(#[0 "\300\301\302\303\304$\207"
		   [add-hook change-major-mode-hook org-show-all append 
		   local]
		   5]
		 #[0 "\300\301\302\303\304$\207"
		   [add-hook change-major-mode-hook 
		   org-babel-show-result-all
		    append local]
		   5]
		 org-babel-result-hide-spec org-babel-hide-all-hashes)
 org-archive-hook '(org-attach-archive-delete-maybe)
 org-confirm-elisp-link-function 'yes-or-no-p
 org-agenda-before-write-hook '(org-agenda-add-entry-text)
 org-metaup-hook '(org-babel-load-in-session-maybe)
 org-bibtex-headline-format-function #[257 "\300\236A\207" [:title] 3 
 "\n\n(fn ENTRY)"]
 org-babel-pre-tangle-hook '(save-buffer)
 org-tab-first-hook '(org-babel-hide-result-toggle-maybe
		      org-babel-header-arg-expand)
 org-log-done 'time
 org-agenda-loop-over-headlines-in-active-region nil
 org-occur-hook '(org-first-headline-recenter)
 org-log-into-drawer t
 org-cycle-hook '(org-cycle-hide-archived-subtrees 
 org-cycle-hide-drawers
		  org-cycle-show-empty-lines
		  org-optimize-window-after-visibility-change)
 org-todo-keywords '((sequence "TODO(t)" "SOMEDAY(s)" "|" "DONE(d!)")
		     (sequence "NEXT(n)" "WAIT(w@/!)" "|" 
		     "CANCELED(c@/!)"))
 org-speed-command-hook '(org-speed-command-activate
			  org-babel-speed-command-activate)
 org-export-before-parsing-hook '(org-attach-expand-links)
 org-confirm-shell-link-function 'yes-or-no-p
 org-link-parameters '(("attachment" :follow org-attach-follow :complete
			org-attach-complete-link)
		       ("id" :follow org-id-open)
		       ("eww" :follow org-eww-open :store 
		       org-eww-store-link)
		       ("rmail" :follow org-rmail-open :store
			org-rmail-store-link)
		       ("mhe" :follow org-mhe-open :store 
		       org-mhe-store-link)
		       ("irc" :follow org-irc-visit :store 
		       org-irc-store-link
			:export org-irc-export)
		       ("info" :follow org-info-open :export 
		       org-info-export
			:store org-info-store-link)
		       ("gnus" :follow org-gnus-open :store
			org-gnus-store-link)
		       ("docview" :follow org-docview-open :export
			org-docview-export :store 
			org-docview-store-link)
		       ("bibtex" :follow org-bibtex-open :store
			org-bibtex-store-link)
		       ("bbdb" :follow org-bbdb-open :export 
		       org-bbdb-export
			:complete org-bbdb-complete-link :store
			org-bbdb-store-link)
		       ("w3m" :store org-w3m-store-link) ("file+sys")
		       ("file+emacs") ("shell" :follow 
		       org-link--open-shell)
		       ("news" :follow
			#[514 "\301\300\302Q\"\207"
			  ["news" browse-url ":"] 6 "\n\n(fn URL ARG)"]
			)
		       ("mailto" :follow
			#[514 "\301\300\302Q\"\207"
			  ["mailto" browse-url ":"] 6 "\n\n(fn URL 
			  ARG)"]
			)
		       ("https" :follow
			#[514 "\301\300\302Q\"\207"
			  ["https" browse-url ":"] 6 "\n\n(fn URL ARG)"]
			)
		       ("http" :follow
			#[514 "\301\300\302Q\"\207"
			  ["http" browse-url ":"] 6 "\n\n(fn URL ARG)"]
			)
		       ("ftp" :follow
			#[514 "\301\300\302Q\"\207" ["ftp" browse-url 
                         ":"]
			  6 "\n\n(fn URL ARG)"]
			)
		       ("help" :follow org-link--open-help)
		       ("file" :complete org-link-complete-file)
		       ("elisp" :follow org-link--open-elisp)
		       ("doi" :follow org-link--open-doi))
 org-link-elisp-confirm-function 'yes-or-no-p
 )


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

* Re: Bug: Duplicate logbook entry for repeated tasks [9.4.6 (9.4.6-gab9f2a @ /home/gustavo/.emacs.d/elpa/org-9.4.6/)]
  2021-06-14 13:39 Bug: Duplicate logbook entry for repeated tasks [9.4.6 (9.4.6-gab9f2a @ /home/gustavo/.emacs.d/elpa/org-9.4.6/)] Gustavo Barros
@ 2021-06-14 18:01 ` Bhavin Gandhi
  2021-06-14 18:41   ` Gustavo Barros
  2021-07-10 13:48   ` [PATCH] " Ihor Radchenko
  0 siblings, 2 replies; 18+ messages in thread
From: Bhavin Gandhi @ 2021-06-14 18:01 UTC (permalink / raw)
  To: Gustavo Barros; +Cc: emacs-orgmode

On Mon, 14 Jun 2021 at 19:10, Gustavo Barros wrote:
> The marking of repeated tasks as "done" is currently resulting in
> duplicate entries in the "LOGBOOK" drawer, which is not expected.  I
> don't know exactly when this came to be, but it does not happen in the
> current built-in version (9.4.4), while it does in the latest release
> (9.4.6).

I was able to reproduce this, and here are my findings as well as a
reproducible configuration with only a few settings.

=test.org= file:

#+begin_src org
* TODO First

* TODO Read book
SCHEDULED: <2021-06-15 Tue +1d>
#+end_src

Start emacs -Q and open the test.org file.

Go to the 'First' entry, and do C-c C-t. This adds an a timestamp record
(this does not happen with 9.4.4, not sure if the default behavior has
been changed).

Changed test.org file:

#+begin_src org
* DONE First

- State "DONE"       from "TODO"       [2021-06-14 Mon 23:23]
* TODO Read book
SCHEDULED: <2021-06-15 Tue +1d>
#+end_src

Now, load this configuration:

#+begin_src elisp
(add-to-list 'load-path "~/src/org-mode/lisp/")

(setq org-todo-keywords
      '((sequence "TODO(t)" "|" "DONE(d!)")))
#+end_src

Open the test.org again, and go to the "Read book" entry, mark it done
with C-c C-t. Now, there are two timestamp changes getting recorded.

#+begin_src org
[…]
* TODO Read book
SCHEDULED: <2021-06-16 Wed +1d>
:PROPERTIES:
:LAST_REPEAT: [2021-06-14 Mon 23:26]
:END:
- State "DONE"       from "TODO"       [2021-06-14 Mon 23:26]
- State "DONE"       from "TODO"       [2021-06-14 Mon 23:26]
#+end_src

-- 
Bhavin Gandhi (bhavin192) | https://geeksocket.in


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

* Re: Bug: Duplicate logbook entry for repeated tasks [9.4.6 (9.4.6-gab9f2a @ /home/gustavo/.emacs.d/elpa/org-9.4.6/)]
  2021-06-14 18:01 ` Bhavin Gandhi
@ 2021-06-14 18:41   ` Gustavo Barros
  2021-07-10 13:48   ` [PATCH] " Ihor Radchenko
  1 sibling, 0 replies; 18+ messages in thread
From: Gustavo Barros @ 2021-06-14 18:41 UTC (permalink / raw)
  To: Bhavin Gandhi; +Cc: emacs-orgmode

Hi Bhavin,

On Mon, 14 Jun 2021 at 15:01, Bhavin Gandhi <bhavin7392@gmail.com> 
wrote:

> On Mon, 14 Jun 2021 at 19:10, Gustavo Barros wrote:
>> The marking of repeated tasks as "done" is currently resulting in
>> duplicate entries in the "LOGBOOK" drawer, which is not expected.  I
>> don't know exactly when this came to be, but it does not happen in 
>> the
>> current built-in version (9.4.4), while it does in the latest release
>> (9.4.6).
>
> I was able to reproduce this, and here are my findings as well as a
> reproducible configuration with only a few settings.

Thank you for taking the time to try this out and for confirming you are 
able to reproduce the issue.

(I'm marking this bug confirmed.  I'd normally refrain from "self 
confirming" but, since Bhavin was able to reproduce and I consider this 
one to be particularly relevant, I made an exception.)

Best regards,
Gustavo.


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

* [PATCH] Re: Bug: Duplicate logbook entry for repeated tasks [9.4.6 (9.4.6-gab9f2a @ /home/gustavo/.emacs.d/elpa/org-9.4.6/)]
  2021-06-14 18:01 ` Bhavin Gandhi
  2021-06-14 18:41   ` Gustavo Barros
@ 2021-07-10 13:48   ` Ihor Radchenko
  2021-07-10 16:46     ` Gustavo Barros
                       ` (2 more replies)
  1 sibling, 3 replies; 18+ messages in thread
From: Ihor Radchenko @ 2021-07-10 13:48 UTC (permalink / raw)
  To: Bhavin Gandhi; +Cc: emacs-orgmode, Gustavo Barros

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

Bhavin Gandhi <bhavin7392@gmail.com> writes:

> I was able to reproduce this, and here are my findings as well as a
> reproducible configuration with only a few settings.

The breakage was introduced in commit c67037:

[c670379adfbdc4883d3cfa230289fd2829993265] Fix `org-agenda-todo' undo behavior when logging (not adding note)

The fix is attached.

Best,
Ihor


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-duplicate-logbook-entry-for-repeated-tasks.patch --]
[-- Type: text/x-diff, Size: 1353 bytes --]

From ff3c0f6524d4165518ec0c53b49a58162ff7b2a9 Mon Sep 17 00:00:00 2001
Message-Id: <ff3c0f6524d4165518ec0c53b49a58162ff7b2a9.1625924841.git.yantar92@gmail.com>
From: Ihor Radchenko <yantar92@gmail.com>
Date: Sat, 10 Jul 2021 21:43:44 +0800
Subject: [PATCH] Fix duplicate logbook entry for repeated tasks

* lisp/org.el (org-add-log-setup): Always run `org-add-log-note' via
`post-command-hook'.  Otherwise, there is no way to know if a note was
requested for `this-command'.  Running `org-add-log-note' directly
would, for example, break `org-auto-repeat-maybe' as reported in [1].

[1] https://orgmode.org/list/CAOn=hbcaW1R6vtun-E2r4LS=j3dp=VjqmjGtzy8UC1SyPArKbA@mail.gmail.com
---
 lisp/org.el | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index ffcc5945d..3d15771a2 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -10939,9 +10939,7 @@ (defun org-add-log-setup (&optional purpose state prev-state how extra)
 	org-log-note-extra extra
 	org-log-note-effective-time (org-current-effective-time)
         org-log-setup t)
-  (if (eq how 'note)
-      (add-hook 'post-command-hook 'org-add-log-note 'append)
-    (org-add-log-note purpose)))
+  (add-hook 'post-command-hook 'org-add-log-note 'append))
 
 (defun org-skip-over-state-notes ()
   "Skip past the list of State notes in an entry."
-- 
2.31.1


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

* Re: [PATCH] Re: Bug: Duplicate logbook entry for repeated tasks [9.4.6 (9.4.6-gab9f2a @ /home/gustavo/.emacs.d/elpa/org-9.4.6/)]
  2021-07-10 13:48   ` [PATCH] " Ihor Radchenko
@ 2021-07-10 16:46     ` Gustavo Barros
  2021-07-10 17:14     ` Bhavin Gandhi
  2021-09-26  6:26     ` Bastien
  2 siblings, 0 replies; 18+ messages in thread
From: Gustavo Barros @ 2021-07-10 16:46 UTC (permalink / raw)
  To: Ihor Radchenko; +Cc: emacs-orgmode, Bhavin Gandhi

Hi Ihor,

On Sat, 10 Jul 2021 at 10:48, Ihor Radchenko <yantar92@gmail.com> wrote:

> The breakage was introduced in commit c67037:
>
> [c670379adfbdc4883d3cfa230289fd2829993265] Fix `org-agenda-todo' undo 
> behavior when logging (not adding note)
>
> The fix is attached.

Thank you very much!

Best,
Gustavo.


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

* Re: [PATCH] Re: Bug: Duplicate logbook entry for repeated tasks [9.4.6 (9.4.6-gab9f2a @ /home/gustavo/.emacs.d/elpa/org-9.4.6/)]
  2021-07-10 13:48   ` [PATCH] " Ihor Radchenko
  2021-07-10 16:46     ` Gustavo Barros
@ 2021-07-10 17:14     ` Bhavin Gandhi
  2021-07-11  1:29       ` Ihor Radchenko
  2021-09-26  6:26     ` Bastien
  2 siblings, 1 reply; 18+ messages in thread
From: Bhavin Gandhi @ 2021-07-10 17:14 UTC (permalink / raw)
  To: Ihor Radchenko; +Cc: emacs-orgmode, Gustavo Barros

Hello Ihor,

On Sat, 10 Jul 2021 at 19:18, Ihor Radchenko <yantar92@gmail.com> wrote:
> The breakage was introduced in commit c67037:
>
> [c670379adfbdc4883d3cfa230289fd2829993265] Fix `org-agenda-todo' undo behavior when logging (not adding note)
>
> The fix is attached.

Thank you! I tried the patch, but the original bug[1] which the commit
`c67037' tried to fix, gets introduced again. Basically,
`org-agenda-undo' just removes the logbook entry and the scheduled date
remains new.

[1] https://orgmode.org/list/87v98a8mes.fsf@gnu.org/


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

* Re: [PATCH] Re: Bug: Duplicate logbook entry for repeated tasks [9.4.6 (9.4.6-gab9f2a @ /home/gustavo/.emacs.d/elpa/org-9.4.6/)]
  2021-07-10 17:14     ` Bhavin Gandhi
@ 2021-07-11  1:29       ` Ihor Radchenko
  2021-07-11  5:40         ` Ihor Radchenko
  0 siblings, 1 reply; 18+ messages in thread
From: Ihor Radchenko @ 2021-07-11  1:29 UTC (permalink / raw)
  To: Bhavin Gandhi; +Cc: emacs-orgmode, Gustavo Barros

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

Bhavin Gandhi <bhavin7392@gmail.com> writes:

> Thank you! I tried the patch, but the original bug[1] which the commit
> `c67037' tried to fix, gets introduced again. Basically,
> `org-agenda-undo' just removes the logbook entry and the scheduled date
> remains new.

You are right. I believe that I fixed the breakage in the attached
patch. Also, I noticed that c67037 did not fix the original bug when
state change requests interactive note.

Best,
Ihor


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-duplicate-logbook-entry-for-repeated-tasks.patch --]
[-- Type: text/x-diff, Size: 2158 bytes --]

From f012648b350013e33ef0e88afc85b4fcf048734b Mon Sep 17 00:00:00 2001
Message-Id: <f012648b350013e33ef0e88afc85b4fcf048734b.1625966756.git.yantar92@gmail.com>
From: Ihor Radchenko <yantar92@gmail.com>
Date: Sat, 10 Jul 2021 21:43:44 +0800
Subject: [PATCH] Fix duplicate logbook entry for repeated tasks

* lisp/org.el (org-add-log-setup): Always run `org-add-log-note' via
`post-command-hook'.  Otherwise, there is no way to know if a note was
requested for `this-command'.  Running `org-add-log-note' directly
would, for example, break `org-auto-repeat-maybe' as reported in [1].

* lisp/org-agenda.el (org-agenda-todo): Avoid reintroducing the bug
fixed in c670379adf.

[1] https://orgmode.org/list/CAOn=hbcaW1R6vtun-E2r4LS=j3dp=VjqmjGtzy8UC1SyPArKbA@mail.gmail.com
---
 lisp/org-agenda.el | 6 +++++-
 lisp/org.el        | 4 +---
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/lisp/org-agenda.el b/lisp/org-agenda.el
index 44acd035a..4cd527e5b 100644
--- a/lisp/org-agenda.el
+++ b/lisp/org-agenda.el
@@ -9433,7 +9433,11 @@ (defun org-agenda-todo (&optional arg)
 	 (goto-char pos)
 	 (org-show-context 'agenda)
 	 (let ((current-prefix-arg arg))
-	   (call-interactively 'org-todo))
+	   (call-interactively 'org-todo)
+           ;; Make sure that log is recorded in current undo.
+           (when (and org-log-setup
+                      (not (eq org-log-note-how 'note)))
+             (org-add-log-note)))
 	 (and (bolp) (forward-char 1))
 	 (setq newhead (org-get-heading))
 	 (when (and org-agenda-headline-snapshot-before-repeat
diff --git a/lisp/org.el b/lisp/org.el
index ffcc5945d..3d15771a2 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -10939,9 +10939,7 @@ (defun org-add-log-setup (&optional purpose state prev-state how extra)
 	org-log-note-extra extra
 	org-log-note-effective-time (org-current-effective-time)
         org-log-setup t)
-  (if (eq how 'note)
-      (add-hook 'post-command-hook 'org-add-log-note 'append)
-    (org-add-log-note purpose)))
+  (add-hook 'post-command-hook 'org-add-log-note 'append))
 
 (defun org-skip-over-state-notes ()
   "Skip past the list of State notes in an entry."
-- 
2.31.1


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

* Re: [PATCH] Re: Bug: Duplicate logbook entry for repeated tasks [9.4.6 (9.4.6-gab9f2a @ /home/gustavo/.emacs.d/elpa/org-9.4.6/)]
  2021-07-11  1:29       ` Ihor Radchenko
@ 2021-07-11  5:40         ` Ihor Radchenko
  2021-07-11  5:57           ` Ihor Radchenko
  0 siblings, 1 reply; 18+ messages in thread
From: Ihor Radchenko @ 2021-07-11  5:40 UTC (permalink / raw)
  To: Bhavin Gandhi; +Cc: emacs-orgmode, Gustavo Barros

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

Updating the patch with relevant new test


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-duplicate-logbook-entry-for-repeated-tasks.patch --]
[-- Type: text/x-diff, Size: 3356 bytes --]

From 5b89c95318a38df9f0a6706b6eb5320baafbd4d8 Mon Sep 17 00:00:00 2001
Message-Id: <5b89c95318a38df9f0a6706b6eb5320baafbd4d8.1625981997.git.yantar92@gmail.com>
From: Ihor Radchenko <yantar92@gmail.com>
Date: Sat, 10 Jul 2021 21:43:44 +0800
Subject: [PATCH] Fix duplicate logbook entry for repeated tasks

* lisp/org.el (org-add-log-setup): Always run `org-add-log-note' via
`post-command-hook'.  Otherwise, there is no way to know if a note was
requested for `this-command'.  Running `org-add-log-note' directly
would, for example, break `org-auto-repeat-maybe' as reported in [1].

* lisp/org-agenda.el (org-agenda-todo): Avoid reintroducing the bug
fixed in c670379adf.

* testing/lisp/test-org.el: Add test checking the reported bug.

[1] https://orgmode.org/list/CAOn=hbcaW1R6vtun-E2r4LS=j3dp=VjqmjGtzy8UC1SyPArKbA@mail.gmail.com
---
 lisp/org-agenda.el       |  6 +++++-
 lisp/org.el              |  4 +---
 testing/lisp/test-org.el | 21 +++++++++++++++++++++
 3 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/lisp/org-agenda.el b/lisp/org-agenda.el
index 44acd035a..4cd527e5b 100644
--- a/lisp/org-agenda.el
+++ b/lisp/org-agenda.el
@@ -9433,7 +9433,11 @@ (defun org-agenda-todo (&optional arg)
 	 (goto-char pos)
 	 (org-show-context 'agenda)
 	 (let ((current-prefix-arg arg))
-	   (call-interactively 'org-todo))
+	   (call-interactively 'org-todo)
+           ;; Make sure that log is recorded in current undo.
+           (when (and org-log-setup
+                      (not (eq org-log-note-how 'note)))
+             (org-add-log-note)))
 	 (and (bolp) (forward-char 1))
 	 (setq newhead (org-get-heading))
 	 (when (and org-agenda-headline-snapshot-before-repeat
diff --git a/lisp/org.el b/lisp/org.el
index ffcc5945d..3d15771a2 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -10939,9 +10939,7 @@ (defun org-add-log-setup (&optional purpose state prev-state how extra)
 	org-log-note-extra extra
 	org-log-note-effective-time (org-current-effective-time)
         org-log-setup t)
-  (if (eq how 'note)
-      (add-hook 'post-command-hook 'org-add-log-note 'append)
-    (org-add-log-note purpose)))
+  (add-hook 'post-command-hook 'org-add-log-note 'append))
 
 (defun org-skip-over-state-notes ()
   "Skip past the list of State notes in an entry."
diff --git a/testing/lisp/test-org.el b/testing/lisp/test-org.el
index de3c6f3c9..0634ba608 100644
--- a/testing/lisp/test-org.el
+++ b/testing/lisp/test-org.el
@@ -7397,6 +7397,27 @@ (ert-deftest test-org/auto-repeat-maybe ()
 CLOCK: [2012-03-29 Thu 10:00]--[2012-03-29 Thu 16:40] =>  6:40"
 	(org-todo "DONE")
 	(buffer-string))))))
+  ;; Make sure that logbook state change record does not get
+  ;; duplicated when `org-log-repeat' `org-log-done' are non-nil.
+  (should
+   (string-match-p
+    (rx "* TODO Read book
+SCHEDULED: <2021-06-16 Wed +1d>
+:PROPERTIES:
+:LAST_REPEAT:" (1+ nonl) "
+:END:
+- State \"DONE\"       from \"TODO\"" (1+ nonl) buffer-end)
+    (let ((org-log-repeat 'time)
+	  (org-todo-keywords '((sequence "TODO" "|" "DONE(d!)")))
+          (org-log-into-drawer nil))
+      (org-test-with-temp-text
+          "* TODO Read book
+SCHEDULED: <2021-06-15 Tue +1d>"
+        (org-todo "DONE")
+        (when (memq 'org-add-log-note post-command-hook)
+          (org-add-log-note))
+        (buffer-string))))))
+
 
 \f
 ;;; Timestamps API
-- 
2.31.1


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

* Re: [PATCH] Re: Bug: Duplicate logbook entry for repeated tasks [9.4.6 (9.4.6-gab9f2a @ /home/gustavo/.emacs.d/elpa/org-9.4.6/)]
  2021-07-11  5:40         ` Ihor Radchenko
@ 2021-07-11  5:57           ` Ihor Radchenko
  2021-07-12 17:50             ` Bhavin Gandhi
  2021-09-25 15:25             ` Bastien
  0 siblings, 2 replies; 18+ messages in thread
From: Ihor Radchenko @ 2021-07-11  5:57 UTC (permalink / raw)
  To: Bhavin Gandhi; +Cc: emacs-orgmode, Gustavo Barros

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

And yet another update fixing a typo in previous patch. Sorry


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-duplicate-logbook-entry-for-repeated-tasks.patch --]
[-- Type: text/x-diff, Size: 3413 bytes --]

From 89a60d654663b68f1c149fb3341a50191027f45e Mon Sep 17 00:00:00 2001
Message-Id: <89a60d654663b68f1c149fb3341a50191027f45e.1625983004.git.yantar92@gmail.com>
From: Ihor Radchenko <yantar92@gmail.com>
Date: Sat, 10 Jul 2021 21:43:44 +0800
Subject: [PATCH] Fix duplicate logbook entry for repeated tasks

* lisp/org.el (org-add-log-setup): Always run `org-add-log-note' via
`post-command-hook'.  Otherwise, there is no way to know if a note was
requested for `this-command'.  Running `org-add-log-note' directly
would, for example, break `org-auto-repeat-maybe' as reported in [1].

* lisp/org-agenda.el (org-agenda-todo): Avoid reintroducing the bug
fixed in c670379adf.

* testing/lisp/test-org.el: Add test checking the reported bug.

[1] https://orgmode.org/list/CAOn=hbcaW1R6vtun-E2r4LS=j3dp=VjqmjGtzy8UC1SyPArKbA@mail.gmail.com
---
 lisp/org-agenda.el       |  6 +++++-
 lisp/org.el              |  4 +---
 testing/lisp/test-org.el | 23 ++++++++++++++++++++++-
 3 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/lisp/org-agenda.el b/lisp/org-agenda.el
index 44acd035a..4cd527e5b 100644
--- a/lisp/org-agenda.el
+++ b/lisp/org-agenda.el
@@ -9433,7 +9433,11 @@ (defun org-agenda-todo (&optional arg)
 	 (goto-char pos)
 	 (org-show-context 'agenda)
 	 (let ((current-prefix-arg arg))
-	   (call-interactively 'org-todo))
+	   (call-interactively 'org-todo)
+           ;; Make sure that log is recorded in current undo.
+           (when (and org-log-setup
+                      (not (eq org-log-note-how 'note)))
+             (org-add-log-note)))
 	 (and (bolp) (forward-char 1))
 	 (setq newhead (org-get-heading))
 	 (when (and org-agenda-headline-snapshot-before-repeat
diff --git a/lisp/org.el b/lisp/org.el
index ffcc5945d..3d15771a2 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -10939,9 +10939,7 @@ (defun org-add-log-setup (&optional purpose state prev-state how extra)
 	org-log-note-extra extra
 	org-log-note-effective-time (org-current-effective-time)
         org-log-setup t)
-  (if (eq how 'note)
-      (add-hook 'post-command-hook 'org-add-log-note 'append)
-    (org-add-log-note purpose)))
+  (add-hook 'post-command-hook 'org-add-log-note 'append))
 
 (defun org-skip-over-state-notes ()
   "Skip past the list of State notes in an entry."
diff --git a/testing/lisp/test-org.el b/testing/lisp/test-org.el
index de3c6f3c9..c2267c32d 100644
--- a/testing/lisp/test-org.el
+++ b/testing/lisp/test-org.el
@@ -7396,7 +7396,28 @@ (ert-deftest test-org/auto-repeat-maybe ()
 SCHEDULED: <2012-03-29 Thu +2y>
 CLOCK: [2012-03-29 Thu 10:00]--[2012-03-29 Thu 16:40] =>  6:40"
 	(org-todo "DONE")
-	(buffer-string))))))
+	(buffer-string)))))
+  ;; Make sure that logbook state change record does not get
+  ;; duplicated when `org-log-repeat' `org-log-done' are non-nil.
+  (should
+   (string-match-p
+    (rx "* TODO Read book
+SCHEDULED: <2021-06-16 Wed +1d>
+:PROPERTIES:
+:LAST_REPEAT:" (1+ nonl) "
+:END:
+- State \"DONE\"       from \"TODO\"" (1+ nonl) buffer-end)
+    (let ((org-log-repeat 'time)
+	  (org-todo-keywords '((sequence "TODO" "|" "DONE(d!)")))
+          (org-log-into-drawer nil))
+      (org-test-with-temp-text
+          "* TODO Read book
+SCHEDULED: <2021-06-15 Tue +1d>"
+        (org-todo "DONE")
+        (when (memq 'org-add-log-note post-command-hook)
+          (org-add-log-note))
+        (buffer-string))))))
+
 
 \f
 ;;; Timestamps API
-- 
2.31.1


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

* Re: [PATCH] Re: Bug: Duplicate logbook entry for repeated tasks [9.4.6 (9.4.6-gab9f2a @ /home/gustavo/.emacs.d/elpa/org-9.4.6/)]
  2021-07-11  5:57           ` Ihor Radchenko
@ 2021-07-12 17:50             ` Bhavin Gandhi
  2021-07-13  2:55               ` [PATCH] " Carlo Tambuatco
  2021-07-17 13:41               ` [PATCH] " Ihor Radchenko
  2021-09-25 15:25             ` Bastien
  1 sibling, 2 replies; 18+ messages in thread
From: Bhavin Gandhi @ 2021-07-12 17:50 UTC (permalink / raw)
  To: Ihor Radchenko; +Cc: emacs-orgmode, Gustavo Barros

On Sun, 11 Jul 2021 at 06:59, Ihor Radchenko <yantar92@gmail.com> wrote:
> You are right. I believe that I fixed the breakage in the attached
> patch. Also, I noticed that c67037 did not fix the original bug when
> state change requests interactive note.

Thanks! I tested your latest patch, and it is fixing both the original
issues which `c67037' solved as well as this one.

I was trying to understand your change. So, when we call
`org-agenda-todo', it calls `org-todo' which adds the
post-command-hook. This hook is supposed to run when `org-agenda-todo'
finishes, but instead of that we call it directly. This makes sure that
the change is recorded in `buffer-undo-list'.

Sorry if that's too much to ask, but why don't we need something similar
when org-log-note-how is 'note?  Can you please help me understand that?
I tried reading org-add-log-note and org-store-log-note, but I think I'm
missing something basic here.

Also, should this line from org.el (org-store-log-note) be removed?

  ;; Don't add undo information when called from `org-agenda-todo'.


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

* Re: [PATCH] Bug: Duplicate logbook entry for repeated tasks [9.4.6 (9.4.6-gab9f2a @ /home/gustavo/.emacs.d/elpa/org-9.4.6/)]
  2021-07-12 17:50             ` Bhavin Gandhi
@ 2021-07-13  2:55               ` Carlo Tambuatco
  2021-07-13 15:35                 ` Alan Ristow
  2021-07-17 13:41               ` [PATCH] " Ihor Radchenko
  1 sibling, 1 reply; 18+ messages in thread
From: Carlo Tambuatco @ 2021-07-13  2:55 UTC (permalink / raw)
  To: Bhavin Gandhi; +Cc: Org-Mode Mailing List, Ihor Radchenko, Gustavo Barros

Newbie question: what command do I use to apply the patch…?


> On Jul 12, 2021, at 1:50 PM, Bhavin Gandhi <bhavin7392@gmail.com> wrote:
> 
> On Sun, 11 Jul 2021 at 06:59, Ihor Radchenko <yantar92@gmail.com> wrote:
>> You are right. I believe that I fixed the breakage in the attached
>> patch. Also, I noticed that c67037 did not fix the original bug when
>> state change requests interactive note.
> 
> Thanks! I tested your latest patch, and it is fixing both the original
> issues which `c67037' solved as well as this one.
> 
> I was trying to understand your change. So, when we call
> `org-agenda-todo', it calls `org-todo' which adds the
> post-command-hook. This hook is supposed to run when `org-agenda-todo'
> finishes, but instead of that we call it directly. This makes sure that
> the change is recorded in `buffer-undo-list'.
> 
> Sorry if that's too much to ask, but why don't we need something similar
> when org-log-note-how is 'note?  Can you please help me understand that?
> I tried reading org-add-log-note and org-store-log-note, but I think I'm
> missing something basic here.
> 
> Also, should this line from org.el (org-store-log-note) be removed?
> 
>  ;; Don't add undo information when called from `org-agenda-todo'.
> 



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

* Re: [PATCH] Bug: Duplicate logbook entry for repeated tasks [9.4.6 (9.4.6-gab9f2a @ /home/gustavo/.emacs.d/elpa/org-9.4.6/)]
  2021-07-13  2:55               ` [PATCH] " Carlo Tambuatco
@ 2021-07-13 15:35                 ` Alan Ristow
  2021-07-13 18:08                   ` Bhavin Gandhi
  2021-07-17 13:51                   ` Ihor Radchenko
  0 siblings, 2 replies; 18+ messages in thread
From: Alan Ristow @ 2021-07-13 15:35 UTC (permalink / raw)
  To: Carlo Tambuatco, Bhavin Gandhi
  Cc: Org-Mode Mailing List, Ihor Radchenko, Gustavo Barros

On 7/13/21 4:55 AM, Carlo Tambuatco wrote:
> Newbie question: what command do I use to apply the patch…?

I can tell you what worked for me. I think you probably need to have 
cloned org using git for it to work, but I am far from being a git wiz 
so there could be a workaround I don't know about in the case that you 
have it installed by a more conventional method.

First, I saved the patch file to the same directory where I have org 
cloned (for me, that is ~/.emacs.d/straight/repos/org). Then I opened a 
terminal and ran:

git apply --check 0001-Fix-duplicate-logbook-entry-for-repeated-tasks.patch

to do a dry run and make sure I had saved the patch file in the right 
place that "git apply" would run correctly. It returned nothing, which 
is good, so then I ran:

git apply --check 0001-Fix-duplicate-logbook-entry-for-repeated-tasks.patch

to apply the patch. Since I'm using straight.el as my package manager, I 
then opened Emacs and ran "M-x straight-rebuild-all".

Also, thanks to Ihor and Bhavin for your work on this. Prior to applying 
the patch, I was noticing new bizarre behavior relating to recurring 
tasks. I haven't been running the patched org very long yet, but so far, 
so good.

Finally, as a newbie here myself, I have a naïve question of my own: 
Provided the patch proves successful, how/when is it applied to the 
official org repo? Is it up to Ihor (or somebody else) to make a pull 
request, for example?

Best regards,

Alan



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

* Re: [PATCH] Bug: Duplicate logbook entry for repeated tasks [9.4.6 (9.4.6-gab9f2a @ /home/gustavo/.emacs.d/elpa/org-9.4.6/)]
  2021-07-13 15:35                 ` Alan Ristow
@ 2021-07-13 18:08                   ` Bhavin Gandhi
  2021-07-17 13:51                   ` Ihor Radchenko
  1 sibling, 0 replies; 18+ messages in thread
From: Bhavin Gandhi @ 2021-07-13 18:08 UTC (permalink / raw)
  To: Alan Ristow; +Cc: Org-Mode Mailing List

On Tue, 13 Jul 2021 at 21:06, Alan Ristow <alan@ristow.info> wrote:
>
> Also, thanks to Ihor and Bhavin for your work on this. Prior to applying
> the patch, I was noticing new bizarre behavior relating to recurring
> tasks. I haven't been running the patched org very long yet, but so far,
> so good.

Thank you for confirming that it is working correctly for you.

> Finally, as a newbie here myself, I have a naïve question of my own:
> Provided the patch proves successful, how/when is it applied to the
> official org repo? Is it up to Ihor (or somebody else) to make a pull
> request, for example?

From my very little experience here, the patch will get reviewed by
someone, and one of the maintainers (one with committer access) will
merge/apply it in a few days. It might take more time as well.

https://orgmode.org/worg/org-contribute.html#what-can-I-expect

--
Bhavin Gandhi (bhavin192) | https://geeksocket.in


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

* Re: [PATCH] Re: Bug: Duplicate logbook entry for repeated tasks [9.4.6 (9.4.6-gab9f2a @ /home/gustavo/.emacs.d/elpa/org-9.4.6/)]
  2021-07-12 17:50             ` Bhavin Gandhi
  2021-07-13  2:55               ` [PATCH] " Carlo Tambuatco
@ 2021-07-17 13:41               ` Ihor Radchenko
  1 sibling, 0 replies; 18+ messages in thread
From: Ihor Radchenko @ 2021-07-17 13:41 UTC (permalink / raw)
  To: Bhavin Gandhi; +Cc: emacs-orgmode, Gustavo Barros

Bhavin Gandhi <bhavin7392@gmail.com> writes:

> I was trying to understand your change. So, when we call
> `org-agenda-todo', it calls `org-todo' which adds the
> post-command-hook. This hook is supposed to run when `org-agenda-todo'
> finishes, but instead of that we call it directly. This makes sure that
> the change is recorded in `buffer-undo-list'.

You are almost correct. To be able to use undo from agenda, we must have
all the changes happen inside org-with-remote-undo body. Only then the
changes are recorded into agenda's buffer `buffer-undo-list'.
post-command-hook, if ran after `org-agenda-todo', will only record
changes in the actual org buffer's `buffer-undo-list', but not inside
the agenda's `buffer-undo-list'.

> Sorry if that's too much to ask, but why don't we need something similar
> when org-log-note-how is 'note?  Can you please help me understand that?
> I tried reading org-add-log-note and org-store-log-note, but I think I'm
> missing something basic here.

AFAIK, it is quite hard to do with current log note implementation.
`org-add-log-note' itself does not record the note text. Instead, it
only creates and pre-populates the note buffer and returns the control
to the function calling `org-add-log-note'. Regardless where we call
`org-add-log-note', the actual note text will only be added to the org
buffer when the user presses C-c C-c in the note buffer. And the user
input will only be possible after the current command (in our case
`org-agenda-todo') finishes. Thus, user note will always be added
outside `org-with-remote-undo' and cannot be recorded by agenda.

IMHO, the proper way to handle this would be rewriting the log-note code
using recursive editing. But that's not a trivial change and should be
implemented in a separate patch.

> Also, should this line from org.el (org-store-log-note) be removed?
>
>   ;; Don't add undo information when called from `org-agenda-todo'.

I think so. It appears to be irrelevant to current state of the code.
Someone forgot to remove this comment in one of the past patches. But
removing the comment should be a separate patch itself.

Best,
Ihor


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

* Re: [PATCH] Bug: Duplicate logbook entry for repeated tasks [9.4.6 (9.4.6-gab9f2a @ /home/gustavo/.emacs.d/elpa/org-9.4.6/)]
  2021-07-13 15:35                 ` Alan Ristow
  2021-07-13 18:08                   ` Bhavin Gandhi
@ 2021-07-17 13:51                   ` Ihor Radchenko
  1 sibling, 0 replies; 18+ messages in thread
From: Ihor Radchenko @ 2021-07-17 13:51 UTC (permalink / raw)
  To: Alan Ristow
  Cc: Carlo Tambuatco, Org-Mode Mailing List, Gustavo Barros,
	Bhavin Gandhi

Alan Ristow <alan@ristow.info> writes:

> Finally, as a newbie here myself, I have a naïve question of my own: 
> Provided the patch proves successful, how/when is it applied to the 
> official org repo? Is it up to Ihor (or somebody else) to make a pull 
> request, for example?

An email with [PATCH] in header is the pull request for Org :) You can
see a list of patches, confirmed bugs, and help requests in
https://updates.orgmode.org/

The emails replying to the patch email are the comments to the pull
request.

The merge is up to Org maintainers. They patches are reviewed and merged
as the maintainers get some free time to do it.

Best,
Ihor


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

* Re: [PATCH] Re: Bug: Duplicate logbook entry for repeated tasks [9.4.6 (9.4.6-gab9f2a @ /home/gustavo/.emacs.d/elpa/org-9.4.6/)]
  2021-07-11  5:57           ` Ihor Radchenko
  2021-07-12 17:50             ` Bhavin Gandhi
@ 2021-09-25 15:25             ` Bastien
  2021-09-25 18:15               ` Gustavo Barros
  1 sibling, 1 reply; 18+ messages in thread
From: Bastien @ 2021-09-25 15:25 UTC (permalink / raw)
  To: Ihor Radchenko; +Cc: emacs-orgmode, Gustavo Barros, Bhavin Gandhi

Ihor Radchenko <yantar92@gmail.com> writes:

> And yet another update fixing a typo in previous patch.

Applied, thanks!


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

* Re: [PATCH] Re: Bug: Duplicate logbook entry for repeated tasks [9.4.6 (9.4.6-gab9f2a @ /home/gustavo/.emacs.d/elpa/org-9.4.6/)]
  2021-09-25 15:25             ` Bastien
@ 2021-09-25 18:15               ` Gustavo Barros
  0 siblings, 0 replies; 18+ messages in thread
From: Gustavo Barros @ 2021-09-25 18:15 UTC (permalink / raw)
  To: Bastien; +Cc: emacs-orgmode, Ihor Radchenko, Bhavin Gandhi

Hi Bastien,

On Sat, 25 Sep 2021 at 17:25, Bastien <bzg@gnu.org> wrote:

> Ihor Radchenko <yantar92@gmail.com> writes:
>
>> And yet another update fixing a typo in previous patch.
>
> Applied, thanks!

Thank you! Ihor, thank you for the patch! And thanks to all who chimed 
in.

Best,
Gustavo.


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

* Re: [PATCH] Re: Bug: Duplicate logbook entry for repeated tasks [9.4.6 (9.4.6-gab9f2a @ /home/gustavo/.emacs.d/elpa/org-9.4.6/)]
  2021-07-10 13:48   ` [PATCH] " Ihor Radchenko
  2021-07-10 16:46     ` Gustavo Barros
  2021-07-10 17:14     ` Bhavin Gandhi
@ 2021-09-26  6:26     ` Bastien
  2 siblings, 0 replies; 18+ messages in thread
From: Bastien @ 2021-09-26  6:26 UTC (permalink / raw)
  To: Ihor Radchenko; +Cc: emacs-orgmode, Gustavo Barros, Bhavin Gandhi

Hi Ihor,

Ihor Radchenko <yantar92@gmail.com> writes:

> Bhavin Gandhi <bhavin7392@gmail.com> writes:
>
>> I was able to reproduce this, and here are my findings as well as a
>> reproducible configuration with only a few settings.
>
> The breakage was introduced in commit c67037:
>
> [c670379adfbdc4883d3cfa230289fd2829993265] Fix `org-agenda-todo' undo behavior when logging (not adding note)
>
> The fix is attached.

Applied in the bugfix branch, thanks.  This area is quite fragile, so
please let's test this heavily.

Thanks!

-- 
 Bastien


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

end of thread, other threads:[~2021-09-26  6:27 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-14 13:39 Bug: Duplicate logbook entry for repeated tasks [9.4.6 (9.4.6-gab9f2a @ /home/gustavo/.emacs.d/elpa/org-9.4.6/)] Gustavo Barros
2021-06-14 18:01 ` Bhavin Gandhi
2021-06-14 18:41   ` Gustavo Barros
2021-07-10 13:48   ` [PATCH] " Ihor Radchenko
2021-07-10 16:46     ` Gustavo Barros
2021-07-10 17:14     ` Bhavin Gandhi
2021-07-11  1:29       ` Ihor Radchenko
2021-07-11  5:40         ` Ihor Radchenko
2021-07-11  5:57           ` Ihor Radchenko
2021-07-12 17:50             ` Bhavin Gandhi
2021-07-13  2:55               ` [PATCH] " Carlo Tambuatco
2021-07-13 15:35                 ` Alan Ristow
2021-07-13 18:08                   ` Bhavin Gandhi
2021-07-17 13:51                   ` Ihor Radchenko
2021-07-17 13:41               ` [PATCH] " Ihor Radchenko
2021-09-25 15:25             ` Bastien
2021-09-25 18:15               ` Gustavo Barros
2021-09-26  6:26     ` Bastien

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