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

Deprecate XEmacs support and XEmacs toolbar #59

Merged
merged 1 commit into from
Dec 12, 2020
Merged

Deprecate XEmacs support and XEmacs toolbar #59

merged 1 commit into from
Dec 12, 2020

Conversation

mtreca
Copy link
Collaborator

@mtreca mtreca commented Dec 7, 2020

This is my first attempt at removing support for XEmacs in gnuplot-mode.

These changes are NOT tested, because I haven't yet found a simple way to locally test my changes (I am only getting started in Emacs packages development). However I did not use any functionality to the package, just removed all occurrences of XEmacs, making sure to replace any obsolete functions and variables caused by these changes. I used flycheck and package-lint on the resulting files.

The original commit message is below:

Remove all functions and checks relative to XEmacs, including the
gnuplot-mode toolbar and associated bitmaps.

Mark deprecated modules and functions for future fixes.

@mtreca mtreca self-assigned this Dec 7, 2020
@mtreca
Copy link
Collaborator Author

mtreca commented Dec 7, 2020

Paging @conao3 for review. @dkogan and @tarsius might be interested as well.

@conao3
Copy link
Collaborator

conao3 commented Dec 8, 2020

I think we need to prepare some regression tests first and then, remove something.

@conao3
Copy link
Collaborator

conao3 commented Dec 8, 2020

I fixed CI on #60 (but CI on Emacs-25.* is failed because some of Cask issue).

Please rebase HEAD? Currently, our test-file is very poor, but we can check gnuplot.el could be load and byte-compile.
(I can find some warnings in byte-compile, we need to fix this first?)

@mtreca mtreca mentioned this pull request Dec 12, 2020
@mtreca
Copy link
Collaborator Author

mtreca commented Dec 12, 2020

Rebased and ran tests. Everything is looking good except the deprecation warning for cl and flet, which I intend to fix in a separate commit.

Improving the test suite will be indeed necessary at some point.

@conao3 do the changes look OK for you?

@tarsius
Copy link

tarsius commented Dec 12, 2020

I think you should not bump the version number in this commit. It cannot hurt to have this only be used by master users for a few days and there might be other things to deal with before the release, such as:

Everything is looking good except the deprecation warning for cl and flet, which I intend to fix in a separate commit.

😜

@mtreca
Copy link
Collaborator Author

mtreca commented Dec 12, 2020

I think you should not bump the version number in this commit. It cannot hurt to have this only be used by master users for a few days and there might be other things to deal with before the release

Yes that makes much more sense indeed. My plan was to do a release-specific commit (and move the changelog found in gnuplot.el to a dedicated CHANGELOG file). But wouldn't have waited a few days for the release, which could have been bad since this commit is not trivial and could break things for users. Sorry, this is the first package I am maintaining 😬

I renamed the commit message accordingly.

@mtreca mtreca changed the title [WIP] Bump to 0.8.0. Deprecate XEmacs support and XEmacs toolbar Deprecate XEmacs support and XEmacs toolbar Dec 12, 2020
@conao3
Copy link
Collaborator

conao3 commented Dec 12, 2020

We may remove other xemacs mention?

(M-x rgrep xemacs result)

-*- mode: grep; default-directory: "~/dev/forks/gnuplot/" -*-
Grep started at Sat Dec 12 23:45:43

find . -type d \( -path \*/SCCS ... -prune -o  -type f \( -name \* -o -name .\[\!.\]\* -o -name ..\?\* \) -exec grep --color -i -nH --null -e xemacs \{\} +
./README.org\077:   It should also work on GNU Emacs 22 and XEmacs 21. It may work
./README.org\0135:      yourself. Version numbers in the 20's of Emacs and XEmacs ship
./INSTALL.org\021:      ~./configure EMACS=xemacs~ to use XEmacs.  On Mac OS X, if your
./Makefile.dst\03:## Set this variable to "xemacs" if you use XEmacs
./configure\02458: for ac_prog in emacs xemacs
./configure\02521:  # If $EMACS isn't GNU Emacs or XEmacs, this can blow up pretty badly
./aclocal.m4\0314: AC_CHECK_PROGS([EMACS], [emacs xemacs], [no])
./aclocal.m4\0327:  # If $EMACS isn't GNU Emacs or XEmacs, this can blow up pretty badly
./gnuplot.el\0191:;;  0.4b  Nov 21 1998 <BR> added toolbar for xemacs -- see file
./gnuplot.el\0233:;;        want it to work with font-lock under emacs and xemacs and
./gnuplot.el\0294:;;  0.8.0 Dec 07 2020 <MFT> Remove XEmacs support, including the toolbar
./gnuplot.el\0317:;;    Maxime F. Treca   <MFT> (package update, XEmacs deprecation)
./gnuplot.el\0423:version from 20.2.  Any version of XEmacs 20 or 21 should use the
./gnuplot.el\0424:version from 20.3 but can use either.  XEmacs 19 should use the
./gnuplot.el\01168:;; This is from the header in easymenu.el distributed with XEmacs:
./gnuplot.el\01180:;; ;; XEmacs will bind the map to button3 in each MAPS, but you must
./gnuplot.el\01217:;; There is no `mark-active' variable in XEmacs.  Hassle!  This is not
./gnuplot-context.el\02064:  (if (featurep 'xemacs)                ; Need an alist

Grep finished with 20 matches found at Sat Dec 12 23:45:43

@mtreca
Copy link
Collaborator Author

mtreca commented Dec 12, 2020

I am not going to change the documentation and Makefiles just yet, but I am going to remove what remains in gnuplot and gnuplot-context.el. Note that most of the matches in gnuplot.el are from the changelog at the beginning of the file, and those should be kept (I am going to create a CHANGELOG file though).
I will push a last version tonight and merge.

Remove all functions and checks relative to XEmacs, include the
gnuplot-mode toolbar and associated bitmaps.

Mark deprecated modules and functions for future fixes.
@mtreca mtreca merged commit 62117fe into emacs-gnuplot:master Dec 12, 2020
This was referenced Dec 12, 2020
Copy link
Collaborator

@conao3 conao3 left a comment

Choose a reason for hiding this comment

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

Looks good.

@conao3
Copy link
Collaborator

conao3 commented Dec 13, 2020

Sorry, ignore me.

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.

None yet

3 participants