-
Notifications
You must be signed in to change notification settings - Fork 11
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
Remove warnings #409
Remove warnings #409
Conversation
@@ -1595,11 +1684,10 @@ Return number of matching entries found." | |||
(forward-line) | |||
(setq hdr-pos (cons (point-min) (point)))) | |||
(let ((case-fold-search t) | |||
match-start |
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.
match-start is not used and can thus be removed.
This reverts commit 95f3ba0. This change is from Emacs 30.1 so lets wait with this. The call is still backwards compatible and will only give a warning in Emacs 30.1.
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.
derived-mode-p as of Emacs 28.2 uses &rest to create a single list from all args given, so sending it a list as an arg may break it and is not necessary. Did the calling signature change at some point?
Remove that change and the rest looks good.
Thanks for approving.
It is changed in Emacs 30.1 to only take either one symbol or a list of symbols. So that hit me as a warning since I develop using the latest and greatest! But the test cases in CI made me aware that this change is not backwards compatible so I ha to revert that part of the PR, see commit 1f4381d. Hence point 2 is over struck although I understand it is not clear that the following comment is an explanation on why we keep the old code with using multiple symbols. The reason is that the change in 30.1 is backwards compatible so calling it with multiple symbols still work. I guess it will be removed at some point in the future. So for now we will have to live with the warning in Emacs 30.1. Maybe later it is time to deal with this. |
Yes, will be interesting to see why they made this change.-- BobOn Nov 26, 2023, at 3:20 PM, Mats Lidell ***@***.***> wrote:
Thanks for approving.
derived-mode-p as of Emacs 28.2 uses &rest to create a single list from all args given, so sending it a list as an arg may break it and is not necessary. Did the calling signature change at some point?
It is changed in Emacs 30.1 to only take either one symbol or a list of symbols. So that hit me as a warning since I develop using the latest and greatest!
But the test cases in CI made me aware that this change is not backwards compatible so I ha to revert that part of the PR, see commit 1f4381d. Hence point 2 is over struck although I understand it is not clear that the following comment is an explanation on why we keep the old code with using multiple symbols. The reason is that the change in 30.1 is backwards compatible so calling it with multiple symbols still work. I guess it will be removed at some point in the future. So for now we will have to live with the warning in Emacs 30.1. Maybe later it is time to deal with this.
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because your review was requested.Message ID: ***@***.***>
|
In the bug report there is some rational. https://lists.gnu.org/archive/html/bug-gnu-emacs/2023-11/msg00497.html |
What
Remove warnings
Why
Recent updates have introduced a set of new warnings. This tries to remove the warnings by:
Fix call to derived-mode-p that only takes one arg, a list, and not multiple args.Reverted. This change is introduced in Emacs 30.1 so will not affect older versions and it is backwards compatible so we can ignore this for now.