emacs-orgmode
[Top][All Lists]
Advanced

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

Re: [O] [PATCH] Fix clocktable scope parameter


From: Eduardo Bellani
Subject: Re: [O] [PATCH] Fix clocktable scope parameter
Date: Thu, 15 Dec 2016 21:37:46 -0200
User-agent: mu4e 0.9.17; emacs 24.4.1

Hello Nicolas.

> Thank you. However, this is not a "fix" per se.

I agree. I should be more explicit as to what this is.

>> * lisp/org-clock.el (org-dblock-write:clocktable): Make sure to eval
>>   the scope if it is a lisp expression, or to return the scope if it
>>   is just a list.
>
> This comment is no longer accurate.

Actually, I think the above is accurate, but poorly expressed. As I'll
try to show on the next email. But before that, I though of discussing
this a bit here.

The old behavior was an eval on a form if that form was not a list of
strings. The implicit expectation was for a list of strings to be
returned by that eval call.

The above seems to be a raw attempt to evaluate a function form. In that
case, it seems more elegant to be more explicit and do a

     (apply (car scope) (cdr scope))


This also allows for passing arguments to the function without using the
full power of eval.

What do you guys think?

Nicolas Goaziou writes:

> Hello,
>
> Eduardo Bellani <address@hidden> writes:
>
>> org-clock.el: Fix clocktable scope parameter
>
> Thank you. However, this is not a "fix" per se.
>
>> * lisp/org-clock.el (org-dblock-write:clocktable): Make sure to eval
>>   the scope if it is a lisp expression, or to return the scope if it
>>   is just a list.
>
> This comment is no longer accurate.
>
>> +             (function-name) @r{scan the list of files returned by calling 
>> this function.}
>
> Why should the function be within a list? I suggest something like
>
>   function @r{list of files returned by calling the function with no argument}
>
>> -                ((pred consp) scope)
>> +                    ((and (pred #'listp) (pred (lambda (scope) (symbolp 
>> (car scope)))))
>> +                 (funcall (car scope)))
>
> Per above, it should be
>
>   ((pred functionp) (funcall scope))
>
> Regards,

Attachment: signature.asc
Description: PGP signature


reply via email to

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