Avoid using flet which doesn't work because cl is not required. #592

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
4 participants
Contributor

thierryvolpiatto commented Mar 18, 2013

defkey is not working because cl is not required.
Actually best practice is to avoid using flet by using either lambda or requiring cl-lib and use cl-flet with lexical-bindings enabled.

Contributor

azuk commented Mar 18, 2013

There is (eval-when-compile (require 'cl)) at the top of magit-key-mode.el and since flet is a macro, it ought to be sufficient. I just tested and I didn't see any problems. Note that magit.el also requires cl like that and uses flet, so if there was a problem with it, presumably magit.el would have to be changed as well.

Contributor

thierryvolpiatto commented Mar 18, 2013

Hannu Koivisto notifications@github.com writes:

There is (eval-when-compile (require 'cl)) at the top of
magit-key-mode.el

Yes, didn't see it, sorry.
I change this because I had errors saying defkey was not defined this
morning, probably bad compilation after updating.

and since flet is a macro, it ought to be sufficient. I just tested
and I didn't see any problems. Note that magit.el also requires cl
like that and uses flet, so if there was a problem with it, presumably
magit.el would have to be changed as well.

Yes I see flet is used actually in two places only, using lambda there
too would be better.


Reply to this email directly or view it on GitHub:
#592 (comment)

Thierry
Get my Gnupg key:
gpg --keyserver pgp.mit.edu --recv-keys 59F29997

Member

sigma commented Mar 18, 2013

note that the new-cl-compliant-approach that's promoted by emacs 24.3 is living in the 'next' branch for now (in particular magit.el doesn't require cl anymore)

Member

sigma commented Mar 21, 2013

I think this isn't relevant anymore now that I've merged back the cl-lib stuff that was in next.
Feel free to reopen if further action is needed

sigma closed this Mar 21, 2013

tarsius added this to the 2.1.0 milestone Feb 17, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment