From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nicolas Goaziou Subject: Re: [PATCH] org-agenda: Add 'none setting for org-agenda-overriding-header Date: Sat, 02 Sep 2017 09:49:03 +0200 Message-ID: <87wp5h1qxc.fsf@nicolasgoaziou.fr> References: <87k22416un.fsf@alphapapa.net> <87bmnep8yz.fsf@nicolasgoaziou.fr> <8760dmynkk.fsf@alphapapa.net> <871so9nt04.fsf@nicolasgoaziou.fr> <87pobrc5vp.fsf@alphapapa.net> <87wp5yir25.fsf@nicolasgoaziou.fr> <87a82rkqmf.fsf@alphapapa.net> <8760dfko9f.fsf@alphapapa.net> <87lgma1xgj.fsf@nicolasgoaziou.fr> <87efrphle7.fsf@alphapapa.net> Mime-Version: 1.0 Content-Type: text/plain Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:42250) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1do3B2-0003ZR-4G for emacs-orgmode@gnu.org; Sat, 02 Sep 2017 03:49:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1do3Ax-0004bp-Bk for emacs-orgmode@gnu.org; Sat, 02 Sep 2017 03:49:12 -0400 Received: from relay4-d.mail.gandi.net ([2001:4b98:c:538::196]:50728) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1do3Ax-0004b3-5a for emacs-orgmode@gnu.org; Sat, 02 Sep 2017 03:49:07 -0400 In-Reply-To: <87efrphle7.fsf@alphapapa.net> (Adam Porter's message of "Fri, 01 Sep 2017 21:41:52 -0500") List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org Sender: "Emacs-orgmode" To: Adam Porter Cc: emacs-orgmode@gnu.org Hello, Adam Porter writes: > You're right, that's why IIRC I used cl-defmacro originally. The > issue here seems to be using the keyword argument. It seemed like a > good idea to specify it with the ":default: keyword argument for two > reasons: > > 1. To make it explicitly clear that the body of code passed to the > macro is only the default header, not necessarily the one that will be > used. > > 2. To make it easier to add other arguments in the future. This is an internal macro. We can add anything without problem in the future. For now, I really think `defmacro' is enough. >>> + ;; 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))) > > I see. That doesn't seem to be exactly the same logic as the nested if, Why do you think it isn't equivalent? >>> - (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?). > > Yes, it is always executed: the "when" only applies to the next clause, > and I tested it to be sure, both by executing it and expanding the > macro. I've used cl-loop a lot lately, so it is familiar to me. This looks too much magical to me. Both `do' are treated differently. >> 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)))) > > I guess this is a matter of style, as I prefer the cl-loop version, > which doesn't hide the incrementing in the format call You must be kidding. `cl-loop' hides a lot of things. In any case, you can increment the counter above the `s' binding if you want to. > and avoids another level of nesting just for the counter variable. :) > But if you want me to use the dolist instead, it's up to you. I'd rather have `dolist' yes. That's more basic and therefore, easier to understand. Thank you. Regards, -- Nicolas Goaziou