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

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

TinaRussell
Copy link

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 added 3 commits May 25, 2023 23:24
...so, the advice (literate-elisp-refs--read-all-buffer-forms) is updated accordingly.
(note that I haven't actually tested this in Emacs pre-28.3, but it
_should_ work, now)
@jingtaozf
Copy link
Owner

@TinaRussell Thanks for the pull request.
Can you also update related code in original org files?

For me, I usually edit the original org files, then use the code in section release current library to update the elisp file.

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.

2 participants