literate-elisp icon indicating copy to clipboard operation
literate-elisp copied to clipboard

`literate-elisp-refs--read-all-buffer-forms` now takes SYMBOLS-WITH-POS argument

Open TinaRussell opened this issue 1 year ago • 1 comments

elisp-refs--read-all-buffer-forms now takes two arguments (BUFFER and SYMBOLS-WITH-POS) instead of one, so the around advice literate-elisp-refs--read-all-buffer-forms needs to take three arguments, now (the original function plus the two arguments it takes). This PR makes that change.

There are two problems: the first is that when writing the commit messages, I forgot that elisp-refs isn’t an internal Emacs package and assumed that the change to elisp-refs--read-all-buffer-forms happened with Emacs 28.3, so my commit message for the second commit is a bit misleading (it should say “make compatible with older versions of elisp-refs”). (If compatibility with older versions of elisp-refs isn’t important to you, you can omit that commit entirely; it uses some cl-defun magic to make it so the advice can be called with either set of arguments.)

The second problem is that the change to elisp-refs--read-all-buffer-forms was made to take advantage of the new function in Emacs 29, read-positioning-symbols. Now, if the new SYMBOLS-WITH-POS argument is non-nil, elisp-refs--read-all-buffer-forms will use read-positioning-symbols instead of read. Given that the purpose of the literate-elisp-refs--read-all-buffer-forms advice is to replace the read function with literate-elisp-read-internal temporarily when necessary, I’m assuming we’ll need a literate-elisp equivalent to read-positioning-symbols as well, and to make the literate-elisp-refs--read-all-buffer-forms advice aware of it. (I’m not on Emacs 29, yet, and I don’t know how read-positioning-symbols works, so this PR is just a quick fix so that the literate-elisp-refs--read-all-buffer-forms advice doesn’t return a “wrong number of arguments” error.)

TinaRussell avatar May 26 '23 23:05 TinaRussell