[PATCH] Add org-after-note-stored-hook

[PATCH] Add org-after-note-stored-hook

[Joris Caravati via General discussions about Org-mode.]

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


Hello,

I've been archiving tasks automatically using
`org-after-todo-state-change-hook' but I've been recently bit with a
note (entering a todo state configured with '@') being placed where the
task was before its archival.

This patch aims to offer a way to defer the archival after the
note is stored. Actually, I am using it like this:

#+begin_src elisp
(add-to-list 'org-after-todo-state-change-hook
       (lambda ()
         ;; States configured without mandatory note
         (when (member org-state '("DONE"))
           (my/org-roam-archive-to-today))
         ;; States configured with mandatory note
         (when (member org-state '("CANCELLED" "READ"))
           (add-to-list 'org-after-note-stored-hook 'my/org-roam-archive-to-today))))
#+end_src

With `my/org-roam-archive-to-today' removing itself from
`org-after-note-stored-hook'.

Hopefully I did not miss an existing way to do this.

Regards,
Joris


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-org-after-note-stored-hook.patch --]
[-- Type: text/x-patch, Size: 2342 bytes --]

From 18de09a3aa08e3d06f180165530cbaeeccdf3ccf Mon Sep 17 00:00:00 2001
From: Joris Caravati <tyx@sula.io>
Date: Tue, 2 Jan 2024 22:50:32 +0100
Subject: [PATCH] lisp/org.el: Add `org-after-note-stored-hook'

* lisp/org.el: Add `org-after-note-stored-hook' which is called at the
end of the `org-store-log-note' function.
* etc/ORG-NEWS: Document the new hook.

This change allows customization after a note is taken. One case where
it is useful is when one wants to move a task after a state change but
cannot do so in `org-after-todo-state-change' because the new state is
configured to take a note (with '@' in `org-todo-keywords').

Setting this hook in `org-after-todo-state-change' allows to defer the
move after the note is taken and prevents the note to be placed where
the task was before being moved.

TINYCHANGE
---
 etc/ORG-NEWS | 4 ++++
 lisp/org.el  | 6 +++++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index c54473f55..911e8ffeb 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -742,6 +742,10 @@ Completion is enabled for links to man pages added using ~org-insert-link~:
 =C-c C-l man RET emacscl TAB= to get =emacsclient=.  Of course, the ~ol-man~
 library should be loaded first.
 
+*** New hook [[doc::org-after-note-stored-hook][org-after-note-stored-hook]]
+
+This new hook runs when a note has been stored.
+
 ** New functions and changes in function arguments
 *** ~org-fold-hide-drawer-all~ is now interactive
 
diff --git a/lisp/org.el b/lisp/org.el
index 6e6e075b4..fad21d8ba 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -1944,6 +1944,9 @@ Lisp variable `org-state'."
   :group 'org-todo
   :type 'hook)
 
