Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

"cl-flet" macros in recent emacsen break yas--fom-set-next-from #330

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants

yyr commented Nov 8, 2012

No description provided.

Contributor

rolandwalker commented Nov 8, 2012

Your issue may be related to my pull request #324. Can you tell me your exact Emacs version so that I can duplicate?

yyr commented Nov 8, 2012

Hi,

GNU Emacs 24.2.50.2 (x86_64-unknown-linux-gnu, GTK+ Version 2.20.1)
of 2012-11-07 on okhotsk19

Owner

joaotavora commented Nov 8, 2012

Thanks @rolandwalker for following up on this. I'm afraid I have little time for yasnippet lately and have added https://github.com/capitaomorte/yasnippet#important-note-regarding-bug-reporting. Let's keep this pull request open for followin up on this issue though.

Contributor

rolandwalker commented Nov 8, 2012

It's a pre-release Emacs, which was exactly what I was aiming to address in #324. By eyeball @yyr's change does the right thing, but the alias must also change, will duplicate.

Owner

joaotavora commented Nov 9, 2012

Ok thanks. Apparently the macro is called cl-flet* in the pre-release, but the reason the new macro failed is that it should really be labels here: we don't need the dynamic behaviour of the non-standard elisp flet here... Maybe labels is exactly the same as cl-flet*.... For some reason flet wasn't breaking before. Is labels also going to be renamed?

Contributor

rolandwalker commented Nov 9, 2012

Yes, labels will be obsoleted by cl-labels, and you could use cl-labels there. You would need some backward-compatibility aliases as with flet. What a pain!

But @yyr made a good catch: they are adding cl-flet* in addition to cl-flet, and cl-flet* actually has closer semantics to flet (definitions can refer to previous ones). The obsolescence docs point to the wrong one.

So my #324 should be revised anyway, which would cover this.

@yyr, I recommend you change every occurrence of the symbol cl-flet to cl-flet* in yasnippet.el in your fix branch. I have tested that from version 22.3 - nightly snapshot. Then @capitaomorte can decide how to handle the issue.

yyr commented Nov 9, 2012

@capitaomorte Sorry for not giving enough background while submitting bug. I will follow procedures next time.

@rolandwalker Sure I will add.

Owner

joaotavora commented Nov 9, 2012

@rolandwalker It should really be labels. emacs devels are changing everything into "cl-" and that is indeed a pain, but aside from lobbying against it (and I am reluctant to do that) there is no alternative. Maybe we can create a yasnippet-compat.el package that adds cl-* conditionally and also some other functions (see the "hacks" section). But only do that once 24.3 is released.

@yyr no problem, that's just an automated response I built (cause I really have little time for many back and forth emails), and you were the unlucky first to get it.

yyr commented Nov 9, 2012

So should we change to cl-labels with a compatibility macro for now.? or leave as it is until 24.3 release ?

Owner

joaotavora commented Nov 9, 2012

I think we should leave this as is until 24.3 release. I'll leave this pull request open though. Feel free to add commits to this pull request (maybe some commits that puts all the compatiblity macros and hacks in yasnippet.el to a new yasnippet-compat.el file, that would only be loaded for emacsen < 24.3).

Or maybe just leave it be, I've posted the emacs-devel mailing list to get an idea of what they mean to do with this.

Owner

joaotavora commented Nov 11, 2012

OK, I asked on emacs-devel and the whole conversation in in this thread http://comments.gmane.org/gmane.emacs.devel/151211.

This means:

  • we should use labels for every situation except the for those in yas-compile-directory. labels is not going away so soon,
  • the better fix in yas-compile-directory, according to Stefan, is to use defadvice, described in the thread.
  • cl-flet in the newest pretest does not provide the dynamic rebinding behaviour same behaviour as flet, so @rolandwalker your fix of #324 is broken. The tests should have caught it, but they are using flet as well to catch misbehaviour (anyway I don't know how the tests ran in the latest pretest as they still use flet).

Emacs devels talk of a forward compatibility lib, so maybe @rolandwalker you want to contribute there.

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