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

[Nicolas Goaziou <mail@nicolasgoaziou.fr>]

Aaron Ecay <aaronecay@gmail.com> 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.

> 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.

> 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.

> If it was up to me, there would be only three kinds of code touching
> lists in org:
> - code that manipulates org-elements objects and (where necessary)
>   converts them to strings using the exporter
> - a single function in ob-core that takes org-elements lists and converts
>   them into a simple nested list format, for use as input to babel code
>   blocks
> - a single function in ob-core that is the inverse of the one above, for
>   processing the output of code blocks
>
> I think this implies:
> - org-list-parse-list deprecated/removed
> - org-list-to-generic deprecated/removed
> - callers of these two functions updated to use org-elements/ox (with new
>   helper functions introduced for this purpose if it seems necessary)
>
> The simplicity gains from this would be:
> - fewer functions in the public API of org (org-list-parse-list is replaced
>   by preexisting org-elements functions, and org-list-to-generic by ox
>   functions)
> - hopefully less code to maintain (in terms of lines), though it remains
>   to be seen how much or if at all these changes actually shrink the
>   code base
> - all org code manipulating lists uses a single format (org-elements).
>   Babel code blocks are not org code (and often not in elisp), so they
>   are the only thing that gets to use a different format
> - the plist format controlling org-list-to-generic goes away

Like for radio tables, radio lists are meant to be used in foreign
buffers, i.e., buffers not in Org mode. They have almost the same
requirements as Babel code blocks, which explains why they share the
same representation. The same happens with radio tables, which provide
their own representation to Babel, through `org-table-to-lisp'. If only
for symmetry, Org List should be the provider of any list
representation.

Moreover, `org-table-to-generic' is a powerful and flexible thin wrapper
around Org Export. In particular, `org-table-to-latex' is quite
different from calling `org-latex-export-to-latex' on an Org list.
Similarly, `org-list-to-generic' could be implemented as such, and,
therefore, may not be discarded too easily.

I'm not married to radio lists, and I wouldn't mind them being removed,
but, if they are to be kept, they should at least be implemented
correctly, not left in their current dumbed down state. Unfortunately,
if I had to guess, I'd say that radio lists have no user, unlike to
radio tables (which have their own video on a popular site).

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.

  - 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'.


Regards,