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

[Nicolas Goaziou <n.goaziou@gmail.com>]

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