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

Set add-log-current-defun-function #629

Merged

Conversation

deejayem
Copy link
Contributor

A number of emacs' programming modes1 set the variable add-log-current-defun-function to a function which returns the current defun/function/variable/etc name, e.g. in lisp/emacs-lisp/lisp-mode.el: (setq-local add-log-current-defun-function #'lisp-current-defun-name) (https://git.savannah.gnu.org/cgit/emacs.git/tree/lisp/emacs-lisp/lisp-mode.el#n683).

This variable is used by which-function-mode to get the name of the current function (otherwise it falls back to some logic based on imenu). It is also used by easy-kill, which then allows the copying of the current defun name (M-w D). The latter is the reason I wanted this, as it doesn't work without setting the variable. I haven't discovered anything else that uses it.

The function I have added (clojure-current-defun-name) is a copy of lisp-current-defun-name from lisp-mode.el (https://git.savannah.gnu.org/cgit/emacs.git/tree/lisp/emacs-lisp/lisp-mode.el#n730), with the small addition of the code to skip metadata (indicated the "Skip metadata" comment), and a tweak to the comments.

I have been using this for a few weeks without issue. It works with def, defn, defn-, defmacro and ns fine.

The convention seems to be to call the function foo-current-defun-name, even when the language doesn't use defun (e.g. perl-current-defun-name, sh-current-defun-name, html-current-defun-name), so I have stuck with that.

1 A quick search in lisp/ in the emacs source turned up these: autoconf, idlwave, cfengine, fortran, cperl-mode, scheme, m4-mode, make-mode, perl-mode, sh-script, tcl, cc-mode, python, octave, ruby-mode, f90, texinfo, sgml-mode, css-mode, tex-mode, diff-mode, org-mode.


Before submitting a PR mark the checkboxes for the items you've done (if you
think a checkbox does not apply, then leave it unchecked):

  • The commits are consistent with our contribution guidelines.
  • You've added tests (if possible) to cover your change(s). Bugfix, indentation, and font-lock tests are extremely important!
  • You've run M-x checkdoc and fixed any warnings in the code you've written.
  • You've updated the changelog (if adding/changing user-visible functionality).
  • You've updated the readme (if adding/changing user-visible functionality).

Thanks!

@deejayem deejayem marked this pull request as ready for review August 24, 2022 16:32
clojure-mode.el Outdated
@@ -524,6 +524,35 @@ replacement for `cljr-expand-let`."
#'clojure-space-for-delimiter-p)
(advice-add 'paredit-convolute-sexp :after #'clojure--replace-let-bindings-and-indent)))

(defun clojure-current-defun-name ()
"Return the name of the defun at point, or nil."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The indentation of the entire function seems off (it's 2 spaces too indented), so you might want to use indent-region on it.

I'd also suggest expanding the docstring to mention this function is meant to be used by which-func.

@bbatsov
Copy link
Member

bbatsov commented Aug 24, 2022

Hmm, somehow I've totally missed this. Your patch looks good, sans the indentation remark I've made inline.

@bbatsov bbatsov merged commit 905abd0 into clojure-emacs:master Aug 24, 2022
@bbatsov
Copy link
Member

bbatsov commented Aug 24, 2022

Thanks!

@deejayem deejayem deleted the set-add-log-current-defun-function branch December 1, 2022 06:11
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