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 warning about bug in some emacs versions #17
base: master
Are you sure you want to change the base?
Conversation
Could you also add the warning to README.md? |
@@ -135,10 +135,18 @@ | |||
;; editline instead of readline. Consider recompiling bash, python, | |||
;; etc with readline support instead. | |||
|
|||
;; *** Note: emacs contained a bug from ~24.3.91 to ~24.5-rc3-fixed where |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your bug was reported for 25.0.50, so the bit about 24.5-rc3-fixed seems off to me? Or do you mean that is when the bug was introduced?
I think I was a bit tired, didn't fully think this through. I've updated it to use patch version, and got the right version. |
... now. |
Addressed your comments, except, normalizing patch-ver to 0 won't work. It would include 0 when I test <= and exclude when I test >=, but I'd like to include it on both conditions. I couldn't think of a better way to do it, without making things overly complicated / verbose. |
I think you are mistaken: (if patch-ver (>= 91 patch-ver) t) -->
(if nil (>= 91 nil) t) -->
t
(>= 91 patch-ver) -->
(>= 91 0) -->
t and (if patch-ver (<= patch-ver 50) t) -->
(if nil (<= nil 50) t) -->
t
(<= patch-ver 50) -->
(<= 0 50) -->
t |
Heh, we were both right.
should be
However, I think you were right to say it's not worth it. I thought, I bet someone else did this already, and they did: http://www.emacswiki.org/emacs/versions.el, and if it's not worth including a library (which I don't think it is), it's not worth having this code, because there are downsides of potential bugs, added complexity for new contributors, etc. I've updated the pull request to be just the comment. |
Fair enough. Just add it to the README as well per my first comment and I will merge :) |
you alive @ian-kelling? i'll amend your commit myself if you don't do it by next time i check in. |
#11 is fixed upstream. Add warning for affected versions and call it fixed.