[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
- [O] Lexical binding bug in org-list.el?, Kaushal Modi, 2015/11/06
- Re: [O] Lexical binding bug in org-list.el?, Kaushal Modi, 2015/11/06
- Re: [O] Lexical binding bug in org-list.el?, Aaron Ecay, 2015/11/06
- Re: [O] Lexical binding bug in org-list.el?, Kaushal Modi, 2015/11/06
- Re: [O] Lexical binding bug in org-list.el?, Nicolas Goaziou, 2015/11/06
- Re: [O] Lexical binding bug in org-list.el?,
Aaron Ecay <=
- Re: [O] Lexical binding bug in org-list.el?, Nicolas Goaziou, 2015/11/07
- Re: [O] Lexical binding bug in org-list.el?, Aaron Ecay, 2015/11/07
- Re: [O] Lexical binding bug in org-list.el?, Nicolas Goaziou, 2015/11/08
- Re: [O] Lexical binding bug in org-list.el?, Aaron Ecay, 2015/11/08
- Re: [O] Lexical binding bug in org-list.el?, Kaushal Modi, 2015/11/09
- Re: [O] Lexical binding bug in org-list.el?, Nicolas Goaziou, 2015/11/11