It's time to turn on lexical-binding

[tarsius]

It is often impossible to use a -form inside a --form. This is the case when -form expands to --form or --some-other-form.

In the following example -first expands to --first (whose expansion contains --each-while which binds it),

(progn
  (--first (--first (message "--:-- %s %s (user error)" it it)
                    '("--inner"))
           '("--outer"))

  (--first (-first (lambda (-inner)
                     (message "--:-  %s %s (failure)" it -inner))
                   '(" -inner"))
           '("--outer"))

  (-first (lambda (-outer)
            (--first (message " -:-- %s %s (success)" -outer it)
                     '("--inner")))
          '(" -outer"))
  )

and produces

--:-- --inner --inner (user error)
--:-   -inner  -inner (failure)
 -:--  -outer --inner (success)

• The first variant obviously cannot work, we refer to it twice expecting it to magically be different each time, i.e. it's a user error.
• The third variant works correctly, it properly refers to the variable of the inner --filter. (Almost impossible to get that wrong).
• The second variant should also work - but does not. its value should be "--outer" but is "-inner".

That is because -filter expands to --filter which uses --each-while which binds it around code which may contain uses of it which are intended to refer to a binding established outside of -filter (in this case the outer --filter.

This is a very serious flaw. Users cannot just use a -form inside a --form because -form may expand to --other-from which shadows the it from --form. This problem does not exist when -form does not expand to --other-form, or when -inner (or --inner even), is used in a place where the it of --outer is not in effect (E.g. inside COND of an outer --when-let).

Edit: in the second "safe" case, if the it let-binding is established carelessly, i.e. around code which is not actually supposed to have access to it, then this would fail too. I have a vague memory of that having happened to me, but don't remember what form it was.

Users should not have to know the implementation details to know which forms from dash nest and which do not. -map for example can be used as an inner form, because it just calls mapcar instead of expanding to --map (otherwise I would have used that as example :-).

Fortunately there is an easy fix - turn on lexical-binding in dash.el too. (Okay maybe that doesn't fix every such problem but the majority of such bugs would go away instantly).

Of course you could also refactor dash.el to fix each incarnation of this bug individually but the code would get quite a bit uglier and you would likely miss an incarnation or two. Forcing everyone to finally update to 24.1 is the lesser evil than having users wonder what the hell they are doing wrong, when infact the issue is a bug in dash.el not their understanding of how these forms are supposed to work. I for one have been wondering this quite a few times by now, but only now took the time to properly investitgate, I just didn't expect such a grave bug to linger for so long in such a popular library.

[Fuco1]

See also #56 (comment)

I'm ok with lexical binding, though it would require some changes to api (for example, -compare-fn is sometimes passed dynamically). But that should simply be refactored into -by version which takes the function as argument anyway (see the OP of post linked above)

[magnars]

Yeah, sounds like a good reason to justify abandoning support for Emacs 23.

[occidens]

In addition to addressing the problem that @tarsius identifies, enabling lexical binding will also open up the range of possibilities for addressing #43, since closures could simplify iterating over non-list sequences (see also #80). Of course thats not the only way to solve it, but at least we'd have all of Emacs Lisp's current tools at our disposal.

That said, does anyone have a sense of how many people at this point would be affected by this change? I suppose the only ones who would be really caught off guard would be those who are using Emacs 23 and getting dash.el using package.el.

[magnars]

It was @purcell who convinced me to keep supporting E23 in the first place. Any thoughts, Steve?

[purcell]

I feel like requiring Emacs 24 is a very big step, since possibly hundreds of packages directly or transitively depend on dash, and all of those will cease to be installable on Emacs 23.

(I have no opinion on the specifics of this issue, other than that anaphoric macros are the work of the devil. ;)

[rejeep]

Sometimes the only way to get people to upgrade is to break their stuff. I say drop Emacs 23 support!

[tarsius]

24.1 was released three years ago, see https://www.gnu.org/software/emacs/history.html. It's laudable that we don't needlessly make it harder for users, but at some point it has to stop. After all "installing Emacs manually" is a skill worth having.

@purcell Yes, it is a big step, but it's about time we take it. You instructing new elisp authors to preserve backward compatibility when they submit their first creation, doesn't exactly make it easier for us maintainers of popular libraries/packages. Not saying it was wrong of you to do that, just that it has downsides too: what could have been a smooth transition now is something that will affect many users at once, when the big players finally start moving.

I think it would be a good idea for us more active maintainers of third-party extensions to take this step together - so that the blame doesn't fall on which ever popular package takes it first.

Magit's next release, 2.1.0, will require Emacs >= 24.2 (and Git >= 1.9.4 (sadly still)). So you could just sit it out, I suppose :-)

I have no opinion on the specifics of this issue, other than that anaphoric macros are the work of the devil. ;)

https://www.youtube.com/watch?v=ZRXGsPBUV5g

[purcell]

You instructing new elisp authors to preserve backward compatibility when they submit their first creation, doesn't exactly make it easier for us maintainers of popular libraries/packages. Not saying it was wrong of you to do that, just that it has downsides too: what could have been a smooth transition now is something that will affect many users at once, when the big players finally start moving.

I generally tell authors who are using minor bits of new functionality (setq-local, declaring lexical-binding etc) that they either need to depend on (emacs "24") or make compatibility changes, then leave the decision up to them. I actually don't care what they do, as long as the declared dependencies are accurate.

And I'm not saying that dash shouldn't be updated to require Emacs 24: just warning that it's going to affect a lot of stuff. It would be relatively straightforward to find the github usernames of authors whose MELPA packages depend directly or transitively on dash, and CC them on this github issue, so perhaps that would be a helpful warning and step along the way. The archive.json or the standard archive-contents file provides this information, in conjunction with recipes.json.

[tarsius]

Please consider turning on lexical-binding now, even though dash still supports older Emacsen with don't know about lexical-binding and will just ignore it. That way this bug is fixed for users of recent Emacsen and maintainers of packages which depend on dash can additionally depend on Emacs v24.1 if they want to be sure their package is not affected by this issue.

I'm ok with lexical binding, though it would require some changes to api (for example, -compare-fn is sometimes passed dynamically).

That's not a problem because this variable is properly defined using defvar and that causes it to have dynamic scope, even when lexical-binding is t.

[magnars]

If we can fix a bug by adding the lexical binding pragma, without breaking older Emacsen, I'm certainly all for it. Is that all it takes, @tarsius?

What I'd really like is to move dash-functional into dash core as part of a breaking change 3.0 release, which would then drop support for E23. Do you have any statistics about E23 usage from Melpa, @purcell?

[purcell]

If we can fix a bug by adding the lexical binding pragma, without breaking older Emacsen, I'm certainly all for it. Is that all it takes, @tarsius?

Yes. Just be careful that you don't actually start relying on lexical binding behaviour before you're ready to declare a dependency on Emacs 24.

What I'd really like is to move dash-functional into dash core as part of a breaking change 2.0 release, which would then drop support for E23. Do you have any statistics about E23 usage from Melpa, @purcell?

No, we have no way to tell how many people are using E23. It'd probably be worth figuring out the intersection of the packages which depend directly or transitively on dash and the packages which don't already depend directly or transitively on E24 - they would be the ones affected.

[magnars]

I have added the lexical binding pragma to dash.el and released 2.12.1 with this fix. Thanks for the insight, @purcell and @tarsius.

[tarsius]

😋 Thanks!

We will take full advantage of lexical binding in an upcoming 2.0

2.0?! That should be 3.0 right?

[tarsius]

2.0

Ah, I see you already noticed that typo.

[Fuco1]

Awesome stuff, thanks for moving this forward :)

[CestDiego]

Thank you for keeping maintaining this :)

[Fuco1]

Anyone has something more to say, or can we close this? Lexical binding has been on for half a year and no problems were reported so far.

[tarsius]

Hasn't caused any issues for me. Thanks, by the way!

[magnars]

👍

[basil-conto]

What I'd really like is to move dash-functional into dash core as part of a breaking change 3.0 release, which would then drop support for E23.

Emacs 23 support was dropped in version 2.14.0 over a year ago (e9919f6), so how do people feel about merging dash and dash-functional now?

[magnars]

How does it break on E23? lør. 17. aug. 2019 kl. 18:17 skrev Basil L. Contovounesios < notifications@github.com>:

…

What I'd really like is to move dash-functional into dash core as part of a breaking change 3.0 release, which would then drop support for E23. Emacs 23 support was dropped in version 2.14.0 over a year ago (e9919f6 <e9919f6>), so how do people feel about merging dash and dash-functional now? — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#130?email_source=notifications&email_token=AACA4OPBCNRETQK6PWRKDN3QFAQBRA5CNFSM4A6CR4F2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4QOTQA#issuecomment-522250688>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AACA4OJP5U2IIHVXWZSP3LLQFAQBRANCNFSM4A6CR4FQ> .

[basil-conto]

How does it break on E23?

How does what break?

[magnars]

If we make this change, how does it break? What stops working in E23? lør. 17. aug. 2019 kl. 20:29 skrev Basil L. Contovounesios < notifications@github.com>:

…

How does it break on E23? How does what break? — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#130?email_source=notifications&email_token=AACA4OLMXGIUFZSBM2JDINTQFA7Q3A5CNFSM4A6CR4F2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4QRAGY#issuecomment-522260507>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AACA4OPSPCMIO6I7PBA5IN3QFA7Q3ANCNFSM4A6CR4FQ> .

[Fuco1]

As far as I understand neither package works with Emacs older than 24. So merging them will change nothing. If someone uses them on E23 they probably vendor the libraries already since Melpa does not allow installing older versions and is by far the most used channel. Actually I'm not even sure if you can use Melpa (package.el) on E23.

[purcell]

As far as I understand neither package works with Emacs older than 24. So merging them will change nothing. If someone uses them on E23 they probably vendor the libraries already since Melpa does not allow installing older versions and is by far the most used channel. Actually I'm not even sure if you can use Melpa (package.el) on E23.

Agreed, I think you can safely assume that nobody is trying to use any subset of MELPA on E23.

[magnars]

Thanks for chiming in, Steve. That's useful to know. So what can we make better with dropped E23-support without breaking people's code on higher Emacs versions?

…

On Sun, Aug 18, 2019 at 4:25 AM Steve Purcell ***@***.***> wrote: As far as I understand neither package works with Emacs older than 24. So merging them will change nothing. If someone uses them on E23 they probably vendor the libraries already since Melpa does not allow installing older versions and is by far the most used channel. Actually I'm not even sure if you can use Melpa (package.el) on E23. Agreed, I think you can safely assume that nobody is trying to use any subset of MELPA on E23. — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#130?email_source=notifications&email_token=AACA4OL4CC6MKSO3HE2TFFDQFCXHZA5CNFSM4A6CR4F2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4QXBJY#issuecomment-522285223>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AACA4OKDA6GJLM7AZNK34ODQFCXHZANCNFSM4A6CR4FQ> .

[basil-conto]

So what can we make better with dropped E23-support without breaking people's code on higher Emacs versions?

My original question was whether merging dash and dash-functional would be desirable now that both assume lexical-binding. Previously, when dash still supported E23, merging the two was not possible. The benefit of merging them now is increased consistency and simplicity at the package level.

The complication in merging them is how to signal that the new merged dash supplants the old standalone dash-functional. I would hope that ELPA provides a way to specify such a conflict between specific versions of different packages.

[magnars]

That’s not a good enough reason to break lots of code that we have no control over. I suggest we close this issue and accept that we have two packages. søn. 18. aug. 2019 kl. 16:47 skrev Basil L. Contovounesios < notifications@github.com>:

…

So what can we make better with dropped E23-support without breaking people's code on higher Emacs versions? My original question was whether merging dash and dash-functional would be desirable now that both assume lexical-binding. Previously, when dash still supported E23, merging the two was not possible. The benefit of merging them now is increased consistency and simplicity at the package level. The complication in merging them is how to signal that the new merged dash supplants the old standalone dash-functional. I would hope that ELPA provides a way to specify such a conflict between specific versions of different packages. — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#130?email_source=notifications&email_token=AACA4OJYGNO3R46F7IOH5XTQFFOHTA5CNFSM4A6CR4F2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4RBQ7A#issuecomment-522328188>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AACA4OLUTJADKSIMOO4SHIDQFFOHTANCNFSM4A6CR4FQ> .

[basil-conto]

That’s not a good enough reason to break lots of code that we have no control over.

How would merging the two packages break existing code?

I suggest we close this issue

Actually, if this discussion is going to continue at all, it deserves its own new issue.

and accept that we have two packages.

I'm sure everyone's already accepted this, but that doesn't mean things can't evolve. ;) The whole point of a separate package AIUI was due to lexical-binding compatibility. Now that this is no longer an issue, why not revisit dash packaging with an eye towards improvement? (No-one has suggested an explicitly backward-incompatible change so far.)

[magnars]

I’m happy to accept evolution that does not cause breakage. If this is can be done with no breaking changes, I’m not opposed to the change. søn. 18. aug. 2019 kl. 20:02 skrev Basil L. Contovounesios < notifications@github.com>:

…

That’s not a good enough reason to break lots of code that we have no control over. How would merging the two packages break existing code? I suggest we close this issue Actually, if this discussion is going to continue at all, it deserves its own new issue. and accept that we have two packages. I'm sure everyone's already accepted this, but that doesn't mean things can't evolve. ;) The whole point of a separate package AIUI was due to lexical-binding compatibility. Now that this is no longer an issue, why not revisit dash packaging with an eye towards improvement? (No-one has suggested an explicitly backward-incompatible change so far.) — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#130?email_source=notifications&email_token=AACA4OPLLVPEAUZI6TDKXETQFGFDHA5CNFSM4A6CR4F2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4RE74Q#issuecomment-522342386>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AACA4ONRY7XPQ3HR4QBYMHDQFGFDHANCNFSM4A6CR4FQ> .

[Fuco1]

If both packages implement same functions than what gets used depends entirely on the random order in which Emacs loads these packages. So we need to fix this by reliably only running the functions from the main package.

What we could do is release a new version of dash-functional which would just require dash thus being essentialy a no-op. This would require people updating both at the same time though.

Also we could require dash-functional from newer dash and then (since it loads before dash loads) it would just overwrite the functions with the ones from itself.

I'm not sure if this works how I imagine it, though I'm pretty sure it does and therefore it could actually work in a BC manner. Someone ought to test first though.