{"action":"created","issue":{"url":"https://api.github.com/repos/magnars/dash.el/issues/130","labels_url":"https://api.github.com/repos/magnars/dash.el/issues/130/labels{/name}","comments_url":"https://api.github.com/repos/magnars/dash.el/issues/130/comments","events_url":"https://api.github.com/repos/magnars/dash.el/issues/130/events","html_url":"https://github.com/magnars/dash.el/issues/130","id":62694157,"number":130,"title":"It's time to turn on lexical-binding","user":{"login":"tarsius","id":25046,"avatar_url":"https://avatars.githubusercontent.com/u/25046?v=3","gravatar_id":"","url":"https://api.github.com/users/tarsius","html_url":"https://github.com/tarsius","followers_url":"https://api.github.com/users/tarsius/followers","following_url":"https://api.github.com/users/tarsius/following{/other_user}","gists_url":"https://api.github.com/users/tarsius/gists{/gist_id}","starred_url":"https://api.github.com/users/tarsius/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tarsius/subscriptions","organizations_url":"https://api.github.com/users/tarsius/orgs","repos_url":"https://api.github.com/users/tarsius/repos","events_url":"https://api.github.com/users/tarsius/events{/privacy}","received_events_url":"https://api.github.com/users/tarsius/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":8,"created_at":"2015-03-18T13:28:40Z","updated_at":"2015-04-05T15:33:34Z","closed_at":null,"body":"It is often impossible to use a `-form` inside a `--form`. This is the case when `-form` expands to `--form` or `--some-other-form`.\r\n\r\nIn the following example `-first` expands to `--first` (whose expansion contains `--each-while` which binds `it`),\r\n\r\n```lisp\r\n(progn\r\n  (--first (--first (message \"--:-- %s %s (user error)\" it it)\r\n                    '(\"--inner\"))\r\n           '(\"--outer\"))\r\n\r\n  (--first (-first (lambda (-inner)\r\n                     (message \"--:-  %s %s (failure)\" it -inner))\r\n                   '(\" -inner\"))\r\n           '(\"--outer\"))\r\n\r\n  (-first (lambda (-outer)\r\n            (--first (message \" -:-- %s %s (success)\" -outer it)\r\n                     '(\"--inner\")))\r\n          '(\" -outer\"))\r\n  )\r\n```\r\n\r\nand produces\r\n\r\n```\r\n--:-- --inner --inner (user error)\r\n--:-   -inner  -inner (failure)\r\n -:--  -outer --inner (success)\r\n```\r\n\r\n* 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.\r\n* The third variant works correctly, `it` properly refers to the variable of the inner `--filter`. (Almost impossible to get that wrong).\r\n* The second variant should also work - but does not. `it`s value should be `\"--outer\"` but is `\"-inner\"`.\r\n\r\nThat 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`.\r\n\r\nThis 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`).\r\n\r\nEdit: 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.\r\n\r\nUsers 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 :-).\r\n\r\nFortunately 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).\r\n\r\nOf 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.\r\n"},"comment":{"url":"https://api.github.com/repos/magnars/dash.el/issues/comments/89793861","html_url":"https://github.com/magnars/dash.el/issues/130#issuecomment-89793861","issue_url":"https://api.github.com/repos/magnars/dash.el/issues/130","id":89793861,"user":{"login":"purcell","id":5636,"avatar_url":"https://avatars.githubusercontent.com/u/5636?v=3","gravatar_id":"","url":"https://api.github.com/users/purcell","html_url":"https://github.com/purcell","followers_url":"https://api.github.com/users/purcell/followers","following_url":"https://api.github.com/users/purcell/following{/other_user}","gists_url":"https://api.github.com/users/purcell/gists{/gist_id}","starred_url":"https://api.github.com/users/purcell/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/purcell/subscriptions","organizations_url":"https://api.github.com/users/purcell/orgs","repos_url":"https://api.github.com/users/purcell/repos","events_url":"https://api.github.com/users/purcell/events{/privacy}","received_events_url":"https://api.github.com/users/purcell/received_events","type":"User","site_admin":false},"created_at":"2015-04-05T15:33:34Z","updated_at":"2015-04-05T15:33:34Z","body":"> 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.\r\n\r\nI 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.\r\n\r\nAnd 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](http://melpa.org/archive.json) or the standard `archive-contents` file provides this information, in conjunction with [recipes.json](http://melpa.org/recipes.json)."}}