[Top][All Lists]

[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 11:54:44 +0000
User-agent: Notmuch/0.20.2+65~gbd5504e (http://notmuchmail.org) Emacs/ (x86_64-unknown-linux-gnu)

Hi Nicolas,

2015ko azaroak 7an, Nicolas Goaziou-ek idatzi zuen:
> Hello,
> Aaron Ecay <address@hidden> writes:
>> The org-list code is a mess, and I think we should hold off on converting
>> it to lexical scoping until it can be refactored in a more dedicated
>> way.
> It seems a bit strong considering there's only one issue so far. I don't
> think the code is a mess, but "Send and receive lists" part clearly
> needs an overhaul.
>> Nonetheless I include the patch, in case it’s helpful to anyone.
> Lifting `org-list--get-text', `org-list--parse-item', etc. isn't
> necessary. 

I’ll defer to your judgment, of course, especially where it comes to
fixing the immediate issue.

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.
2. It’s impossible to unit test the small let-bound functions.

In view of this, do you think it would be desirable in the long term to
refactor code like this into top-level functions?

> I just forgot a `letrec' in `org-list-parse-list'.

Oh ok.  I had never seen letrec before these recent lexical binding
changes, so I wasn’t familiar with how it could fix the problems.  But
I’m glad it can.

> [...]

I originally wrote individual responses to the points in the rest of
your email, but then I realized that it’s slightly more than a year
since we had an eerily similar discussion:
<http://mid.gmane.org/address@hidden>.  I don’t think we
reached any conclusion then.  To avoid the same thing happening, I’m
going to try to step back and sketch the vision I have about this code
from a high-level perspective.

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

You said:

> Actually, `org-list-to-generic' in its current form isn't adequate for
> anything. Even `org-list-to-*' functions do not use it.


> we can re-implement `org-list-to-subtree' without using
> `org-list-to-generic'.

Which I think means we are on more or less the same page about
org-list-to-generic.  You also said:

> `org-list-parse-list' should be simplified, too, as its current return
> value is useless (e.g., nothing uses [CBON] anymore in the code base).

Here I’m more radical than you: since the current return value is useless
(I agree) and hardly used anywhere, my conclusion is to just get rid of
the function, rather than trying to invent a way in which it could be

I hope that having laid it out in this way helps to understand what I
have in mind.  I’d of course be very glad to hear what your thoughts
are, and hopefully we’ll be able to work out how to proceed.

Aaron Ecay

reply via email to

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