[PATCH] Fix org-agenda-skip-scheduled-if-deadline-is-shown bug

[PATCH] Fix org-agenda-skip-scheduled-if-deadline-is-shown bug

[Morgan Smith]

lisp/org-agenda.el (org-agenda-get-scheduled): Consolidate deadline
fetching code.  Don't check if deadline is shown when
'org-agenda-skip-scheduled-if-deadline-is-shown' has a value of
'repeated-after-deadline'.

Currently when 'org-agenda-skip-scheduled-if-deadline-is-shown' has a
value of 'repeated-after-deadline' then there is no effect.  This is
because when 'org-agenda-get-scheduled' is run on later dates, the
previous deadlines are not put in 'deadline-pos'.
---
 lisp/org-agenda.el | 49 +++++++++++++++++++++++-----------------------
 1 file changed, 24 insertions(+), 25 deletions(-)

diff --git a/lisp/org-agenda.el b/lisp/org-agenda.el
index a1b2f3dc4..df1e8cb7d 100644
--- a/lisp/org-agenda.el
+++ b/lisp/org-agenda.el
@@ -6606,25 +6606,26 @@ 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))))
+                  (deadline (and (or org-agenda-skip-scheduled-delay-if-deadline
+                                     org-agenda-skip-scheduled-if-deadline-is-shown)
+                                 (when-let ((timestamp (org-element-property :deadline el)))
+                                   (time-to-days
+                                    (org-timestamp-to-time
+                                     timestamp)))))
 	          (suppress-delay
-		   (let ((deadline (and org-agenda-skip-scheduled-delay-if-deadline
-                                        (org-element-property
-                                         :raw-value
-                                         (org-element-property :deadline el)))))
-		     (cond
-		      ((not deadline) nil)
-		      ;; The current item has a deadline date, so
-		      ;; evaluate its delay time.
-		      ((integerp org-agenda-skip-scheduled-delay-if-deadline)
-		       ;; Use global delay time.
-		       (- org-agenda-skip-scheduled-delay-if-deadline))
-		      ((eq org-agenda-skip-scheduled-delay-if-deadline
-			   'post-deadline)
-		       ;; Set delay to no later than DEADLINE.
-		       (min (- schedule
-			       (org-agenda--timestamp-to-absolute deadline))
-			    org-scheduled-delay-days))
-		      (t 0))))
+                   (cond
+                    ((not (and deadline org-agenda-skip-scheduled-delay-if-deadline)) nil)
+                    ;; The current item has a deadline date, so
+                    ;; evaluate its delay time.
+                    ((integerp org-agenda-skip-scheduled-delay-if-deadline)
+                     ;; Use global delay time.
+                     (- org-agenda-skip-scheduled-delay-if-deadline))
+                    ((eq org-agenda-skip-scheduled-delay-if-deadline
+                         'post-deadline)
+                     ;; Set delay to no later than DEADLINE.
+                     (min (- schedule deadline)
+                          org-scheduled-delay-days))
+                    (t 0)))
 	          (ddays
 		   (cond
 		    ;; Nullify delay when a repeater triggered already
@@ -6661,16 +6662,14 @@ scheduled items with an hour specification like [h]h:mm."
 	     ;; doesn't apply to habits.
 	     (when (pcase org-agenda-skip-scheduled-if-deadline-is-shown
 		     ((guard
-		       (or (not (memq (line-beginning-position 0) deadline-pos))
+		       (or (not deadline)
 			   habitp))
 		      nil)
 		     (`repeated-after-deadline
-		      (let ((deadline (time-to-days
-                                       (when (org-element-property :deadline el)
-                                         (org-time-string-to-time
-                                          (org-element-interpret-data
-                                           (org-element-property :deadline el)))))))
-		        (and (<= schedule deadline) (> current deadline))))
+                      (and (<= schedule deadline) (> current deadline)))
+                     ((guard
+                       (not (memq (line-beginning-position 0) deadline-pos)))
+                      nil)
 		     (`not-today pastschedp)
 		     (`t t)
 		     (_ nil))
-- 
2.41.0

Re: [PATCH] Fix org-agenda-skip-scheduled-if-deadline-is-shown bug

[Ihor Radchenko]

Morgan Smith <Morgan.J.Smith@outlook.com> writes:

> lisp/org-agenda.el (org-agenda-get-scheduled): Consolidate deadline
> fetching code.  Don't check if deadline is shown when
> 'org-agenda-skip-scheduled-if-deadline-is-shown' has a value of
> 'repeated-after-deadline'.
>
> Currently when 'org-agenda-skip-scheduled-if-deadline-is-shown' has a
> value of 'repeated-after-deadline' then there is no effect.  This is
> because when 'org-agenda-get-scheduled' is run on later dates, the
> previous deadlines are not put in 'deadline-pos'.

May you please provide a detailed reproducer demonstrating the bug you
are trying to fix? Such reproducer could be a basis of a new test.

-- 
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] Fix org-agenda-skip-scheduled-if-deadline-is-shown bug

[Morgan Smith]

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

Ihor Radchenko <yantar92@posteo.net> writes:

> Morgan Smith <Morgan.J.Smith@outlook.com> writes:
>
>> lisp/org-agenda.el (org-agenda-get-scheduled): Consolidate deadline
>> fetching code.  Don't check if deadline is shown when
>> 'org-agenda-skip-scheduled-if-deadline-is-shown' has a value of
>> 'repeated-after-deadline'.
>>
>> Currently when 'org-agenda-skip-scheduled-if-deadline-is-shown' has a
>> value of 'repeated-after-deadline' then there is no effect.  This is
>> because when 'org-agenda-get-scheduled' is run on later dates, the
>> previous deadlines are not put in 'deadline-pos'.
>
> May you please provide a detailed reproducer demonstrating the bug you
> are trying to fix? Such reproducer could be a basis of a new test.

See a detailed reproducer attached to this mail.  It has a task defined
as this:

* TODO task
SCHEDULED: <2017-03-06 Mon +2d> DEADLINE: <2017-03-10>

Running said reproducer currently fails as follows (the numbers are the
days of the month).

(string-equal
"06\nScheduled: task\n08\nScheduled: task\n10\nScheduled: task\nDeadline:  task\n"
"06\nScheduled: task\n08\nScheduled: task\n10\nScheduled: task\nDeadline:  task\n12\nScheduled: task\n")


As you can see, we expect to not see anything scheduled after the
deadline if 'org-agenda-skip-scheduled-if-deadline-is-shown' is set to
'repeated-after-deadline', however, we actually see that things continue
to be scheduled.  Hence the bug is that that option currently does
nothing.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Testing-Add-tests-for-org-agenda-skip-scheduled-if-d.patch --]
[-- Type: text/x-patch, Size: 2949 bytes --]

From d7e72b9f0189b98d7ae59f0a56d54ffe70583956 Mon Sep 17 00:00:00 2001
From: Morgan Smith <Morgan.J.Smith@outlook.com>
Date: Tue, 2 Jan 2024 14:25:07 -0500
Subject: [PATCH] Testing: Add tests for
 'org-agenda-skip-scheduled-if-deadline-is-shown'

* testing/lisp/test-org-agenda.el
(test-org-agenda/org-agenda-skip-scheduled-if-deadline-is-shown): New test.
---
 testing/lisp/test-org-agenda.el | 45 +++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/testing/lisp/test-org-agenda.el b/testing/lisp/test-org-agenda.el
index 409d44192..6618fabc5 100644
--- a/testing/lisp/test-org-agenda.el
+++ b/testing/lisp/test-org-agenda.el
@@ -651,6 +651,51 @@ functions."
       (should (= arg-f-call-cnt 1))
       (should (equal f-called-args '(1 2 3))))))
 
+(ert-deftest test-org-agenda/org-agenda-skip-scheduled-if-deadline-is-shown ()
+  "Test values for `org-agenda-skip-scheduled-if-deadline-is-shown'."
+  (cl-assert (not org-agenda-sticky) nil "precondition violation")
+  (cl-assert (not (org-test-agenda--agenda-buffers))
+             nil "precondition violation")
+  (dolist (test-time '("2017-03-06" "2017-03-10"))
+    (let ((org-agenda-custom-commands
+           '(("f" "no fluff" agenda ""
+              ((org-agenda-overriding-header "")
+               (org-agenda-todo-keyword-format "")
+               (org-agenda-prefix-format "%s")
+               (org-agenda-format-date "%d")
+               (org-agenda-show-all-dates nil)))))
+          (org-deadline-warning-days 0)
+          (todayp (string= test-time "2017-03-10")))
+      (dolist (org-agenda-skip-scheduled-if-deadline-is-shown (list nil t 'not-today 'repeated-after-deadline))
+        (org-test-at-time test-time
+          (org-test-agenda-with-agenda
+       "
+* TODO task
+SCHEDULED: <2017-03-06 Mon +2d> DEADLINE: <2017-03-10>
+"
+       (should
+        (string-equal
+         (concat "06\n"
+                 "Scheduled: task\n"
+                 (if todayp ; We don't show repeats scheduled in the past
+                     ""
+                   "08\nScheduled: task\n")
+                 "10\n"
+                 (if (and org-agenda-skip-scheduled-if-deadline-is-shown
+                          (not (and (not todayp) (eq org-agenda-skip-scheduled-if-deadline-is-shown 'not-today)))
+                          (not (eq org-agenda-skip-scheduled-if-deadline-is-shown 'repeated-after-deadline)))
+                     ""
+                   (if todayp
+                       "Sched. 4x: task\n"
+                     "Scheduled: task\n"))
+                 "Deadline:  task\n"
+                 (unless (eq org-agenda-skip-scheduled-if-deadline-is-shown 'repeated-after-deadline)
+                   "12\nScheduled: task\n"))
+         (progn
+           (org-agenda nil "f")
+           (substring-no-properties (buffer-string)))))
+       (org-test-agenda--kill-all-agendas)))))))
+
 
 \f
 (provide 'test-org-agenda)
-- 
2.41.0

Re: [PATCH] Fix org-agenda-skip-scheduled-if-deadline-is-shown bug

[Ihor Radchenko]

Morgan Smith <Morgan.J.Smith@outlook.com> writes:

>> May you please provide a detailed reproducer demonstrating the bug you
>> are trying to fix? Such reproducer could be a basis of a new test.
>
> See a detailed reproducer attached to this mail.  It has a task defined
> as this:
>
> * TODO task
> SCHEDULED: <2017-03-06 Mon +2d> DEADLINE: <2017-03-10>
>
> Running said reproducer currently fails as follows (the numbers are the
> days of the month).
>
> (string-equal
> "06\nScheduled: task\n08\nScheduled: task\n10\nScheduled: task\nDeadline:  task\n"
> "06\nScheduled: task\n08\nScheduled: task\n10\nScheduled: task\nDeadline:  task\n12\nScheduled: task\n")
>
> As you can see, we expect to not see anything scheduled after the
> deadline if 'org-agenda-skip-scheduled-if-deadline-is-shown' is set to
> 'repeated-after-deadline', however, we actually see that things continue
> to be scheduled.  Hence the bug is that that option currently does
> nothing.

I think that you misunderstand the purpose of 'repeated-after-deadline
value. Let me provide an example:

    'repeated-after-deadline allows this task
    
    * Do me every day before March, 16th (included)
      SCHEDULED: <2013-03-12 mar. +1d> DEADLINE: <2013-03-16 sam.>
    
    to step being displayed after March, 16th.

-- 
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] Fix org-agenda-skip-scheduled-if-deadline-is-shown bug

[Morgan Smith]

Ihor Radchenko <yantar92@posteo.net> writes:

> Morgan Smith <Morgan.J.Smith@outlook.com> writes:
>
>>> May you please provide a detailed reproducer demonstrating the bug you
>>> are trying to fix? Such reproducer could be a basis of a new test.
>>
>> See a detailed reproducer attached to this mail.  It has a task defined
>> as this:
>>
>> * TODO task
>> SCHEDULED: <2017-03-06 Mon +2d> DEADLINE: <2017-03-10>
>>
>> Running said reproducer currently fails as follows (the numbers are the
>> days of the month).
>>
>> (string-equal
>> "06\nScheduled: task\n08\nScheduled: task\n10\nScheduled: task\nDeadline:  task\n"
>> "06\nScheduled: task\n08\nScheduled: task\n10\nScheduled: task\nDeadline:  task\n12\nScheduled: task\n")
>>
>> As you can see, we expect to not see anything scheduled after the
>> deadline if 'org-agenda-skip-scheduled-if-deadline-is-shown' is set to
>> 'repeated-after-deadline', however, we actually see that things continue
>> to be scheduled.  Hence the bug is that that option currently does
>> nothing.
>
> I think that you misunderstand the purpose of 'repeated-after-deadline
> value. Let me provide an example:
>
>     'repeated-after-deadline allows this task
>
>     * Do me every day before March, 16th (included)
>       SCHEDULED: <2013-03-12 mar. +1d> DEADLINE: <2013-03-16 sam.>
>
>     to step being displayed after March, 16th.

In my above example I have a task with a deadline of March 10th.
My test shows that it will continue to be scheduled after March 10th, on
the 12th.  That is wrong.  It is still scheduled on the 10th which is
good.  Being scheduled on the 12th is wrong.

Re: [PATCH] Fix org-agenda-skip-scheduled-if-deadline-is-shown bug

[Ihor Radchenko]

[CCing Bastien]

Morgan Smith <Morgan.J.Smith@outlook.com> writes:

>> I think that you misunderstand the purpose of 'repeated-after-deadline
>> value. Let me provide an example:
>>
>>     'repeated-after-deadline allows this task
>>
>>     * Do me every day before March, 16th (included)
>>       SCHEDULED: <2013-03-12 mar. +1d> DEADLINE: <2013-03-16 sam.>
>>
>>     to step being displayed after March, 16th.
>
> In my above example I have a task with a deadline of March 10th.
> My test shows that it will continue to be scheduled after March 10th, on
> the 12th.  That is wrong.  It is still scheduled on the 10th which is
> good.  Being scheduled on the 12th is wrong.

Right. This is because after 72c3f5e8e refactoring,
`org-agenda-skip-scheduled-if-deadline-is-shown' is only respected when
the deadline is actually shown.

However, judging from the discussion of the original feature request
(https://list.orgmode.org/CAC10T-n8F+Evxf7vGcMT48R5ovoDy030AV1Cj3XyX02piF0tEQ@mail.gmail.com/T/#u),
it looks like the purpose of 'repeated-after-deadline value is broader
than just hiding scheduled record when the deadline record is actually
in agenda. AFAIU, the purpose is hiding the scheduled information
completely and unconditionally once the agenda day is beyond the
deadline.

But then `org-agenda-skip-scheduled-if-deadline-is-shown' does not look
like the right place for this customization. It would make more sense to
have a dedicated custom variable for this. WDYT?

I am adding Bastien to the loop - he authored the original feature
implementation.

-- 
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] Fix org-agenda-skip-scheduled-if-deadline-is-shown bug

[Morgan Smith]

Ihor Radchenko <yantar92@posteo.net> writes:

> But then `org-agenda-skip-scheduled-if-deadline-is-shown' does not look
> like the right place for this customization. It would make more sense to
> have a dedicated custom variable for this. WDYT?
>
> I am adding Bastien to the loop - he authored the original feature
> implementation.

I can't quite remember at this point but I'm pretty sure some
permutations are impossible with the current setup (like ignoring both
things that are scheduled today and in the future when the deadline is
today).

Regardless, I would like to say: perfect is the enemy of good.  It's a
little disheartening to see my work to stagnate over this.  If we want
to rename stuff, great, lets do that.  Otherwise I would appreciate it
if this work got incorporated.  If nothing else, I would at least like
to see my test accepted as I believe it is a good regression test.  If
you would like I can even remove the 'repeated-after-deadline' stuff and
resubmit it.

Re: [PATCH] Fix org-agenda-skip-scheduled-if-deadline-is-shown bug

[Ihor Radchenko]

Morgan Smith <Morgan.J.Smith@outlook.com> writes:

> Ihor Radchenko <yantar92@posteo.net> writes:
>
>> But then `org-agenda-skip-scheduled-if-deadline-is-shown' does not look
>> like the right place for this customization. It would make more sense to
>> have a dedicated custom variable for this. WDYT?
>> ..
> I can't quite remember at this point but I'm pretty sure some
> permutations are impossible with the current setup (like ignoring both
> things that are scheduled today and in the future when the deadline is
> today).

Your statement does not look like something related to my question about
moving 'repeated-after-deadline value to some other (existing or new)
customization.

Or did you mean to provide a generic suggestion about adding more agenda
customizations?

> Regardless, I would like to say: perfect is the enemy of good.  It's a
> little disheartening to see my work to stagnate over this.

Nothing is stagnated. I am just waiting for Bastien to chime in, if he
decides to. He is the person who can provide important context.
We allow up to one month for people to reply on the mailing list.
See https://orgmode.org/worg/org-mailing-list.html#org474747a

> ... If we want
> to rename stuff, great, lets do that.  Otherwise I would appreciate it
> if this work got incorporated.  If nothing else, I would at least like
> to see my test accepted as I believe it is a good regression test.  If
> you would like I can even remove the 'repeated-after-deadline' stuff and
> resubmit it.

I first need to decide what to do and I'd like to gather as much
information as possible (from Bastien or other interested users) before
making the decision. There is no rush to make changes ASAP.

-- 
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] Fix org-agenda-skip-scheduled-if-deadline-is-shown bug

[Morgan Smith]

Ihor Radchenko <yantar92@posteo.net> writes:

> Morgan Smith <Morgan.J.Smith@outlook.com> writes:
>
>> Ihor Radchenko <yantar92@posteo.net> writes:
>>
>>> But then `org-agenda-skip-scheduled-if-deadline-is-shown' does not look
>>> like the right place for this customization. It would make more sense to
>>> have a dedicated custom variable for this. WDYT?
>>> ..
>> I can't quite remember at this point but I'm pretty sure some
>> permutations are impossible with the current setup (like ignoring both
>> things that are scheduled today and in the future when the deadline is
>> today).
>
> Your statement does not look like something related to my question about
> moving 'repeated-after-deadline value to some other (existing or new)
> customization.
>
By adding another variable to control the 'repeated-after-deadline'
behavior, we would allow the user to combine this behavior with the
other behaviors of 'org-agenda-skip-scheduled-if-deadline-is-shown'.
This would allow for more permutations.  This is a point in favor of
adding a dedicated custom variable.
>
> Or did you mean to provide a generic suggestion about adding more agenda
> customizations?
>
Maybe we could create some data-structure that represents an agenda and
allow the user to manipulate that directly.  I'm not really a fan of
having a ton of variables controlling the agenda.  Although I imagine I
might gain some respect for it if I tried to implement this feature.
>
> Nothing is stagnated. I am just waiting for Bastien to chime in, if he
> decides to. He is the person who can provide important context.
> We allow up to one month for people to reply on the mailing list.
> See https://orgmode.org/worg/org-mailing-list.html#org474747a
>
I was unaware of the one month policy.  Thank you for letting me know.
Due to that lack of a bug tracking system I have a fear that things on
the mailing list would get lost.  Maybe this fear is unfounded?

Re: [PATCH] Fix org-agenda-skip-scheduled-if-deadline-is-shown bug

[Ihor Radchenko]

Morgan Smith <Morgan.J.Smith@outlook.com> writes:

> Maybe we could create some data-structure that represents an agenda and
> allow the user to manipulate that directly.  I'm not really a fan of
> having a ton of variables controlling the agenda.  Although I imagine I
> might gain some respect for it if I tried to implement this feature.

Agenda is one of the oldest codes we have in Org mode. It can certainly
see improvements and simplifications.
If you have ideas about providing better agenda customization, feel free
to share them.

>> Nothing is stagnated. I am just waiting for Bastien to chime in, if he
>> decides to. He is the person who can provide important context.
>> We allow up to one month for people to reply on the mailing list.
>> See https://orgmode.org/worg/org-mailing-list.html#org474747a
>>
> I was unaware of the one month policy.  Thank you for letting me know.
> Due to that lack of a bug tracking system I have a fear that things on
> the mailing list would get lost.  Maybe this fear is unfounded?

We do have a bug tracker - https://tracker.orgmode.org/bugs
Although it is still experimental.

In any case, I keep track of all the bugs and patches in my Org notes.

-- 
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] Fix org-agenda-skip-scheduled-if-deadline-is-shown bug

[Ihor Radchenko]

Ihor Radchenko <yantar92@posteo.net> writes:

> Right. This is because after 72c3f5e8e refactoring,
> `org-agenda-skip-scheduled-if-deadline-is-shown' is only respected when
> the deadline is actually shown.
>
> However, judging from the discussion of the original feature request
> (https://list.orgmode.org/CAC10T-n8F+Evxf7vGcMT48R5ovoDy030AV1Cj3XyX02piF0tEQ@mail.gmail.com/T/#u),
> it looks like the purpose of 'repeated-after-deadline value is broader
> than just hiding scheduled record when the deadline record is actually
> in agenda. AFAIU, the purpose is hiding the scheduled information
> completely and unconditionally once the agenda day is beyond the
> deadline.
>
> But then `org-agenda-skip-scheduled-if-deadline-is-shown' does not look
> like the right place for this customization. It would make more sense to
> have a dedicated custom variable for this. WDYT?

No response from Bastien. I am going ahead with introducing a new custom
option. The old option is removed, but remains supported for backwards
compatibility.

Fixed, on main.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=b26745b98

-- 
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] Fix org-agenda-skip-scheduled-if-deadline-is-shown bug

[Bastien Guerry]

Hi Ihor,

Ihor Radchenko <yantar92@posteo.net> writes:

> No response from Bastien.

Yes, sorry for that. In general, don't wait for my answer unless you
really needed it: you've got pre-approval as co-maintainer, I don't
want my current lack of availability to be a blocker.

Thanks,

-- 
 Bastien Guerry

Re: [PATCH] Fix org-agenda-skip-scheduled-if-deadline-is-shown bug

[Ihor Radchenko]

Bastien Guerry <bzg@gnu.org> writes:

> Ihor Radchenko <yantar92@posteo.net> writes:
>
>> No response from Bastien.
>
> Yes, sorry for that. In general, don't wait for my answer unless you
> really needed it: you've got pre-approval as co-maintainer, I don't
> want my current lack of availability to be a blocker.

No blocker here. In this particular case, the question was not to
Bastien the maintainer, but to Bastien the committer :) Because you
could remember details about the discussed feature that I cannot easily
derive from the list archives.

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