Fix #459: 'find-tag-marker-ring' is an obsolete variable (as of 25.1)

[maximbaz]

I have tried @vonavi's patch, proposed in #459, and it does seem to fix the described problem.

For your convenience I'm converting the patch into PR, with a hope that it can be reviewed and merged faster.

[gracjan]

@z0rch: In case you missed the build failure:

In toplevel form:
ghci-script-mode.el:20:1:Error: Cannot open load file: no such file or directory, xref
make: *** [ghci-script-mode.elc] Error 1

[maximbaz]

I'll look into it later this week.

[maximbaz]

@gracjan, I can see that xref.el was introduced on 25 Dec 2014, so this change applies only for the newest Emacs versions.

Since you support Emacs back to v23, you clearly don't want to merge this patch right away.

Do you have any examples in mind, how are you dealing with such situations? I'm sure it is not the first breaking change in Emacs that you see. Then I could improve this patch according to the pattern you use for this project.

[gracjan]

@z0rch: we are starting to take backward compatibility seriously. As of now there is haskell-compat.el that is a general bin for compatibility. We also ship our version of cl-lib (used for tests only for now when no other cl-lib is available).

xref stuff seems to be super-new in Emacs. Isn't there a way to accomplish same funcitonality using somewhat more stable method? Does xref provide a migration guide?

[maximbaz]

xref stuff seems to be super-new in Emacs. Isn't there a way to accomplish same functionality using somewhat more stable method? Does xref provide a migration guide?

@gracjan, unfortunately my knowledge of elisp is pretty limited, I can't tell if a third "better" option exist.

However based on what you described, I believe the right approach is to use new functions, but provide aliases for them in haskell-compat.el that replicate the old behavior. This will ensure that newer Emacs versions use the most recent API directly and don't require any wrappers, and at the same time backwards compatibility is preserved for older Emacs versions.

Especially since in this particular case, only two aliases are needed: xref-pop-marker-stack and xref-push-marker-stack.

Would you agree?

---

Now to a more technical level, I'm having issues with aliasing the latter function. Could you please have a look at haskell-compat.el file?

If I just put the call to ring-insert function, I get a Travis error: reference to free variable find-tag-marker-ring. This is the first bit that I don't understand, i.e. why it happens and why it was not an issue before (it was used in both haskell-commands.el and inf-haskell.el).
I added the dummy defvar as suggested here, but still looks suspicious.

The second problem is that I get another Travis error: the function ring-insert is not known to be defined. Moreover, (require 'ring) just before the usage doesn't seem to help.

I'm sure these are newbie questions, but I would appreciate if you could give me a quick hint on those so that I could complete this pull request as appropriate.

[gracjan]

@z0rch: Your thinking is correct. haskell-compat.el should be the place to put backward compatibility shims. The issues should go away when you find which require statements should go on top of haskell-compat.el file.

Remember to require haskell-compat too.

[gracjan]

@z0rch: quesiton: how do you test Emacs25? I cannot find a prebuild version that could be put in .travis.yml file. Is there anything like this available?

[maximbaz]

Then I'll look more and try to find out which require I am missing, thanks.

I use the following ppa to get pre-built Emacs 25, have a look if that would suit you.

[gracjan]

@z0rch: Hmm, we seem to be using emacs-snapshot, like this:

if [ "$EMACS" = "emacs-snapshot" ]; then
    sudo add-apt-repository -y ppa:cassou/emacs &&
    sudo apt-get update -qq &&
    sudo apt-get install -qq emacs-snapshot &&
    sudo apt-get install -qq emacs-snapshot-el emacs-snapshot-gtk;
fi

I'm not sure which version of emacs we are getting here.

[gracjan]

I see we are using old ppa for emacs-snapshot and this sources GNU Emacs 24.3.50.1, so not exactly the newest snapshot.

[maximbaz]

move (require 'ring) to the top, use usual magic that disables errors.

That was the thing, with this change all errors are gone. Thanks, @gracjan!

There is no need to ignore (require 'ring) error, since this file exists in all Emacs versions. I have addressed all the others comments though.

Travis is hanging today, but I installed Emacs 24 locally and ran the make EMACS=emacs24 check, it passed. Looking forward to see the Travis results, in order to eventually get this and #477 merged 😄

[gracjan]

I took the liberty of cleaning up the history a little (preserving your name) and using your work in #477.

[gracjan]

@purcell: this pr is subsumed by #477 and can be closed.