emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
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

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