Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improvements to org-journal-new-scheduled-entry and org-journal-reschedule-scheduled-entry #420

Closed
wants to merge 3 commits into from

Conversation

TPKato
Copy link
Contributor

@TPKato TPKato commented Nov 5, 2023

This commit improves the behavior of org-journal-new-scheduled-entry and org-journal-reschedule-scheduled-entry.
More specifically,

  • It behaves more like org-schedule (with "SCHEDULED: ").
  • Inserting a time stamp <...> is now more consistent. (no meaningless 00:00, time range can also be specified.)
  • The time stamp (not the current time) is also added to a heading (in an org-mode way: TODO + time, not time + TODO).

Related issues/pull requests:

Description:

  • org-journal-new-scheduled-entry will insert followings if a time is not specified.

    M-x org-journal-new-scheduled-entry
    +1[RET] or S-<right>[RET]

    ** TODO _
    SCHEDULED: <2023-11-04 Sa>
    

    (_ shows a cursor position)

  • If you specify a time, org-journal-new-scheduled-entry inserts time in a heading and as a timestamp with "SCHEDULED: ".

    M-x org-journal-new-scheduled-entry
    23:59[RET]

    ** TODO 23:59 _			; Note that this is NOT the current time, but the scheduled time.
    SCHEDULED: <2023-11-03 Fr 23:59>
    

    You may think that the time stamp after "TODO" is unneeded/redandant, but it is useful when you use org-todo-list (C-c a t):

    20231104:   TODO 23:59 I must do this.
    
  • You can also specify time range.

    M-x org-journal-new-scheduled-entry
    22:00+1[RET]

    ** TODO 22:00 _
    SCHEDULED: <2023-11-04 Sa 22:00-23:00>
    
  • Use C-u to avoid inserting "TODO".

    C-u M-x org-journal-new-scheduled-entry
    23:59[RET]

    ** 23:59 _
    SCHEDULED: <2023-11-04 Sa 23:59>
    

Note that org-journal-reschedule-scheduled-entry now only changes a timestamp with "SCHEDULED: " (and the time in a heading). Bare timestamp <...> without "SCHEDULED: " remains untouched.

@bastibe
Copy link
Owner

bastibe commented Nov 16, 2023

From a quick glance, this looks reasonable. But I'm not using this feature (any more), so I can't comment on whether this breaks existing workflows or not.

@jmay
Copy link
Collaborator

jmay commented Nov 22, 2023

@TPKato Could you possibly write a test case for this change? Maybe duplicate org-journal-scheduled-weekly-test and add something that verifies the new behavior.

@TPKato
Copy link
Contributor Author

TPKato commented Nov 26, 2023

@jmay
I tried to write the tests, but I'm not sure if I wrote them correctly because this is my first time writing tests with ERT.
Feel free to modify or completely rewrite them ;-)
(And the test for org-journal-reschedule-scheduled-entry is very minimal.)

@TPKato
Copy link
Contributor Author

TPKato commented Dec 4, 2023

If we want to, we could apply the following additional patch to my pull request and introduce a new variable to make it less breaking (though I personally don't think this is necessary). Even with the following patch, it is still breaking, but for example * 20:00 TODO* TODO 20:00, or <2023-11-03 Fr 00:00><2023-11-03 Fr> (removing the unnecessary 00:00) would be considered bug fixes.

With this patch, the string "SCHEDULED:" will be inserted only if the user has set the variable (*1). If we consider it as an enhancement, this patch might be reasonable.

(*1)
To make my PR less breaking, I've set the default to "", but I strongly believe it's better to set it to org-scheduled-string for the compatibility with org-mode. In this case, we can write something like this in CHANGELOG or README:

If you want to avoid inserting "SCHEDULED:" before the timestamp (like the old version of org-journal), you can set the value of org-journal-schdeuled-string to "" in your setup:
(setq org-journal-schdeuled-string "")

(Sorry, the patch was incomplete and has been edited.)

--- a/org-journal.el
+++ b/org-journal.el
@@ -394,6 +394,10 @@ This prefix key is used for:
 - `org-journal-search' (key \"s\")"
   :type 'string)

+(defcustom org-journal-scheduled-string ""      ; or org-scheduled-string
+  "String added before a time stamp for schedules."
+  :type 'string)
+
 (defvar org-journal-after-entry-create-hook nil
   "Hook called after journal entry creation.")

@@ -1125,7 +1129,8 @@ With non-nil prefix argument create a regular entry instead of a TODO entry."
         (insert (format-time-string org-journal-time-format time)))
     (save-excursion
       (insert "\n"
-	      org-scheduled-string " ") ; "SCHEDULED: "
+              org-journal-scheduled-string
+              (if (length> org-journal-scheduled-string 0) " " ""))
       (org-insert-time-stamp
        time org-time-was-given nil nil nil (list org-end-time-was-given)))))

@@ -1149,7 +1154,7 @@ With non-nil prefix argument create a regular entry instead of a TODO entry."
                 "\\(TODO \\)?"
                 "\\("
                 (replace-regexp-in-string "[[:digit:]]" "[[:digit:]]"
-					  (format-time-string org-journal-time-format '(0 0) t))
+                                          (format-time-string org-journal-time-format '(0 0) t))
                 "\\)")))
           (if (re-search-forward regexp (line-end-position) t)
               (progn
@@ -1158,11 +1163,14 @@ With non-nil prefix argument create a regular entry instead of a TODO entry."
                     (insert (format-time-string org-journal-time-format time))))))
         ;; update time of timestamp (<...>)
         (org-back-to-heading)
-        (if (re-search-forward (concat org-scheduled-string "[[:blank:]]*" org-ts-regexp) nil t)
+        (if (re-search-forward (concat (regexp-quote org-journal-scheduled-string)
+                                       "[[:blank:]]*" org-ts-regexp)
+                               nil t)
             (replace-match "")
           (org-end-of-subtree)
           (insert "\n"))
-        (insert org-scheduled-string " ")
+        (insert org-journal-scheduled-string
+                (if (length> org-journal-scheduled-string 0) " " ""))
         (org-insert-time-stamp time org-time-was-given nil nil nil (list org-end-time-was-given))
         (org-cut-subtree))
       (let (org-journal-carryover-items)

@bastibe
Copy link
Owner

bastibe commented Dec 5, 2023

Compatibility with different date formats is an issue I run into frequently. Any improvement on that front would be highly appreciated.

jmay added a commit to jmay/org-journal that referenced this pull request Dec 6, 2023
@jmay
Copy link
Collaborator

jmay commented Dec 6, 2023

Combined this PR with @TPKato's patch and PR #422, submitting a separate PR with the results

@casch-at casch-at added this to the 2.3.0 milestone Dec 7, 2023
@casch-at casch-at closed this in a834a53 Jan 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants