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

Ensure text property keys are symbols. #918

Merged
merged 1 commit into from
Dec 18, 2014

Conversation

llasram
Copy link
Contributor

@llasram llasram commented Dec 18, 2014

Maps received via nREPL have string keys. The Emacs Lisp text property functions expect all property keys to be symbols. Thus we must first intern any non-symbol property keys prior to applying them as text properties. Fixes #885.

@@ -74,7 +82,7 @@ positions before and after executing BODY."
(let ((start (cl-gensym)))
`(let ((,start (point)))
(prog1 (progn ,@body)
(add-text-properties ,start (point) ,props)))))
(add-text-properties ,start (point) (cider-intern-keys ,props))))))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we should fix the offending call instead of being defensive here. After all - the function was invoked with incorrect arguments.

@bbatsov
Copy link
Member

bbatsov commented Dec 18, 2014

Please, prefix the commit message with [Fix #885]. I prefer this over adding such information to the comment's body.

Maps received via nREPL have string keys.  The Emacs Lisp text property
functions expect all property keys to be symbols.  Thus we must first intern any
non-symbol property keys prior to applying them as text properties.
@llasram
Copy link
Contributor Author

llasram commented Dec 18, 2014

Good point. Made requested changes and force-pushed.

@bbatsov
Copy link
Member

bbatsov commented Dec 18, 2014

👍

bbatsov added a commit that referenced this pull request Dec 18, 2014
Ensure text property keys are symbols.
@bbatsov bbatsov merged commit f7f953e into clojure-emacs:master Dec 18, 2014
@llasram llasram deleted the intern-keys branch December 18, 2014 14:58
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.

Keybindings in Test Report Mode not working.
2 participants