-
Notifications
You must be signed in to change notification settings - Fork 10
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 xemacs compatibility code fragments #187
Conversation
I think we should delete only XEmacs clauses but leave InfoDock ones, as things like InfoDock menus will probably be used in the future. If this is rather complicated, we can table this for a future release. Thanks, Bob |
Are you referring to code selected by |
589b926
to
61ea7de
Compare
Hi @rswgnu, looked over the old PRs and found this. Can you take a look again? It is rebased and ChangeLog updated to reflect that changes looks like they are done now. I think it should be OK. Passes all tests but I'm afraid might not touch on many test cases ... From a quick look it seems like the Infodock menues are not touched in this PR so it is preserving those for potential future use. |
Sounds like good things to discuss at our next meeting.
…-- Bob
On Apr 19, 2022, at 5:06 PM, Mats Lidell ***@***.***> wrote:
delete only XEmacs clauses but leave InfoDock ones
Are you referring to code selected by (featurep 'infodock) or in general where InfoDock is mentioned? Keeping the menus I can understand. They stand out as different implementation. I would prefer using source control for keeping that but it is more at the level of a nit for me so I will not complain to much if we keep it on the master branch. 😄
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because your review was requested.
|
posn-area is was introduce at or before version 22.1.
use-region-p was introduced at or before version 23.1.
61ea7de
to
51fbe54
Compare
hyrolo-menu.el
Outdated
@@ -108,4 +108,3 @@ | |||
(provide 'hyrolo-menu) | |||
|
|||
;;; hyrolo-menu.el ends here | |||
oo |
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.
I don't see these garbage characters in the latest tip, FYI.
What
Remove
XEmacs
compatibility code fragmentsWhy
We don't support
XEmacs
so the compatibility code that is left gets in the way (clutters the code) and some times even causes warnings which we want to reduce to a minimum.Note
I took one step further here and since
XEmacs
in many cases is similar toInfoDock
I removed that too. I know it is not 100% true and I also know you might want to bringInfoDock
back to life some day. But we have version control to support that and I guess a futureInfoDock
would be based on a modernGNU Emacs
so most of the old code would not be useful. If you do not agree please advice how I should deal with the parts whereInfoDock
is mentioned. (InfoDock
can still be mentioned here and there since I have gone over the code looking forXEmacs
references.)The PR can probably best be reviewed a commit at a time or a file at the time. Should be mostly the same except for the
ChangeLog
entries.All tests passes but I do not think our test covers most of the parts that has been touched so review with that in mind.
I have not touched
mouse-sh.el
since that looks really old and scary. Do you have some insights into what of that is really used and worth keeping.There is also a part in
kotl-mode.el
where the naming of two functions are explained since there was a byte compiler error for those functions inXEmacs
. I kept that as a reminder that these functions now could maybe be given their proper names instead. But that will be a longer PR so leaving that for later. On the other hand why not just keep the names and forget about the reason for them being a bit off!?