emacs-orgmode
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [O] [PATCH] org-agenda: Add 'none setting for org-agenda-overriding-


From: Nicolas Goaziou
Subject: Re: [O] [PATCH] org-agenda: Add 'none setting for org-agenda-overriding-header
Date: Wed, 23 Aug 2017 10:48:28 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux)

Adam Porter <address@hidden> writes:

> Here are the patches.  Please let me know if any other changes are
> needed.

Thank you! Comments follow.

> +(defmacro org-agenda--insert-overriding-header (&key default)

There is no "&key" in `defmacro'.

It should be (default).

> +  "Insert header into agenda view depending on value of 
> `org-agenda-overriding-header'.
> +If the empty string, don't insert a header.  If any other string,
> +insert it as a header.  If nil, insert DEFAULT, which should
> +evaluate to a string."
> +  (declare (debug (&key form)))

It needs to be updated according to the above.

> +                         ;; Format week number span
> +                         (cond ((< (- d2 d1) 350)
> +                                (if (= w1 w2)
> +                                    (format " (W%02d)" w1)
> +                                  (format " (W%02d-W%02d)" w1 w2)))
> +                               (t ""))

  (cond ((<= 350 (- d2 d1)) "")
        ((= w1 w2) (format " (W%02d)" w1))
        (t (format " (W%02d-W%02d)" w1 w2)))

> -                 (let ((n 0) s)
> -                   (mapc (lambda (x)
> -                           (setq s (format "(%d)%s" (setq n (1+ n)) x))
> -                           (if (> (+ (current-column) (string-width s) 1) 
> (frame-width))
> -                               (insert "\n                     "))
> -                           (insert " " s))
> -                         kwds))
> +                 (cl-loop for keyword in kwds
> +                          and num from 1
> +                          for string = (format "(%d)%s" num keyword)
> +                          when (> (+ (current-column) (string-width string) 
> 1)
> +                                  (window-width))
> +                          do (insert "\n                     ")
> +                          do (insert " " string))

Ouch. Why `cl-loop' over `dolist'? Also it looks wrong since the last
`do' is not always executed? (or is it?).

I know there is more than one way to skin a cat, but I'd rather use
a straightforward one:

  (let ((n 0))
    (dolist (k kwds)
      (let ((s (format "(%d)%s" (cl-incf n) k)))
        (when (> (+ (current-column) (string-width s) 1) (frame-width))
          (insert "\n                     "))
        (insert " " s))))


Regards,



reply via email to

[Prev in Thread] Current Thread [Next in Thread]