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

Replace an undefined function with a cognate #52

Merged
merged 2 commits into from
Aug 3, 2020

Conversation

prsteele
Copy link
Contributor

@prsteele prsteele commented Aug 3, 2020

The map-put! function is not defined in Emacs 26.3 with recent
copies of f, s, dash, and markdown-mode installed. However,
map-put is, and seems to maintain the intended effects. Namely, with
this modification calling the function neuron-toggle-connection-type
on a link toggles whether ?cf is included at the end of the link.

The `map-put!` function is not defined in Emacs 26.3 with recent
copies of `f`, `s`, `dash`, and `markdown-mode` installed. However,
`map-put` is, and seems to maintain the intended effects. Namely, with
this modification calling the function `neuron-toggle-connection-type`
on a link toggles whether `?cf` is included at the end of the link.
@prsteele
Copy link
Contributor Author

prsteele commented Aug 3, 2020

Although now that I search more, it seems like map-put might be the old form, and map-put! is the future --- see http://git.savannah.gnu.org/cgit/emacs.git/tree/lisp/emacs-lisp/map.el#n142 .

In fact, the first tag I can find with map-put! is emacs-27.0.90 (http://git.savannah.gnu.org/cgit/emacs.git/tree/lisp/emacs-lisp/map.el?h=emacs-27.0.90).

@felko
Copy link
Owner

felko commented Aug 3, 2020

Indeed, some other issue (#29) made me switch to map-2.1. I was taking care of using backward compatible functions so far but then I forgot to check if map-put! was also compatible. As many emacs users (including me) are still on 26.3, I'd like to find an equivalent of map-put! that works in both versions. Fortunately, the map-2.1 documentation comes with an alternative to map-put!:

This macro is obsolete since 27.1; use map-put! or (setf (map-elt ...) ...) instead

I'll merge your PR if you use the setf thing and that it works with both versions of map.el.

Thanks for reporting

@felko felko added the emacs-compat The code is imcompatible with another Emacs version label Aug 3, 2020
@prsteele
Copy link
Contributor Author

prsteele commented Aug 3, 2020

I've updated the PR to use the literal macro expansion of map-put!, which appears to be the same macro expansion of map-put. I'm guessing that map-put and map-put! have identical semantics, but they are trying to be more clear that it is a non-pure function by including the !.

Thanks for taking the time to review this, and for the Emacs integration itself, it's working very nicely so far!

@felko
Copy link
Owner

felko commented Aug 3, 2020

That works for me, I'll merge it. Thanks again

@felko felko merged commit ea260a0 into felko:master Aug 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
emacs-compat The code is imcompatible with another Emacs version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants