vermiculus / magithub

**DEPRECATED - please use Forge instead!** -- Magit-based interfaces to GitHub
GNU General Public License v3.0
579 stars 62 forks source link

Adapt to change in how magit-buffer-lock-functions is called #351

Closed tarsius closed 5 years ago

tarsius commented 5 years ago

See https://github.com/magit/magit/commit/bbee428073e8972ea62b.

vermiculus commented 5 years ago

Sorry, I didn't even see your comment on the referenced commit. Thanks for making this PR.

What exactly does this do? I know very little about cl-defun except that it supports keyword arguments.

vermiculus commented 5 years ago

Also, this is introducing a compile warning: variable ‘_args’ not left unused

tarsius commented 5 years ago

Argument lists support destructuring. In Common Lisp, destructuring is only allowed with defmacro; this package allows it with cl-defun and other argument lists as well. In destructuring, any argument variable (var in the above example) can be replaced by a list of variables, or more generally, a recursive argument list. The corresponding argument value must be a list whose elements match this recursive argument list.

The warning is because the cl-defun macro expands to a defun (or not, didn't check) that uses all the variables, in order to actually bind them, probably using something like let. So they "are being used" after macro expansion. I saw something similar happen with when-let and adopted the convention to use e.g. -unused instead of _unused in such cases.

But you don't really need to use cl-defun, you can also use regular defun with a single argument ARGS and then use car on that yourself.

vermiculus commented 5 years ago

Ah, so this cl-defun form is a destructuring bind?

tarsius commented 5 years ago

Yep.

vermiculus commented 5 years ago

Assuming the CI build above checks out, I'm ready to go when you are.

tarsius commented 5 years ago

I have already pushed the change to Magit, so merging this isn't really optional.

vermiculus commented 5 years ago

Done. Thanks!

vermiculus commented 5 years ago

For posterity, the relevant commit is https://github.com/magit/magit/commit/48d0431610bf47fb1e1497fabde7b5ff2c7caa71