+(defcustom org-after-note-stored-hook nil
+  "Hook which is run after a note was stored")
+
 (defvar org-blocker-hook nil
   "Hook for functions that are allowed to block a state change.
 
@@ -10729,7 +10732,8 @@ items are State notes."
   (with-current-buffer (marker-buffer org-log-note-return-to)
     (goto-char org-log-note-return-to))
   (move-marker org-log-note-return-to nil)
-  (when org-log-post-message (message "%s" org-log-post-message)))
+  (when org-log-post-message (message "%s" org-log-post-message))
+  (run-hooks 'org-after-note-stored-hook))
 
 (defun org-remove-empty-drawer-at (pos)
   "Remove an empty drawer at position POS.
-- 
2.38.5

Re: [PATCH] Add org-after-note-stored-hook

[Joris Caravati via General discussions about Org-mode.]

Hello,

Is there anything wrong with the patch or with the proposition of
adding a hook there? The first I'd like to know for doing better next
time (or maybe correcting this one), the second to know if  I should
stop using it.

Thanks!
Joris

Joris Caravati <tyx+org@sula.io> writes:

> Hello,
>
> I've been archiving tasks automatically using
> `org-after-todo-state-change-hook' but I've been recently bit with a
> note (entering a todo state configured with '@') being placed where the
> task was before its archival.
>
> This patch aims to offer a way to defer the archival after the
> note is stored. Actually, I am using it like this:
>
> #+begin_src elisp
> (add-to-list 'org-after-todo-state-change-hook
>        (lambda ()
>          ;; States configured without mandatory note
>          (when (member org-state '("DONE"))
>            (my/org-roam-archive-to-today))
>          ;; States configured with mandatory note
>          (when (member org-state '("CANCELLED" "READ"))
>            (add-to-list 'org-after-note-stored-hook 'my/org-roam-archive-to-today))))
> #+end_src
>
> With `my/org-roam-archive-to-today' removing itself from
> `org-after-note-stored-hook'.
>
> Hopefully I did not miss an existing way to do this.
>
> Regards,
> Joris
>
> [2. text/x-patch; 0001-Add-org-after-note-stored-hook.patch]...

Re: [PATCH] Add org-after-note-stored-hook

[Ihor Radchenko]

Joris Caravati via "General discussions about Org-mode."
<emacs-orgmode@gnu.org> writes:

> Is there anything wrong with the patch or with the proposition of
> adding a hook there? The first I'd like to know for doing better next
> time (or maybe correcting this one), the second to know if  I should
> stop using it.

It just went unnoticed.
Thanks for the bump!

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: [PATCH] Add org-after-note-stored-hook

[Ihor Radchenko]

Joris Caravati via "General discussions about Org-mode."
<emacs-orgmode@gnu.org> writes:

> I've been archiving tasks automatically using
> `org-after-todo-state-change-hook' but I've been recently bit with a
> note (entering a todo state configured with '@') being placed where the
> task was before its archival.
>
> This patch aims to offer a way to defer the archival after the
> note is stored. Actually, I am using it like this:
>
> #+begin_src elisp
> (add-to-list 'org-after-todo-state-change-hook
>        (lambda ()
>          ;; States configured without mandatory note
>          (when (member org-state '("DONE"))
>            (my/org-roam-archive-to-today))
>          ;; States configured with mandatory note
>          (when (member org-state '("CANCELLED" "READ"))
>            (add-to-list 'org-after-note-stored-hook 'my/org-roam-archive-to-today))))
> #+end_src
>
> With `my/org-roam-archive-to-today' removing itself from
> `org-after-note-stored-hook'.
>
> Hopefully I did not miss an existing way to do this.

An easier way would be forcing note earlier by calling
`org-add-log-note' from your hook.

Although, removing heading from inside
`org-after-todo-state-change-hook' is a bad idea - Org mode does not
expect the heading to disappear from under the cursor when this hook is
executed. I recommend using `org-trigger-hook' instead.

So, your patch does not seem necessary to achieve what you want.

That said, it does make sense to add a hook after storing a log note,
but not at the very end, when point may not be at the note stored:

>    (with-current-buffer (marker-buffer org-log-note-return-to)
>      (goto-char org-log-note-return-to))
>    (move-marker org-log-note-return-to nil)
> -  (when org-log-post-message (message "%s" org-log-post-message)))
> +  (when org-log-post-message (message "%s" org-log-post-message))
> +  (run-hooks 'org-after-note-stored-hook))

This will leave point in buffer where the note was invoked. For example,
in agenda buffer. I doubt that the users of this hook will expect such a
thing.

It will probably be better to run such new hooks right before (message "Note stored")
in `org-store-log-note'.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: [PATCH] Add org-after-note-stored-hook

[Joris Caravati via General discussions about Org-mode.]

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

Ihor Radchenko <yantar92@posteo.net> writes:
> Although, removing heading from inside
> `org-after-todo-state-change-hook' is a bad idea - Org mode does not
> expect the heading to disappear from under the cursor when this hook is
> executed. I recommend using `org-trigger-hook' instead.

Thanks for your help! I switched to `org-trigger-hook' without issue.

> An easier way would be forcing note earlier by calling
> `org-add-log-note' from your hook.

I remember trying that before adding the hook (and I tried again today)
but the problem with adding the note directly is that I only seem to
manage storing the note where the task was before being archived,
eg. with this (or any variants I could think of while making sense of
what note functions do):

#+begin_src elisp
  (when (member (plist-get properties ':to) '("CANCELLED" "READ"))
    (org-add-log-note)
    (my/org-roam-archive-to-today))
#+end_src

which I find logical, since `org-store-log-note' is only called after
=C-c C-c= is pressed, whereas the archival function is called just after
the note buffer is created. So I still struggle to see how I could do
without the hook (maybe if the archival function would return the
position of the task after moving it, but that seems more complicated
than just using the hook).

> It will probably be better to run such new hooks right before (message "Note stored")
> in `org-store-log-note'.

A patch modified to match the suggested location for the `run-hooks' is
attached.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-lisp-org.el-Add-org-after-note-stored-hook.patch --]
[-- Type: text/x-patch, Size: 2233 bytes --]

From 779d6b85acf9c30d7230bffccb2f98764372034a Mon Sep 17 00:00:00 2001
From: Joris Caravati <tyx@sula.io>
Date: Sun, 12 May 2024 21:29:52 +0200
Subject: [PATCH] lisp/org.el: Add `org-after-note-stored-hook'

* lisp/org.el: Add `org-after-note-stored-hook' which is called at the
end of the `org-store-log-note' function.
* etc/ORG-NEWS: Document the new hook.

This change allows customization after a note is taken. One case where
it is useful is when one wants to move a task after a state change but
cannot do so in `org-after-todo-state-change' because the new state is
configured to take a note (with '@' in `org-todo-keywords').

Setting this hook in `org-after-todo-state-change' allows to defer the
move after the note is taken and prevents the note to be placed where
the task was before being moved.

TINYCHANGE
---
 etc/ORG-NEWS | 4 ++++
 lisp/org.el  | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index 87b72ad12..4b7636765 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -1569,6 +1569,10 @@ optional argument =NEW-HEADING-CONTAINER= specifies where in the
 buffer it will be added.  If not specified, new headings are created
 at level 1 at the end of the accessible part of the buffer, as before.
 
+*** New hook [[doc::org-after-note-stored-hook][org-after-note-stored-hook]]
+
+This new hook runs when a note has been stored.
+
 ** Miscellaneous
 *** =org-crypt.el= now applies initial visibility settings to decrypted entries
 
diff --git a/lisp/org.el b/lisp/org.el
index 598b4ca23..64f6d07ee 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -1990,6 +1990,9 @@ Lisp variable `org-state'."
   :group 'org-todo
   :type 'hook)
 
+(defcustom org-after-note-stored-hook nil
+  "Hook which is run after a note was stored")
+
 (defvar org-blocker-hook nil
   "Hook for functions that are allowed to block a state change.
 
@@ -10845,6 +10848,7 @@ items are State notes."
                (unless (string-empty-p line)
 	         (indent-line-to ind)
 	         (insert-and-inherit line))))
+           (run-hooks 'org-after-note-stored-hook)
 	   (message "Note stored")
 	   (org-back-to-heading t))))))
   ;; Don't add undo information when called from `org-agenda-todo'.
