emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* org-agenda-bulk-custom-functions Customize mismatch
@ 2016-05-08 13:57 Phil Hudson
  2016-05-10 21:34 ` Nicolas Goaziou
  0 siblings, 1 reply; 2+ messages in thread
From: Phil Hudson @ 2016-05-08 13:57 UTC (permalink / raw)
  To: emacs-orgmode

Another defcustom mismatched with the code.

At line 9962 of org-agenda.el (20160502 release) we have:

  (setq cmd (list (cadr (assoc action org-agenda-bulk-custom-functions)))

This assumes that `org-agenda-bulk-custom-functions` is a
list-of-two-element-lists alist. Thus, the `cadr` form returns (should
return) the second element of one two-element list from the alist.

However, the defcustom is declared as type alist, which in defcustom
means (by default) a list-of-conses alist.

See if you agree with my thinking about this. This means that the code
must always fail if the Customize UI is used to populate
`org-agenda-bulk-custom-functions`. That in turn implies that (to a
first approximation) zero users apart from me have tried to declare a
custom bulk agenda command with its own custom key via Customize. Oh
well.

If my diagnosis is correct, then I see two ways to fix it: either change
the defcustom to create a list-of-two-element-lists alist, or change the
code so that it handles a list-of-conses alist.

The first approach is supported by defcustom and gives us the chance to
be both more descriptive and more prescriptive. It shouldn't break
anything that isn't in fact already broken.

The second approach is as simple as changing that `cadr` to a `cdr`, but
it does risk breaking working configs where the alist was populated
procedurally.

I propose the first. Here's a patch.

-- 
Phil Hudson                   http://hudson-it.ddns.net
@UWascalWabbit                 PGP/GnuPG ID: 0x887DCA63


--- org-20160502/org-agenda.el	2016-05-07 10:18:22.000000000 +0100
+++ org-mode-mod/org-agenda.el	2016-05-08 14:39:52.000000000 +0100
@@ -1999,7 +1999,7 @@
 With selected entries in an agenda buffer, `B R' will call
 the custom function `set-category' on the selected entries.
 Note that functions in this alist don't need to be quoted."
-  :type 'alist
+  :type '(alist :key-type character :value-type (group function))
   :version "24.1"
   :group 'org-agenda)
 

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: org-agenda-bulk-custom-functions Customize mismatch
  2016-05-08 13:57 org-agenda-bulk-custom-functions Customize mismatch Phil Hudson
@ 2016-05-10 21:34 ` Nicolas Goaziou
  0 siblings, 0 replies; 2+ messages in thread
From: Nicolas Goaziou @ 2016-05-10 21:34 UTC (permalink / raw)
  To: Phil Hudson; +Cc: emacs-orgmode

Hello,

Phil Hudson <phil.hudson@iname.com> writes:

> The first approach is supported by defcustom and gives us the chance to
> be both more descriptive and more prescriptive. It shouldn't break
> anything that isn't in fact already broken.
>
> The second approach is as simple as changing that `cadr` to a `cdr`, but
> it does risk breaking working configs where the alist was populated
> procedurally.
>
> I propose the first. Here's a patch.

Looks good. Thank you.

Could you send it using "git format-patch". Also, could you provide an
appropriate commit message? See
<http://orgmode.org/worg/org-contribute.html> for details.

Regards,

-- 
Nicolas Goaziou

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2016-05-10 21:34 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-08 13:57 org-agenda-bulk-custom-functions Customize mismatch Phil Hudson
2016-05-10 21:34 ` Nicolas Goaziou

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).