[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [O] Patch: org-agenda-skip-deadline-prewarning-if-scheduled prior to
From: |
Nicolas Goaziou |
Subject: |
Re: [O] Patch: org-agenda-skip-deadline-prewarning-if-scheduled prior to scheduled date |
Date: |
Wed, 17 Oct 2012 15:07:23 +0200 |
Hello,
address@hidden 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