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 11:54:44 +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:
> 
> 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
  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

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.

And

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

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]