Skip to content

Conversation

pdbrown
Copy link
Contributor

@pdbrown pdbrown commented Apr 21, 2021

Fixes #2934

  • The commits are consistent with our contribution guidelines
  • You've added tests (if possible) to cover your change(s)
  • All tests are passing (eldev test)
  • All code passes the linter (eldev lint) which is based on elisp-lint and includes
  • You've updated the changelog (if adding/changing user-visible functionality)
  • You've updated the user manual (if adding/changing user-visible functionality)

(with-current-buffer buffer
(cider-mode +1))))
(cider-mode +1)
(when global-eldoc-mode
Copy link
Member

Choose a reason for hiding this comment

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

I'm really puzzled by this part. In theory global-eldoc-mode should have trigged eldoc-mode, so it seems very weird to enable it manually if the global mode is enabled.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah this does look very weird, so I just pushed an update to the inline comment. I looked into globalized minor modes a bit, and from what I can tell, they call their e.g. turn-on-eldoc-mode function when the global mode is toggled on or when new file-visiting buffers are created while it's on, and turn-on-eldoc-mode only actually enables eldoc-mode if eldoc--supported-p returns true when turn-on-eldoc-mode is called. So old clojure buffers did call turn-on-eldoc-mode, but it didn't do anything because eldoc wasn't supported before cider was enabled. Now that cider is enabled, eldoc is supported, but global minor modes won't re-run any turn-on functions in reponse to just enabling another minor mode (cider-mode), so we have to do it ourselves, but only if the user hasn't disabled global-eldoc-mode.

@pdbrown pdbrown force-pushed the bugfix/global-eldoc-mode branch from 12e648b to 37ec0c6 Compare April 21, 2021 18:27
@bbatsov
Copy link
Member

bbatsov commented Apr 21, 2021

Okay, that makes sense. Thanks for looking into this!

@bbatsov bbatsov merged commit 78540f5 into clojure-emacs:master Apr 21, 2021
@pdbrown
Copy link
Contributor Author

pdbrown commented Apr 21, 2021

No problem!

@pdbrown pdbrown deleted the bugfix/global-eldoc-mode branch April 21, 2021 19:23
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.

eldoc-mode disabled, global-eldoc-mode ignored.

2 participants