search

felko / neuron-mode

An emacs mode for editing Zettelkasten notes with neuron
GNU General Public License v3.0
118 stars 21 forks source link

Replace an undefined function with a cognate #52

Closed prsteele closed 4 years ago

prsteele commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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; usemap-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

prsteele commented 4 years ago

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 commented 4 years ago

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