From: Aaron Ecay <aaronecay@gmail.com>
To: Nicolas Goaziou <mail@nicolasgoaziou.fr>,
Kaushal Modi <kaushal.modi@gmail.com>
Cc: emacs-org list <emacs-orgmode@gnu.org>
Subject: Re: Lexical binding bug in org-list.el?
Date: Sat, 07 Nov 2015 11:54:44 +0000 [thread overview]
Message-ID: <87r3k2t46z.fsf@gmail.com> (raw)
In-Reply-To: <87ziyq7j8t.fsf@nicolasgoaziou.fr>
Hi Nicolas,
2015ko azaroak 7an, Nicolas Goaziou-ek idatzi zuen:
>
> Hello,
>
> Aaron Ecay <aaronecay@gmail.com> 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/87d2anougv.fsf@gmail.com>. 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
next prev parent reply other threads:[~2015-11-07 11:54 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-11-06 19:43 Lexical binding bug in org-list.el? Kaushal Modi
2015-11-06 19:47 ` Kaushal Modi
2015-11-06 20:45 ` Aaron Ecay
2015-11-06 21:13 ` Kaushal Modi
2015-11-07 0:20 ` Nicolas Goaziou
2015-11-07 11:54 ` Aaron Ecay [this message]
2015-11-07 16:48 ` Nicolas Goaziou
2015-11-07 21:30 ` Aaron Ecay
2015-11-08 14:57 ` Nicolas Goaziou
2015-11-08 19:55 ` Aaron Ecay
2015-11-09 15:23 ` Kaushal Modi
2015-11-11 9:33 ` Nicolas Goaziou
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.orgmode.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87r3k2t46z.fsf@gmail.com \
--to=aaronecay@gmail.com \
--cc=emacs-orgmode@gnu.org \
--cc=kaushal.modi@gmail.com \
--cc=mail@nicolasgoaziou.fr \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).