Patch: org-agenda-skip-deadline-prewarning-if-scheduled prior to scheduled date

Patch: org-agenda-skip-deadline-prewarning-if-scheduled prior to scheduled date

[Justus-dev]

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

Hi,

I have scratched an itch:

I would like not to be bothered with TODOs that have a scheduled date in
the future.  However, as of that scheduled date I would like their
deadline prewarnings to appear in the agenda, just like non-scheduled
TODOs.  I do not see how to achieve this behavior using the existing
org-mode facilities (I run the org shipped with Emacs 24.2.1).

I solved this by adding an option to
org-agenda-skip-deadline-prewarning-if-scheduled that behaves like
setting it to an integer value (custom deadline warning lead time), but
sets it to the number of days between scheduled and deadline dates.  As
a result, deadline warnings prior to the scheduled date are eliminated.

I attach two diffs: presched-24.2.1.diff reflects the change against
org-agenda.el from Emacs 24.2.1 I developed and tested;
presched-latest.diff (almost identical) reflects the same change against
today's org-agenda.el from the org-mode git.  The latter is untested,
but I'm fairly confident it works since the changes are very localized
and do not appear to interact with differences between the two original
org-agenda.els.

Please review the patch carefully:

- In org-agenda-get-deadlines, I moved the relevant form ("(setq
  suppress-prewarning ...)") down into the middle of a long (setq ...)
  because it builds on values computed above and provides values used
  below.

- I split the long (setq ...) into three separate (setq ...)s for
  readability because all other assignments are compact while this one
  is long and complex.

- Because of this complexity I added some comments which hopefully makes
  things readable.  However, a more experienced lisp programmer may be
  able to rewrite the code such that it is crystal clear even without
  comments.

- Instead of the lead-time logic described above, a different way to
  achieve a similar effect would be to add a condition that checks
  whether the scheduled date is in the future, in which case the
  prewarning is removed unconditionally.  This would presumably be done
  by setting suppress-prewarning to 0.  I have not tried to implement
  this.  As far as I can see, this would likewise involve computing the
  difference between two dates, and the resulting code would be of about
  the same complexity, but setting the max. lead to the scheduled date
  (instead of to 0) appears more sound to me.

Here's a suggested commit message:

org-agenda: New option: skip deadline prewarning if scheduled in the future

* lisp/org-agenda.el (org-agenda-skip-deadline-prewarning-if-scheduled):
  Add an option to skip the deadline prewarning if the scheduled date is
  in the future.

Comments?

Justus


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: presched-24.2.1.diff --]
[-- Type: text/x-diff, Size: 3655 bytes --]

--- org-agenda-24.2.1.el	2012-09-08 23:58:12.000000000 +0200
+++ org-agenda-24.2.1-presched.el	2012-10-17 09:12:01.020184803 +0200
@@ -789,9 +789,10 @@
 This will apply on all days where a prewarning for the deadline would
 be shown, but not at the day when the entry is actually due.  On that day,
 the deadline will be shown anyway.
-This variable may be set to nil, t, or a number which will then give
-the number of days before the actual deadline when the prewarnings
-should resume.
+This variable may be set to nil, t, the symbol `pre-scheduled', 
+or a number which will then give the number of days before the actual
+deadline when the prewarnings should resume.  The symbol `pre-scheduled'
+eliminates the deadline prewarning only prior to the scheduled date.
 This can be used in a workflow where the first showing of the deadline will
 trigger you to schedule it, and then you don't want to be reminded of it
 because you will take care of it on the day when scheduled."
@@ -800,6 +801,7 @@
   :version "24.1"
   :type '(choice
 	  (const :tag "Alwas show prewarning" nil)
+	  (const :tag "Remove prewarning prior to scheduled date" pre-scheduled)
 	  (const :tag "Remove prewarning if entry is scheduled" t)
 	  (integer :tag "Restart prewarning N days before deadline")))
 
@@ -5206,23 +5208,13 @@
 	 (regexp org-deadline-time-regexp)
 	 (todayp (org-agenda-todayp date)) ; DATE bound by calendar
 	 (d1 (calendar-absolute-from-gregorian date))  ; DATE bound by calendar
-	 d2 diff dfrac wdays pos pos1 category org-category-pos
+	 ds d2 diff dfrac wdays pos pos1 category org-category-pos
 	 tags suppress-prewarning ee txt head face s todo-state
 	 show-all upcomingp donep timestr)
     (goto-char (point-min))
     (while (re-search-forward regexp nil t)
-      (setq suppress-prewarning nil)
       (catch :skip
 	(org-agenda-skip)
-	(when (and org-agenda-skip-deadline-prewarning-if-scheduled
-		   (save-match-data
-		     (string-match org-scheduled-time-regexp
-				   (buffer-substring (point-at-bol)
-						     (point-at-eol)))))
-	  (setq suppress-prewarning
-		(if (integerp org-agenda-skip-deadline-prewarning-if-scheduled)
-		    org-agenda-skip-deadline-prewarning-if-scheduled
-		  0)))
 	(setq s (match-string 1)
 	      txt nil
 	      pos (1- (match-beginning 1))
@@ -5233,8 +5225,31 @@
 	      d2 (org-time-string-to-absolute
 		  (match-string 1) d1 'past show-all
 		  (current-buffer) pos)
-	      diff (- d2 d1)
-	      wdays (if suppress-prewarning
+	      diff (- d2 d1))
+	(setq suppress-prewarning
+	      (if (and org-agenda-skip-deadline-prewarning-if-scheduled
+		       (let ((item (buffer-substring (point-at-bol)
+						     (point-at-eol))))
+			 (setq ds (save-match-data
+				    (if (string-match
+					 org-scheduled-time-regexp item)
+					(match-string 1 item))))))
+		  ;; If prewarnings of scheduled items are to be skipped
+		  ;;    and the current item has a scheduled date (in ds),
+		  ;; calculate prewarning lead time:
+		  (if (integerp
+		       org-agenda-skip-deadline-prewarning-if-scheduled)
+		      ;; use prewarning-restart lead time:
+		      org-agenda-skip-deadline-prewarning-if-scheduled
+		    (if (eq org-agenda-skip-deadline-prewarning-if-scheduled
+			    'pre-scheduled)
+			;; show first prewarning no earlier than scheduled date:
+			(min (- d2 (org-time-string-to-absolute
+				    ds d1 'past show-all (current-buffer) pos))
+			     org-deadline-warning-days)
+		      ;; set prewarning to deadline:
+		      0))))
+	(setq wdays (if suppress-prewarning
 			(let ((org-deadline-warning-days suppress-prewarning))
 			  (org-get-wdays s))
 		      (org-get-wdays s))

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: presched-latest.diff --]
[-- Type: text/x-diff, Size: 3668 bytes --]

--- org-agenda-latest.el	2012-10-17 08:39:50.101427177 +0200
+++ org-agenda-latest-presched.el	2012-10-17 09:17:40.170574930 +0200
@@ -827,9 +827,10 @@
 This will apply on all days where a prewarning for the deadline would
 be shown, but not at the day when the entry is actually due.  On that day,
 the deadline will be shown anyway.
-This variable may be set to nil, t, or a number which will then give
-the number of days before the actual deadline when the prewarnings
-should resume.
+This variable may be set to nil, t, the symbol `pre-scheduled', 
+or a number which will then give the number of days before the actual
+deadline when the prewarnings should resume.  The symbol `pre-scheduled'
+eliminates the deadline prewarning only prior to the scheduled date.
 This can be used in a workflow where the first showing of the deadline will
 trigger you to schedule it, and then you don't want to be reminded of it
 because you will take care of it on the day when scheduled."
@@ -838,6 +839,7 @@
   :version "24.1"
   :type '(choice
 	  (const :tag "Alwas show prewarning" nil)
+	  (const :tag "Remove prewarning prior to scheduled date" pre-scheduled)
 	  (const :tag "Remove prewarning if entry is scheduled" t)
 	  (integer :tag "Restart prewarning N days before deadline")))
 
@@ -5807,23 +5809,13 @@
 	 (regexp org-deadline-time-regexp)
 	 (todayp (org-agenda-todayp date)) ; DATE bound by calendar
 	 (d1 (calendar-absolute-from-gregorian date))  ; DATE bound by calendar
-	 d2 diff dfrac wdays pos pos1 category category-pos level
+	 ds d2 diff dfrac wdays pos pos1 category category-pos level
 	 tags suppress-prewarning ee txt head face s todo-state
 	 show-all upcomingp donep timestr warntime)
     (goto-char (point-min))
     (while (re-search-forward regexp nil t)
-      (setq suppress-prewarning nil)
       (catch :skip
 	(org-agenda-skip)
-	(when (and org-agenda-skip-deadline-prewarning-if-scheduled
-		   (save-match-data
-		     (string-match org-scheduled-time-regexp
-				   (buffer-substring (point-at-bol)
-						     (point-at-eol)))))
-	  (setq suppress-prewarning
-		(if (integerp org-agenda-skip-deadline-prewarning-if-scheduled)
-		    org-agenda-skip-deadline-prewarning-if-scheduled
-		  0)))
 	(setq s (match-string 1)
 	      txt nil
 	      pos (1- (match-beginning 1))
@@ -5834,8 +5826,31 @@
 	      d2 (org-time-string-to-absolute
 		  (match-string 1) d1 'past show-all
 		  (current-buffer) pos)
-	      diff (- d2 d1)
-	      wdays (if suppress-prewarning
+	      diff (- d2 d1))
+	(setq suppress-prewarning
+	      (if (and org-agenda-skip-deadline-prewarning-if-scheduled
+		       (let ((item (buffer-substring (point-at-bol)
+						     (point-at-eol))))
+			 (setq ds (save-match-data
+				    (if (string-match
+					 org-scheduled-time-regexp item)
+					(match-string 1 item))))))
+		  ;; If prewarnings of scheduled items are to be skipped
+		  ;;    and the current item has a scheduled date (in ds),
+		  ;; calculate prewarning lead time:
+		  (if (integerp
+		       org-agenda-skip-deadline-prewarning-if-scheduled)
+		      ;; use prewarning-restart lead time:
+		      org-agenda-skip-deadline-prewarning-if-scheduled
+		    (if (eq org-agenda-skip-deadline-prewarning-if-scheduled
+			    'pre-scheduled)
+			;; show first prewarning no earlier than scheduled date:
+			(min (- d2 (org-time-string-to-absolute
+				    ds d1 'past show-all (current-buffer) pos))
+			     org-deadline-warning-days)
+		      ;; set prewarning to deadline:
+		      0))))
+	(setq wdays (if suppress-prewarning
 			(let ((org-deadline-warning-days suppress-prewarning))
 			  (org-get-wdays s))
 		      (org-get-wdays s))

Re: Patch: org-agenda-skip-deadline-prewarning-if-scheduled prior to scheduled date

[Nicolas Goaziou]

Hello,

Justus-dev@Piater.name writes:

> I would like not to be bothered with TODOs that have a scheduled date in
> the future.  However, as of that scheduled date I would like their
> deadline prewarnings to appear in the agenda, just like non-scheduled
> TODOs.  I do not see how to achieve this behavior using the existing
> org-mode facilities (I run the org shipped with Emacs 24.2.1).

I'm not sure if that's already possible either. Could someone confirm it
isn't?

> Here's a suggested commit message:
>
> org-agenda: New option: skip deadline prewarning if scheduled in the future
>
> * lisp/org-agenda.el (org-agenda-skip-deadline-prewarning-if-scheduled):
>   Add an option to skip the deadline prewarning if the scheduled date is
>   in the future.
>
> Comments?

Since you provide both the patch and the commit message, what about
providing it completely as an attachement (with i.e. git format-patch)?

A few minor comments about the code.

> -	 d2 diff dfrac wdays pos pos1 category category-pos level
> +	 ds d2 diff dfrac wdays pos pos1 category category-pos level

I wouldn't define the "ds" variable here, since it's local to
suppress-prewarning computation. It's clearer to let-bind it around the
(if (and org-agenda-skip-deadline-prewarning-if-scheduled ....)).

> +			 (setq ds (save-match-data
> +				    (if (string-match
> +					 org-scheduled-time-regexp item)
> +					(match-string 1 item))))))

Since it is setq'ed only once, it can be let-bound. Also, about style:

  (if (string-match
      org-scheduled-time-regexp item)
    (match-string 1 item))

=>

  (and (string-match org-scheduled-time-regexp item)
       (match-string 1 item))

> +		  ;; If prewarnings of scheduled items are to be skipped
> +		  ;;    and the current item has a scheduled date (in ds),
> +		  ;; calculate prewarning lead time:

Small mistake in comments indentation.

> +		  (if (integerp
> +		       org-agenda-skip-deadline-prewarning-if-scheduled)
> +		      ;; use prewarning-restart lead time:

Please capitalize comments and end them with a period.

> +		      org-agenda-skip-deadline-prewarning-if-scheduled
> +		    (if (eq org-agenda-skip-deadline-prewarning-if-scheduled
> +			    'pre-scheduled)
> +			;; show first prewarning no earlier than scheduled date:

Ditto.

> +			(min (- d2 (org-time-string-to-absolute
> +				    ds d1 'past show-all (current-buffer) pos))
> +			     org-deadline-warning-days)
> +		      ;; set prewarning to deadline:

Ditto.

Also, since your contribution is more than 20 lines long, did you sign
the FSF papers?

Thanks for your work.


Regards,

-- 
Nicolas Goaziou

Re: Patch: org-agenda-skip-deadline-prewarning-if-scheduled prior to scheduled date

[Justus-dev]

Nicolas,

Many thanks for your comments, I will heed them all.

But first one more request for comments that I forgot to include in my
original message:

Computing the difference between two dates in the presence of repeating
dates is a subtle affair.  Here, in computing the difference between
deadline (d2) and scheduled date (ds),

			(min (- d2 (org-time-string-to-absolute
				    ds d1 'past show-all (current-buffer) pos))
			     org-deadline-warning-days)

I simply followed the time-to-deadline computation of the current TODO
item from a few lines above:

	(setq ...
	      d2 (org-time-string-to-absolute
		  (match-string 1) d1 'past show-all
		  (current-buffer) pos)
	      diff (- d2 d1))

I sort-of see what this org-time-string-to-absolute call does, but I do
not fully understand its implications.  It would be good for someone who
does to verify that my code indeed rightly follows the same logic.

It appears to work for me with various repeating and non-repeating TODOs
with dates in past and future, but I certainly did not exhaustively test
all possible combinations of sensible and nonsensical combinations of
non/repeating scheduled and deadline dates, show-all, and so on.

Cheers,
Justus

-- 
Intelligent and Interactive Systems https://iis.uibk.ac.at/
Institute of Computer Science, University of Innsbruck, Austria

Re: Patch: org-agenda-skip-deadline-prewarning-if-scheduled prior to scheduled date

[Nicolas Goaziou]

Hello,

Justus-dev@Piater.name writes:

> But first one more request for comments that I forgot to include in my
> original message:
>
> Computing the difference between two dates in the presence of repeating
> dates is a subtle affair.  Here, in computing the difference between
> deadline (d2) and scheduled date (ds),
>
> 			(min (- d2 (org-time-string-to-absolute
> 				    ds d1 'past show-all (current-buffer) pos))
> 			     org-deadline-warning-days)
>
> I simply followed the time-to-deadline computation of the current TODO
> item from a few lines above:
>
> 	(setq ...
> 	      d2 (org-time-string-to-absolute
> 		  (match-string 1) d1 'past show-all
> 		  (current-buffer) pos)
> 	      diff (- d2 d1))
>
> I sort-of see what this org-time-string-to-absolute call does, but I do
> not fully understand its implications.  It would be good for someone who
> does to verify that my code indeed rightly follows the same logic.

The code looks good to me. If there is no objection, I think we can
apply the patch.


Regards,

-- 
Nicolas Goaziou

Re: Patch: org-agenda-skip-deadline-prewarning-if-scheduled prior to scheduled date

[Justus-bulk]

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

Hello,

Nicolas Goaziou <n.goaziou@gmail.com> wrote on Wed, 17 Oct 2012 15:07:23
+0200:

> Since you provide both the patch and the commit message, what about
> providing it completely as an attachement (with i.e. git format-patch)?

I attach a patch against the current git master.

>> -	 d2 diff dfrac wdays pos pos1 category category-pos level
>> +	 ds d2 diff dfrac wdays pos pos1 category category-pos level
>
> I wouldn't define the "ds" variable here, since it's local to
> suppress-prewarning computation. It's clearer to let-bind it around the
> (if (and org-agenda-skip-deadline-prewarning-if-scheduled ....)).
>
>> +			 (setq ds (save-match-data
>> +				    (if (string-match
>> +					 org-scheduled-time-regexp item)
>> +					(match-string 1 item))))))
>
> Since it is setq'ed only once, it can be let-bound.

I did not find a fully satisfactory way to do this.  I can wrap the (if
(and org-agenda-skip-deadline-prewarning-if-scheduled ...)) inside a
dedicated (let ...), but I would like to evaluate ds only if org-... is
non-nil, so I would have to keep the setq inside the conditional.  The
only gain would be the limited scope of the ds, but the price of the
additional (let ...) clutter is high, I think.

My current solution (see the patch) involves some minor refactoring at
the price of an extra (if ds ...) that I don't see a way to get rid of,
but I find the result quite readable.

Also, I replaced the if-elseif chain by a cond for clarity.

> Also, since your contribution is more than 20 lines long, did you sign
> the FSF papers?

I signed the FSF GNU EMACS copyright assignment form a few years ago.
Do I need to do something else for org-mode?

Justus



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org-agenda-New-option-skip-deadline-prewarning-if-sc.patch --]
[-- Type: text/x-diff, Size: 4103 bytes --]

From 342abbe56b786fa7361c6ef574623c4c07915058 Mon Sep 17 00:00:00 2001
From: Justus Piater <justus-dev@piater.name>
Date: Fri, 26 Oct 2012 12:47:47 +0200
Subject: [PATCH] org-agenda: New option: skip deadline prewarning if
 scheduled in the future

* lisp/org-agenda.el (org-agenda-skip-deadline-prewarning-if-scheduled):
  Add an option to skip the deadline prewarning if the scheduled date is
  in the future.
---
 lisp/org-agenda.el |   45 ++++++++++++++++++++++++++++++---------------
 1 file changed, 30 insertions(+), 15 deletions(-)

diff --git a/lisp/org-agenda.el b/lisp/org-agenda.el
index 08066d2..ea4abfd 100644
--- a/lisp/org-agenda.el
+++ b/lisp/org-agenda.el
@@ -827,9 +827,10 @@ deadlines are always turned off when the item is DONE."
 This will apply on all days where a prewarning for the deadline would
 be shown, but not at the day when the entry is actually due.  On that day,
 the deadline will be shown anyway.
-This variable may be set to nil, t, or a number which will then give
-the number of days before the actual deadline when the prewarnings
-should resume.
+This variable may be set to nil, t, the symbol `pre-scheduled', 
+or a number which will then give the number of days before the actual
+deadline when the prewarnings should resume.  The symbol `pre-scheduled'
+eliminates the deadline prewarning only prior to the scheduled date.
 This can be used in a workflow where the first showing of the deadline will
 trigger you to schedule it, and then you don't want to be reminded of it
 because you will take care of it on the day when scheduled."
@@ -838,6 +839,7 @@ because you will take care of it on the day when scheduled."
   :version "24.1"
   :type '(choice
 	  (const :tag "Always show prewarning" nil)
+	  (const :tag "Remove prewarning prior to scheduled date" pre-scheduled)
 	  (const :tag "Remove prewarning if entry is scheduled" t)
 	  (integer :tag "Restart prewarning N days before deadline")))
 
@@ -5812,18 +5814,8 @@ See also the user option `org-agenda-clock-consistency-checks'."
 	 show-all upcomingp donep timestr warntime)
     (goto-char (point-min))
     (while (re-search-forward regexp nil t)
-      (setq suppress-prewarning nil)
       (catch :skip
 	(org-agenda-skip)
-	(when (and org-agenda-skip-deadline-prewarning-if-scheduled
-		   (save-match-data
-		     (string-match org-scheduled-time-regexp
-				   (buffer-substring (point-at-bol)
-						     (point-at-eol)))))
-	  (setq suppress-prewarning
-		(if (integerp org-agenda-skip-deadline-prewarning-if-scheduled)
-		    org-agenda-skip-deadline-prewarning-if-scheduled
-		  0)))
 	(setq s (match-string 1)
 	      txt nil
 	      pos (1- (match-beginning 1))
@@ -5834,8 +5826,31 @@ See also the user option `org-agenda-clock-consistency-checks'."
 	      d2 (org-time-string-to-absolute
 		  (match-string 1) d1 'past show-all
 		  (current-buffer) pos)
-	      diff (- d2 d1)
-	      wdays (if suppress-prewarning
+	      diff (- d2 d1))
+	(setq suppress-prewarning
+	      (let ((ds (and org-agenda-skip-deadline-prewarning-if-scheduled
+			     (let ((item (buffer-substring (point-at-bol)
+							   (point-at-eol))))
+			       (save-match-data
+				 (and (string-match
+				       org-scheduled-time-regexp item)
+				      (match-string 1 item)))))))
+		(if ds
+		    ;; The current item has a scheduled date (in ds),
+		    ;; so evaluate its prewarning lead time.
+		    (cond
+		     ((integerp
+		       org-agenda-skip-deadline-prewarning-if-scheduled)
+		      ;; Use globally-customized prewarning-restart lead time.
+		      org-agenda-skip-deadline-prewarning-if-scheduled)
+		     ((eq org-agenda-skip-deadline-prewarning-if-scheduled
+			  'pre-scheduled)
+		      ;; Set prewarning to no earlier than scheduled.
+		      (min (- d2 (org-time-string-to-absolute
+				  ds d1 'past show-all (current-buffer) pos))
+			   org-deadline-warning-days))
+		     (t 0))))) ;; Set prewarning to deadline.
+	(setq wdays (if suppress-prewarning
 			(let ((org-deadline-warning-days suppress-prewarning))
 			  (org-get-wdays s))
 		      (org-get-wdays s))
-- 
1.7.10.4

Re: Patch: org-agenda-skip-deadline-prewarning-if-scheduled prior to scheduled date

[Nicolas Goaziou]

Hello,

Justus-bulk@Piater.name writes:

> My current solution (see the patch) involves some minor refactoring at
> the price of an extra (if ds ...) that I don't see a way to get rid of,
> but I find the result quite readable.

It looks good. I would have integrated the (if ds ...) within the cond
(i.e (cond ((not ds) nil) ...)) but it doesn't matter much.

> I signed the FSF GNU EMACS copyright assignment form a few years ago.
> Do I need to do something else for org-mode?

No, it should be fine.

I've applied your patch. Thank you for your contribution.


Regards,

-- 
Nicolas Goaziou

Re: Patch: org-agenda-skip-deadline-prewarning-if-scheduled prior to scheduled date

[Justus-bulk]

Nicolas Goaziou <n.goaziou@gmail.com> wrote on Sun, 28 Oct 2012 11:20:15
+0100:

> I would have integrated the (if ds ...) within the cond (i.e (cond
> ((not ds) nil) ...))

Oh yes!  Please do :-)

Justus