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

Patching cl-defun #39

Closed
manuel-uberti opened this issue Nov 14, 2019 · 9 comments
Closed

Patching cl-defun #39

manuel-uberti opened this issue Nov 14, 2019 · 9 comments
Labels

Comments

@manuel-uberti
Copy link

Hi,

first of all thanks for this package, it seems to be exactly the piece of the puzzle I need for a minor "problem" I have.

I'd like to patch this function:

(cl-defun psession--restore-objects-from-directory
    (&optional (dir psession-elisp-objects-default-directory))
  (let ((file-list (directory-files dir t directory-files-no-dot-files-regexp)))
    (cl-loop for file in file-list do (and file (load file)))))

My aim is to replace (load file) with (load file nil 'nomessage nil). This is what I did:

(el-patch-feature psession)
(with-eval-after-load 'psession
  (el-patch-defun psession--restore-objects-from-directory
    (&optional (dir psession-elisp-objects-default-directory))
    (let ((file-list (directory-files dir t directory-files-no-dot-files-regexp)))
      (cl-loop for file in file-list do
               (and file
                    (load file nil (el-patch-swap nil 'nomessage) nil))))))

In the :init part of my use-package psession snippet. However, upon restarting Emacs I get:

Eager macro-expansion failure: (error "Malformed arglist: (&optional (dir psession-elisp-objects-default-directory))")

What am I doing wrong? :)

@manuel-uberti
Copy link
Author

I fixed the problem with:

(el-patch-deftype cl-defun
  :classify el-patch-classify-function
  :locate el-patch-locate-function
  :declare ((doc-string 3)
            (indent defun)))

Sorry for the noise, just my bad not having read the documentation properly.

@raxod502
Copy link
Member

Seems like a useful thing to have by default. Added, thanks!

@manuel-uberti
Copy link
Author

Oh great, thanks to you!

@manuel-uberti
Copy link
Author

One question, though: is there any reason why it is not on master?

@raxod502
Copy link
Member

We merge to master after some time has passed, to ensure stability

@manuel-uberti
Copy link
Author

Gotcha. No problem, straight makes everything easier these days. 😉

@raxod502
Copy link
Member

I noticed you mentioned this on your blog. Did you perhaps forget to include the el-patch-swap form in your example el-patch-cl-defun code?

@manuel-uberti
Copy link
Author

You're right, thanks for the tip. Although it is still working as I am expecting. Am I using it wrong?

@manuel-uberti
Copy link
Author

Actually, by running el-patch-validate-all I was able to understand what I was doing wrong and now all my patches are valid. So again, thank you @raxod502. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

2 participants