org-roam icon indicating copy to clipboard operation
org-roam copied to clipboard

org-roam-capture doesn't prompt for time with `:time-prompt t`

Open RensOliemans opened this issue 1 year ago • 8 comments

Description

A capture template with :time-prompt t doesn't actually prompt for the time, instead just using the current time.

Steps to Reproduce

Execute this with emacs -Q:

(setq org-directory "~/testing"
      org-roam-directory org-directory)

(progn 
  (require 'package)
  (add-to-list 'package-archives
	       '("melpa" . "http://melpa.org/packages/") t)
  (package-initialize)
  (package-refresh-contents)
  (package-install 'org-roam))

(setq org-roam-directory (concat org-directory "/roam")
      org-roam-capture-templates
      '(("m" "Meeting" entry "* [#1] Meeting %T
%?"
	 :target (file+datetree "meetings.org" month)
	 :prepend t
	 :time-prompt t)))

(org-roam-capture)

Expected Results

I expect to get a prompt (:time-prompt t is set) for the time. This time should then place the entry in the correct datetree, as well as defining the time for the %T element.

Actual Results

The current time is used.

Environment

  • Emacs: GNU Emacs 29.4 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.43, cairo version 1.18.2)
  • Framework: N/A
  • Org: Org mode version 9.6.15 (release_9.6.15 @ /usr/share/emacs/29.4/lisp/org/)

RensOliemans avatar Feb 24 '25 12:02 RensOliemans

There is a bug in the code

(defun patch/org-roam-capture--setup-target-location ()
  "Initialize the buffer, and goto the location of the new capture.
Return the ID of the location."
  (let (p new-file-p)
    (pcase (org-roam-capture--get-target)
      (`(file ,path)
       (setq path (org-roam-capture--target-truepath path)
             new-file-p (org-roam-capture--new-file-p path))
       (when new-file-p (org-roam-capture--put :new-file path))
       (set-buffer (org-capture-target-buffer path))
       (widen)
       (setq p (goto-char (point-min))))
      (`(file+olp ,path ,olp)
       (setq path (org-roam-capture--target-truepath path)
             new-file-p (org-roam-capture--new-file-p path))
       (when new-file-p (org-roam-capture--put :new-file path))
       (set-buffer (org-capture-target-buffer path))
       (setq p (point-min))
       (let ((m (org-roam-capture-find-or-create-olp olp)))
         (goto-char m))
       (widen))
      (`(file+head ,path ,head)
       (setq path (org-roam-capture--target-truepath path)
             new-file-p (org-roam-capture--new-file-p path))
       (set-buffer (org-capture-target-buffer path))
       (when new-file-p
         (org-roam-capture--put :new-file path)
         (insert (org-roam-capture--fill-template head 'ensure-newline)))
       (widen)
       (setq p (goto-char (point-min))))
      (`(file+head+olp ,path ,head ,olp)
       (setq path (org-roam-capture--target-truepath path)
             new-file-p (org-roam-capture--new-file-p path))
       (set-buffer (org-capture-target-buffer path))
       (widen)
       (when new-file-p
         (org-roam-capture--put :new-file path)
         (insert (org-roam-capture--fill-template head 'ensure-newline)))
       (setq p (point-min))
       (let ((m (org-roam-capture-find-or-create-olp olp)))
         (goto-char m)))
      (`(file+datetree ,path ,tree-type)
       (setq path (org-roam-capture--target-truepath path))
       (require 'org-datetree)
       (widen)
       (set-buffer (org-capture-target-buffer path))
       (unless (file-exists-p path)
         (org-roam-capture--put :new-file path))
       (funcall
        (pcase tree-type
          (`week #'org-datetree-find-iso-week-create)
          (`month #'org-datetree-find-month-create)
          (_ #'org-datetree-find-date-create))
        (calendar-gregorian-from-absolute
         (cond
          (org-overriding-default-time
           ;; Use the overriding default time.
           (time-to-days org-overriding-default-time))
          ((org-capture-get :time-prompt)
           ;; Prompt for date.  Bind `org-end-time-was-given' so
           ;; that `org-read-date-analyze' handles the time range
           ;; case and returns `prompt-time' with the start value.
           (let* (;; (org-time-was-given nil)
                  (org-end-time-was-given nil)
                  (prompt-time (org-read-date
                                nil t nil "Date for tree entry:")))
             (org-capture-put
              :default-time
              (if (or org-time-was-given
                      (= (time-to-days prompt-time) (org-today)))
                  prompt-time
                ;; Use 00:00 when no time is given for another
                ;; date than today?
                (apply #'encode-time 0 0
                       org-extend-today-until
                       (cl-cdddr (decode-time prompt-time)))))
             (time-to-days prompt-time)))
	  ((org-capture-get :default-time)
           (time-to-days (org-capture-get :default-time)))
          (t
           ;; Current date, possibly corrected for late night
           ;; workers.
           (org-today)))))
       (setq p (point)))
      (`(node ,title-or-id)
       ;; first try to get ID, then try to get title/alias
       (let ((node (or (org-roam-node-from-id title-or-id)
                       (org-roam-node-from-title-or-alias title-or-id)
                       (user-error "No node with title or id \"%s\"" title-or-id))))
         (set-buffer (org-capture-target-buffer (org-roam-node-file node)))
         (goto-char (org-roam-node-point node))
         (setq p (org-roam-node-point node)))))
    ;; Setup `org-id' for the current capture target and return it back to the
    ;; caller.
    (save-excursion
      (goto-char p)
      (if-let ((id (org-entry-get p "ID")))
          (setf (org-roam-node-id org-roam-capture--node) id)
        (org-entry-put p "ID" (org-roam-node-id org-roam-capture--node)))
      (prog1
          (org-id-get)
        (run-hooks 'org-roam-capture-new-node-hook)))))

(advice-add 'org-roam-capture--setup-target-location :override #'patch/org-roam-capture--setup-target-location)

Use this for now. It should fix your issue.

akashpal-21 avatar Mar 14 '25 03:03 akashpal-21

Hmm, I tried executing your code (after changing the typo :overide to :override) with advice-add, which didn't change anything.

Alternatively, renaming your modified function to org-roam-capture--setup-target-location (the original name) and executing the defun macro (thereby (I think) overriding the original function) also didn't work.

"Didn't work" means here that there is no prompt for time, it just uses current time.

RensOliemans avatar Mar 20 '25 10:03 RensOliemans

Interesting, it is working for me. Apologies for the typo.

Question: In earlier case, there was no prompt to select date - is calender opening to select date?

Note - time has to be inserted here. There is no other seperate menu for selecting time

for example,

if you type 15 13:45 this will correspond to 2025-03-15 13:45 hours. Can you clarify if date selection is now working ?

akashpal-21 avatar Mar 20 '25 10:03 akashpal-21

Oops, apologies. I tested your patch on my regular install and nothing changed. I tried it now with emacs -Q and it works as expected, thanks!

I'll try to find out what's going on in my configuration that breaks this.

RensOliemans avatar Mar 20 '25 10:03 RensOliemans

Suggested patch for review:

--- org-roam-capture.el.orig	2025-03-20 16:27:45.269874044 +0530
+++ org-roam-capture.el	2025-03-20 16:23:57.374571775 +0530
@@ -536,14 +536,11 @@
           (org-overriding-default-time
            ;; Use the overriding default time.
            (time-to-days org-overriding-default-time))
-          ((org-capture-get :default-time)
-           (time-to-days (org-capture-get :default-time)))
           ((org-capture-get :time-prompt)
            ;; Prompt for date.  Bind `org-end-time-was-given' so
            ;; that `org-read-date-analyze' handles the time range
            ;; case and returns `prompt-time' with the start value.
-           (let* ((org-time-was-given nil)
-                  (org-end-time-was-given nil)
+           (let* ((org-end-time-was-given nil)
                   (prompt-time (org-read-date
                                 nil t nil "Date for tree entry:")))
              (org-capture-put
@@ -557,6 +554,8 @@
                        org-extend-today-until
                        (cl-cdddr (decode-time prompt-time)))))
              (time-to-days prompt-time)))
+	  ((org-capture-get :default-time)
+           (time-to-days (org-capture-get :default-time)))
           (t
            ;; Current date, possibly corrected for late night
            ;; workers.

  1. Increase priority of time-prompt in cond. Do not prioritise default-time.
  2. Do not set org-time-was-given to nil.
 (if (or org-time-was-given
                      (= (time-to-days prompt-time) (org-today)))
                  prompt-time
                ;; Use 00:00 when no time is given for another
                ;; date than today?
                (apply #'encode-time 0 0
                       org-extend-today-until
                       (cl-cdddr (decode-time prompt-time)))))

if value is set to nil - then user value is silently discarded.

akashpal-21 avatar Mar 20 '25 11:03 akashpal-21

I think I found the reason the behaviour differed. Below are two capture templates defined. The first one has as target (file+datetree ...) and the second one (file+head ...), so no datetree. However, both have the property :time-prompt t so I would expect it to prompt for both, but it only prompts for the first one.

Since the prompt in the first template influences both the datetree part and the %T part, I would expect the prompt to still occur and influence the %T part in the second template—this is also what Org does.

(setq org-roam-directory (concat org-directory "/roam")
      org-roam-capture-templates
      '(("m" "Meeting" entry "* [#1] Meeting %T
%?"
	 :target (file+datetree "meetings.org" month)
	 :prepend t
	 :time-prompt t)
	("t" "Test" entry "* [#1] Meeting %T
%?"
	 :target (file+head "meetings.org" "abc")
	 :prepend t
	 :time-prompt t)))

RensOliemans avatar Mar 20 '25 11:03 RensOliemans

@RensOliemans It is not currently implemented, time-prompt only works with datetree. I fixed the bug wherein time-prompt did not even work with datetree as documented.

you can try doing something like

(setq org-roam-capture-templates '(("n" "neeting" entry "* Meeting <${time}>
%?"
				    :target (file+head "untitled/${slug}.org"
						       "#+title: ${title}\n\n")
					   :unnarrowed t)))

akashpal-21 avatar Mar 20 '25 11:03 akashpal-21

It is not currently implemented, time-prompt only works with datetree

Thanks for the information, I didn't know that. The <${time}> is a nice workaround, but Orgs datepicker is so beautiful that it feels like a shame to not use it.

Anyway, it seems like this issue is still relevant for the datetree case, shall I create a new issue/feature request for adding time-prompt support for datetime prompts such as %T? In the meantime I'm using plain Org capture templates and creating nodes with M-x org-id-get-create, which works fine for me.

RensOliemans avatar Mar 20 '25 16:03 RensOliemans