Re: [PATCH] Org Agenda Support Argument Collection for Custom Bulk Functions (was: Custom Bulk Functions With Prompt)

[Kevin Foley]

Kyle Meyer <kyle@kyleam.com> writes:
> Kevin Foley writes:
>
>> Side note I'm not sure your example would render properly regardless
>> since `my/bulk-action' and `my/args' aren't functions.
>
> I'm confused by this.  They were defined just above the text you
> quoted:

Sorry about that, I completely missed those.

> Aren't the pcase-let bindings missing a set of parentheses?

Hmm, you're right.  I swear I tested it and it was working so I'm not
sure if I accidentally dropped them or what.

I was hoping to add a test for this use case but it ended up seeming
like a rather large task with how much the function handles coupled with
my inexperience with ERT.

> However, I don't see using pcase-let here as an improvement.  Admittedly
> it's mostly subjective, but I think it's unhelpfully permissive for this
> use case:
>
>   (pcase-let ((`(,a ,b) (list 1 2 3 4)))
>     (list a b))  ; => (1 2)
>
> Fwiw, here's a relevant emacs.devel thread:
> https://lists.gnu.org/archive/html/emacs-devel/2015-07/msg00103.html
> (message ID: <jwvegkfoniv.fsf-monnier+emacs@gnu.org>)

I actually intended to be overly permissive in case additional options
are added in the future.  For example I was thinking it would be nice to
be able to assign a label to the custom action.

> My pcase-based suggestion, on top of your patch, is below.  If that
> looks okay to you, there's no need to resend; I can squash it in.

I'll defer to your pcase solution as I think my reasoning for using
pcase-let isn't particularly strong and I agree your suggestion is
cleaner.

Kevin Foley