From: Tim Ruffing <crypto@timruffing.de>
To: Ihor Radchenko <yantar92@posteo.net>
Cc: emacs-orgmode@gnu.org
Subject: Re: [PATCH] org-agenda: Make sure skipping warning/delay days never increases their number
Date: Mon, 26 Feb 2024 15:07:41 +0100 [thread overview]
Message-ID: <59e48dfe744dc9409ff47183255bc64e92d26d88.camel@timruffing.de> (raw)
In-Reply-To: <87frxvt9dt.fsf@localhost>
[-- Attachment #1: Type: text/plain, Size: 1440 bytes --]
On Wed, 2024-02-14 at 14:20 +0000, Ihor Radchenko wrote:
> >
> We can do this - I do not see any code in the wild using the optional
> arguments. However, we should do it carefully - (1) Move the function
> to
> org-compat and mark obsolete, recommending a new function instead;
> (2)
> Create a new function with a new name (say,
> org-timestamp-get-warning-days) and without ZERO-OR-DELAY argument
> and
> use it across Org mode.
>
Hm, now that I think about it again, I'm not sure that defining a new
function is worth the hassle. There's nothing wrong with the current
function or the optional argument, in the sense that callers can simply
choose not to use the optional argument.
If anything, I think it would rather add a comment that it's
obsolete/there for historical reasons and should not be used. Let me
know if you think that's desirable.
> > - (suppress-prewarning
> > + (max-wdays
>
> May you also rename this variable to be more clear. Something like,
> max-warning-days? These short variable names make the code very
> difficult to read.
>
> > - (wdays (or suppress-prewarning (org-get-wdays
> > s))))
> > + (wdays (min max-wdays (org-get-wdays s))))
>
> warning-days
>
> > - (suppress-delay
> > + (max-ddays
>
> max-deadline-warning-days
>
Done.
Tim
[-- Attachment #2: 0001-org-agenda-Make-sure-skipping-warning-delay-days-nev.patch --]
[-- Type: text/x-patch, Size: 7777 bytes --]
From 145b2526882264985d497ee094006b7144aa860b Mon Sep 17 00:00:00 2001
From: Tim Ruffing <crypto@timruffing.de>
Date: Tue, 13 Feb 2024 10:57:29 +0100
Subject: [PATCH] org-agenda: Make sure skipping warning/delay days never
increases their number
* lisp/org-agenda.el (org-agenda-get-deadlines, org-agenda-get-scheduled):
Use minimum of warning/delay days specified in timestamp cookie and the
limit specified by `org-agenda-skip-deadline-prewarning-if-scheduled' or
`org-agenda-skip-scheduled-delay-if-deadline`, respectively.
* testing/lisp/test-org-agenda.el (test-org-agenda/skip-deadline-prewarning-if-scheduled):
New test
---
lisp/org-agenda.el | 25 +++++++++-----------
testing/lisp/test-org-agenda.el | 41 +++++++++++++++++++++++++++++++++
2 files changed, 52 insertions(+), 14 deletions(-)
diff --git a/lisp/org-agenda.el b/lisp/org-agenda.el
index 23ccea9df..83e1d1264 100644
--- a/lisp/org-agenda.el
+++ b/lisp/org-agenda.el
@@ -6399,14 +6399,14 @@ specification like [h]h:mm."
(org-agenda--timestamp-to-absolute
s base 'future (current-buffer) pos)))))
(diff (- deadline current))
- (suppress-prewarning
+ (max-warning-days
(let ((scheduled
(and org-agenda-skip-deadline-prewarning-if-scheduled
(org-element-property
:raw-value
(org-element-property :scheduled el)))))
(cond
- ((not scheduled) nil)
+ ((not scheduled) most-positive-fixnum)
;; The current item has a scheduled date, so
;; evaluate its prewarning lead time.
((integerp org-agenda-skip-deadline-prewarning-if-scheduled)
@@ -6420,15 +6420,15 @@ specification like [h]h:mm."
org-deadline-warning-days))
;; Set pre-warning to deadline.
(t 0))))
- (wdays (or suppress-prewarning (org-get-wdays s))))
+ (warning-days (min max-warning-days (org-get-wdays s))))
(cond
;; Only display deadlines at their base date, at future
;; repeat occurrences or in today agenda.
((= current deadline) nil)
((= current repeat) nil)
((not today?) (throw :skip nil))
- ;; Upcoming deadline: display within warning period WDAYS.
- ((> deadline current) (when (> diff wdays) (throw :skip nil)))
+ ;; Upcoming deadline: display within warning period WARNING-DAYS.
+ ((> deadline current) (when (> diff warning-days) (throw :skip nil)))
;; Overdue deadline: warn about it for
;; `org-deadline-past-days' duration.
(t (when (< org-deadline-past-days (- diff)) (throw :skip nil))))
@@ -6481,7 +6481,7 @@ specification like [h]h:mm."
'effort-minutes effort-minutes)
level category tags time))
(face (org-agenda-deadline-face
- (- 1 (/ (float diff) (max wdays 1)))))
+ (- 1 (/ (float diff) (max warning-days 1)))))
(upcoming? (and today? (> deadline today)))
(warntime (org-entry-get (point) "APPT_WARNTIME" 'selective)))
(org-add-props item props
@@ -6610,13 +6610,13 @@ scheduled items with an hour specification like [h]h:mm."
(futureschedp (> schedule today))
(habitp (and (fboundp 'org-is-habit-p)
(string= "habit" (org-element-property :STYLE el))))
- (suppress-delay
+ (max-delay-days
(let ((deadline (and org-agenda-skip-scheduled-delay-if-deadline
(org-element-property
:raw-value
(org-element-property :deadline el)))))
(cond
- ((not deadline) nil)
+ ((not deadline) most-positive-fixnum)
;; The current item has a deadline date, so
;; evaluate its delay time.
((integerp org-agenda-skip-scheduled-delay-if-deadline)
@@ -6629,17 +6629,14 @@ scheduled items with an hour specification like [h]h:mm."
(org-agenda--timestamp-to-absolute deadline))
org-scheduled-delay-days))
(t 0))))
- (ddays
+ (delay-days
(cond
;; Nullify delay when a repeater triggered already
;; and the delay is of the form --Xd.
((and (string-match-p "--[0-9]+[hdwmy]" s)
(> schedule (org-agenda--timestamp-to-absolute s)))
0)
- (suppress-delay
- (let ((org-scheduled-delay-days suppress-delay))
- (org-get-wdays s t t)))
- (t (org-get-wdays s t)))))
+ (t (min max-delay-days (org-get-wdays s t))))))
;; Display scheduled items at base date (SCHEDULE), today if
;; scheduled before the current date, and at any repeat past
;; today. However, skip delayed items and items that have
@@ -6647,7 +6644,7 @@ scheduled items with an hour specification like [h]h:mm."
(unless (and todayp
habitp
(bound-and-true-p org-habit-show-all-today))
- (when (or (and (> ddays 0) (< diff ddays))
+ (when (or (and (> delay-days 0) (< diff delay-days))
(> diff (or (and habitp org-habit-scheduled-past-days)
org-scheduled-past-days))
(> schedule current)
diff --git a/testing/lisp/test-org-agenda.el b/testing/lisp/test-org-agenda.el
index f98214b25..3348db473 100644
--- a/testing/lisp/test-org-agenda.el
+++ b/testing/lisp/test-org-agenda.el
@@ -687,6 +687,47 @@ Sunday 7 January 2024
(should-not (org-agenda-files)))
(org-test-agenda--kill-all-agendas))
+(ert-deftest test-org-agenda/skip-deadline-prewarning-if-scheduled ()
+ "Test `org-agenda-skip-deadline-prewarning-if-scheduled'."
+ (org-test-at-time
+ "2024-01-15"
+ (let ((org-agenda-skip-deadline-prewarning-if-scheduled t))
+ (org-test-agenda-with-agenda
+ "* TODO foo\nDEADLINE: <2024-01-20 Sat> SCHEDULED: <2024-01-19 Fri>"
+ (org-agenda-list nil nil 1)
+ (should-not (search-forward "In " nil t))))
+ (let ((org-agenda-skip-deadline-prewarning-if-scheduled 10))
+ (org-test-agenda-with-agenda
+ "* TODO foo\nDEADLINE: <2024-01-20 Sat> SCHEDULED: <2024-01-19 Fri>"
+ (org-agenda-list nil nil 1)
+ (should (search-forward "In " nil t))))
+ ;; Custom prewarning cookie "-3d", so there should be no warning anyway.
+ (let ((org-agenda-skip-deadline-prewarning-if-scheduled 10))
+ (org-test-agenda-with-agenda
+ "* TODO foo\nDEADLINE: <2024-01-20 Sat -3d> SCHEDULED: <2024-01-19 Fri>"
+ (org-agenda-list nil nil 1)
+ (should-not (search-forward "In " nil t))))
+ (let ((org-agenda-skip-deadline-prewarning-if-scheduled 3))
+ (org-test-agenda-with-agenda
+ "* TODO foo\nDEADLINE: <2024-01-20 Sat> SCHEDULED: <2024-01-19 Fri>"
+ (org-agenda-list nil nil 1)
+ (should-not (search-forward "In " nil t))))
+ (let ((org-agenda-skip-deadline-prewarning-if-scheduled nil))
+ (org-test-agenda-with-agenda
+ "* TODO foo\nDEADLINE: <2024-01-20 Sat> SCHEDULED: <2024-01-19 Fri>"
+ (org-agenda-list nil nil 1)
+ (should (search-forward "In " nil t))))
+ (let ((org-agenda-skip-deadline-prewarning-if-scheduled 'pre-scheduled))
+ (org-test-agenda-with-agenda
+ "* TODO foo\nDEADLINE: <2024-01-20 Sat> SCHEDULED: <2024-01-16 Tue>"
+ (org-agenda-list nil nil 1)
+ (should-not (search-forward "In " nil t))))
+ (let ((org-agenda-skip-deadline-prewarning-if-scheduled 'pre-scheduled))
+ (org-test-agenda-with-agenda
+ "* TODO foo\nDEADLINE: <2024-01-20 Sat> SCHEDULED: <2024-01-15 Mon>"
+ (org-agenda-list nil nil 1)
+ (should (search-forward "In " nil t))))))
+
\f
;; agenda redo
--
2.44.0
next prev parent reply other threads:[~2024-02-26 14:09 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-13 10:34 [PATCH] org-agenda: Make sure skipping warning/delay days never increases their number Tim Ruffing
2024-02-13 14:24 ` Ihor Radchenko
2024-02-13 15:58 ` Tim Ruffing
2024-02-14 14:20 ` Ihor Radchenko
2024-02-26 14:07 ` Tim Ruffing [this message]
2024-02-27 12:49 ` Ihor Radchenko
2024-02-27 22:26 ` Tim Ruffing
2024-02-28 11:56 ` Ihor Radchenko
2024-02-28 12:33 ` Bastien Guerry
2024-02-28 13:18 ` Ihor Radchenko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.orgmode.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=59e48dfe744dc9409ff47183255bc64e92d26d88.camel@timruffing.de \
--to=crypto@timruffing.de \
--cc=emacs-orgmode@gnu.org \
--cc=yantar92@posteo.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).