-- 
2.44.0

Re: [PATCH] Add org-after-note-stored-hook

[Ihor Radchenko]

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

Joris Caravati <tyx+org@sula.io> writes:

>> An easier way would be forcing note earlier by calling
>> `org-add-log-note' from your hook.
>
> I remember trying that before adding the hook (and I tried again today)
> but the problem with adding the note directly is that I only seem to
> manage storing the note where the task was before being archived,
> eg. with this (or any variants I could think of while making sense of
> what note functions do):
>
> #+begin_src elisp
>   (when (member (plist-get properties ':to) '("CANCELLED" "READ"))
>     (org-add-log-note)
>     (my/org-roam-archive-to-today))
> #+end_src
>
> which I find logical, since `org-store-log-note' is only called after
> =C-c C-c= is pressed, whereas the archival function is called just after
> the note buffer is created. So I still struggle to see how I could do
> without the hook (maybe if the archival function would return the
> position of the task after moving it, but that seems more complicated
> than just using the hook).

Right. Yet, using a hook is still not ideal - what if you configure todo
keywords to not store note at all in future? Then, the hook will never
be executed or will be executed at the time you don't expect.

Note-taking mechanisms is generally tricky - it is schedule-based,
with note being taken after the current command is executed.

I think I know how to make it possible for the note to be taken at the
right place after subtree archival. May you try the attached patch?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org-save-markers-in-region-Store-log-note-marker.patch --]
[-- Type: text/x-patch, Size: 1561 bytes --]

From 2e24b634275c61ab913851b11d8b8f053f0f88f0 Mon Sep 17 00:00:00 2001
Message-ID: <2e24b634275c61ab913851b11d8b8f053f0f88f0.1716030861.git.yantar92@posteo.net>
From: Ihor Radchenko <yantar92@posteo.net>
Date: Sat, 18 May 2024 13:12:50 +0200
Subject: [PATCH] org-save-markers-in-region: Store log note marker

* lisp/org.el (org-save-markers-in-region): Store
`org-log-note-marker' when storing and reinstalling Org markers.

This change automatically handles note taken after refiling/archiving
without trying to place the note in place of the moved
subtree/heading.

Link: https://orgmode.org/list/m2plyjid8q.fsf@kyon.home
---
 lisp/org.el | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lisp/org.el b/lisp/org.el
index 8d921f11e..bec8a87a4 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -7365,6 +7365,7 @@ (defvar org-markers-to-move nil
 Those markers are stored together with their positions relative to
 the start of the region.")
 
+(defvar org-log-note-marker) ; defined later
 (defun org-save-markers-in-region (beg end)
   "Check markers in region.
 If these markers are between BEG and END, record their position relative
@@ -7374,6 +7375,7 @@ (defun org-save-markers-in-region (beg end)
 buffer.  After re-insertion, `org-reinstall-markers-in-region' must be
 called immediately, to move the markers with the entries."
   (setq org-markers-to-move nil)
+  (org-check-and-save-marker org-log-note-marker beg end)
   (when (featurep 'org-clock)
     (org-clock-save-markers-for-cut-and-paste beg end))
   (when (featurep 'org-agenda)
-- 
2.45.1


[-- Attachment #3: Type: text/plain, Size: 1096 bytes --]


>> It will probably be better to run such new hooks right before (message "Note stored")
>> in `org-store-log-note'.
>
> A patch modified to match the suggested location for the `run-hooks' is
> attached.

Thanks!
Applied, onto main, with amendments.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=f3e306c73
I added necessary keywords to the defcustom, changed the docstring to
state explicitly that the point is at the note taken when running a
hook, and changed the NEWS entry to (1) not have a link - we usually do
not do it; (2) moved it under appropriate heading.
I also removed your example from the commit message as it is not the
best illustration (see my considerations above).
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=f3e306c73

You are now listed among Org mode contributors.
https://git.sr.ht/~bzg/worg/commit/3093b569

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: [PATCH] Add org-after-note-stored-hook

[Joris Caravati via General discussions about Org-mode.]

Ihor Radchenko <yantar92@posteo.net> writes:

> Right. Yet, using a hook is still not ideal - what if you configure todo
> keywords to not store note at all in future? Then, the hook will never
> be executed or will be executed at the time you don't expect.
>
> Note-taking mechanisms is generally tricky - it is schedule-based,
> with note being taken after the current command is executed.
>
> I think I know how to make it possible for the note to be taken at the
> right place after subtree archival. May you try the attached patch?

Indeed, after testing it quickly it does what I want and notes are not a
special case in `org-trigger-hook' anymore which is perfect. Thank you!

> I added necessary keywords to the defcustom, changed the docstring to
> state explicitly that the point is at the note taken when running a
> hook, and changed the NEWS entry to (1) not have a link - we usually do
> not do it; (2) moved it under appropriate heading.
> I also removed your example from the commit message as it is not the
> best illustration (see my considerations above).
> https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=f3e306c73
>
> You are now listed among Org mode contributors.
> https://git.sr.ht/~bzg/worg/commit/3093b569

Thanks for your help!

Re: [PATCH] Add org-after-note-stored-hook

[Ihor Radchenko]

Joris Caravati <tyx+org@sula.io> writes:

> Ihor Radchenko <yantar92@posteo.net> writes:
>
>> I think I know how to make it possible for the note to be taken at the
>> right place after subtree archival. May you try the attached patch?
>
> Indeed, after testing it quickly it does what I want and notes are not a
> special case in `org-trigger-hook' anymore which is perfect. Thank you!

Applied, onto main.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=5fcd1d9de

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>