Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

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

yyr
Copy link

@yyr yyr commented Nov 8, 2012

No description provided.

@rolandwalker
Copy link
Contributor

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

@yyr
Copy link
Author

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

@joaotavora
Copy link
Owner

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.

@rolandwalker
Copy link
Contributor

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.

@joaotavora
Copy link
Owner

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?

@rolandwalker
Copy link
Contributor

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
Copy link
Author

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.

@joaotavora
Copy link
Owner

@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
Copy link
Author

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 ?

@joaotavora
Copy link
Owner

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.

@joaotavora
Copy link
Owner

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 future-proof against obsoleting of flet #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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants