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

[Nicolas Goaziou]

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