emacs-orgmode
[Top][All Lists]
Advanced

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

Re: [O] Lexical binding bug in org-list.el?


From: Aaron Ecay
Subject: Re: [O] Lexical binding bug in org-list.el?
Date: Sat, 07 Nov 2015 21:30:58 +0000
User-agent: Notmuch/0.20.2+65~gbd5504e (http://notmuchmail.org) Emacs/25.0.50.2 (x86_64-unknown-linux-gnu)

Hi Nicolas,

2015ko azaroak 7an, Nicolas Goaziou-ek idatzi zuen:
> 
> Aaron Ecay <address@hidden> writes:
> 
>> On a broader scope, this is just one part of org that is written in this
>> style (one large let that defines many functions which call each other;
>> the body of the let is just a call into one of these functions).  This
>> isn’t idiomatic elisp (at least, I’ve never seen it outside org), and it
>> seems suboptimal for at least two reasons:
>> 1. The interface between the functions isn’t well-defined – they could
>> exchange information via arguments and/or by modifying variables
>> introduced by their containing let.
> 
> They are mutually recursive functions. I don't think that is
> un-idiomatic in the Lisp world. I don't think why the reason above would
> make them sub-optimal, too.

I guess it’s a scheme-ism.  <http://cl-su-ai.lisp.se/msg04741.html>.
Grepping for it in emacs sources, I see 4 uses in vc-* libraries, 1 in
octave, and 1 in emacs-lisp mode.  cl-labels is also implemented in
terms of letrec.  Org has 16 uses so far, all of them introduced since
the lexical binding change (or in any case since the last merge with
emacs).

I’m not saying we shouldn’t use it just because it’s rare, or a
scheme-ism.  Just satisfying my own curiosity out loud.  :)

> 
>> 2. It’s impossible to unit test the small let-bound functions.
> 
> Why would you want to unit test them? They are an implementation detail.
> Only `org-list-parse-list' should be tested here.

I don’t think we should unit-test only public API functions.  Indeed, I
find it easier to write unit tests for small functions, since I don’t
have to construct a lot of extra context to exercise all the corner
cases.

> 
>> In view of this, do you think it would be desirable in the long term to
>> refactor code like this into top-level functions?
> 
> No, I don't. I see no reason to pollute name space with these very
> specific functions.

I’m not sure I get it – interned symbols are cheap, and with the new-ish
double-dash convention for private functions users/downstream developers
can be pretty sure they don’t need to worry about them.

If it’s for us as org devs, I think the benefits outweigh the drawback
of having the extra function names to sift through in code completion
popups (which is the only one I can think of).  But opinions can differ,
and maybe I haven’t considered all the potential negatives.

[...]

> My take on the problem at hand is
> 
>   - change `org-list-parse-list' to provide a simpler output and update
>     Babel should to use that new output.
> 
>   - re-implement `org-list-to-subtree' using directly Elements, or even
>     string massaging.

OK, these both sound do-able.

> 
>   - start a poll on the ML to guess the potential user base for radio
>     lists. Depending on the results, we might either deprecate the whole
>     thing, as you suggest, or implement in a more powerful and clean
>     way. Note that first item implies `org-list-to-generic' should be
>     updated to handle new output for `org-list-parse-list'.

Given that no one’s ever asked for better radio lists despite us
advertising the feature and its unflattering comparison with more
powerful radio tables, I doubt there will be any interest.  But it’s
worth asking.  Should one of us start a new thread to ask this question?

-- 
Aaron Ecay



reply via email to

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