-
Notifications
You must be signed in to change notification settings - Fork 137
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
Add: (-let) Bind vars to EIEIO object slots with &obj #251
base: master
Are you sure you want to change the base?
Conversation
The test fails on Emacs <25, but I can't figure out why. I ran (require 'eieio)
(defclass record () ; No superclasses
((name :initarg :name
:initform ""
:type string
:custom string
:documentation "The name of a person.")
(birthday :initarg :birthday
:initform "Jan 1, 1970"
:custom string
:type string
:documentation "The person's birthday.")
(phone :initarg :phone
:initform ""
:documentation "Phone number."))
"A single record for tracking people I know.")
(defmethod call-record ((rec record) &optional scriptname)
"Dial the phone for the record REC.
Execute the program SCRIPTNAME to dial the phone."
(message "Dialing the phone for %s" (oref rec name))
(shell-command (concat (or scriptname "dialphone.sh")
" "
(oref rec phone))))
(setq rec (record :name "Eric" :birthday "June" :phone "555-5555")) I evaluated each sexp, but when I got to the last one, I get the same kind of error:
This seems bizarre, since I'm using the example straight out of the manual, and it works in Emacs 25. I googled around but couldn't find anything, except a few similar reports of this error with Emacs 24. I didn't see any solutions presented, except in each case to upgrade the related third-party packages, so I don't know what is going on. |
It seems that I also noticed that (pcase-let (((eieio slot2 color slot1) (progn
(defclass -let-test-class ()
((slot1 :initarg :slot1)
(slot2 :initarg :slot2)
(color :initarg :color)))
(-let-test-class :slot1 1 :slot2 2 :color "red"))))
(list slot1 slot2 color)) ; => '(1 2 "red") It has the advantage that it doesn't require manually specifying the name of variables, just the slot names (but you can specify them if you choose). So I guess this PR isn't as useful as it would be without |
There's the simplified binding issue somewhere where you also wouldn't have to specify the slot names and they would derive automatically. If we implement that it will be incldued for all the matchers so the syntax will be basically the same. |
@Fuco1 That would probably be good, although the user would have to be careful in case a slot name shadowed a variable he had already bound. The tests are passing now on Emacs 24 and 25. The test on Emacs 23 fails with |
Yea, we can put some conditions around emacs 23 code, it's supported in backward-compatible way but new features don't have to work there. We need to make sure the features working only on newer emacsen are documented as such, so that people who really care about supporting e23 will avoid them. |
Ok. I'm not sure how to do that, so I'll let you handle it, or if you tell me how you want it done, I'll see what I can do. |
@alphapapa It should be enough to just wrap it in a condition. We have this in the examples.el
So something like that. Tweak the version as appropriate. |
And about the documentation, just put it in the docstring, something like "This only works with emacs xx.x or newer", should be enough. |
Ok, the test only runs on Emacs >23 now, and I added a note to the docstring about that. However, now when I run |
I just noticed that the test was already failing on Emacs 23. See https://travis-ci.org/magnars/dash.el/jobs/294156579. So I don't know if this commit works on Emacs 23, because the tests don't get that far. So, two problems:
|
The same is possible using |
@phst Thanks! I did not know about that. There are so many great things in Emacs, it's easy to be unaware of them for years. :) So I guess that further diminishes the usefulness of this PR. But at the same time, having it in |
I think this could be valuable if we could also destructure We could treat them both as objects and use a bit of runtime check to decide what to do. @alphapapa are you still interested in implementing this? |
Not at this time, sorry. I think it would be nice if |
This extends -let to bind to EIEIO object slots with the &obj symbol.