use-package icon indicating copy to clipboard operation
use-package copied to clipboard

Auto detect mode suffix in hook keyword

Open CeleritasCelery opened this issue 6 years ago • 7 comments

Fixes #691 and fixes #672. The default way that use-package handles the :hook keyword when the function name is not explicitly given is to assume that it matches package name. For example

(use-package ace-jump-mode
  :hook prog-mode)

will add ace-jump-mode to prog-mode-hook.

However when the package name does not match the minor mode name, an in this example:

(use-package company
  :hook prog-mode)

use package will add company to prog-mode-hook, even though company is not a valid mode (it is called company-mode).

This convention of having the package name not be the same as the minor mode name is very common. My PR will auto detect the -mode suffix and append it if it is missing. This will handle both of the above cases correctly. If a package defines a function matching the package name that does not end in -mode, but is still valid to be added to a hook, then it can be added explicitly

(use-package foo
  :hook (prog-mode . foo))

However the last case is far less common then the first two. Therefore I propose that the third case should require a more verbose form instead of the second case.

In summary, we have three scenarios

  1. hook function matches package name and ends in -mode
  2. hook function is equivalent to <package name>-mode
  3. hook function matches package name and does not end in -mode

Currently use-package handles the the first and third cases with the brief form (:hook hook-base-name) and requires a verbose from for the second (:hook (hook-base-name . hook-function)). I am proposing that we change it so that use-package handles the the first and second cases with the brief form and requires a verbose form for the third. I believe this will cover more use-cases with a simpler implementation and lead to less confusion, since the third case is rare, and the second case is common. see the issues mentioned at the start for more info.

I was going to update the documentation, but nothing in the documentation is invalid after this PR because it only provides an example of the first scenario, which is handled the same before and after this PR. however I can add some additional documentation if that would be helpful.

CeleritasCelery avatar Feb 08 '19 17:02 CeleritasCelery

In your description, did you mean the -hook suffix in most places?

jwiegley avatar Jul 16 '19 01:07 jwiegley

No, I did not mean the -hook suffix. The hook suffix is already handled automatically. I mean the -mode suffix in the :hook keyword.

For example this is how use-package will expand now:

(use-package company :hook prog-mode) -> (add-hook 'prog-mode-hook 'company)

This is how I propose that it should expand:

(use-package company :hook prog-mode) -> (add-hook 'prog-mode-hook 'company-mode)

Notice how it automatically adds the -mode suffix. Let me know if something is not clear.

CeleritasCelery avatar Jul 16 '19 15:07 CeleritasCelery

I see. I actually don't want it adding a -mode suffix, then it's being doubly magical.

jwiegley avatar Jul 16 '19 18:07 jwiegley

Would you consider making this an option then? It seems clear to me from the discussion in the issues referenced that many people find the current behavior confusing and unintuitive.

CeleritasCelery avatar Jul 16 '19 22:07 CeleritasCelery

Just so I have this straight:

If the user specifies only (use-package foo :hook bar), and there is a variable named foo-hook, and there is a function named bar-mode, then and only then will be doubly-magical. Otherwise, it falls back on either a variable named bar and a function named foo.

If that's the case, then it seems harmless enough, but the expanded code will need to perform this check on load each time.

jwiegley avatar Jul 16 '19 22:07 jwiegley

If the user specifies only (use-package foo :hook bar), and there is a variable named foo-hook, and there is a function named bar-mode, then and only then will be doubly-magical. Otherwise, it falls back on either a variable named bar and a function named foo.

use-package currently does no checking to see if the hook variable exists, so that will not change. My proposal does not check if bar-mode exists either, it just assumes that it does (unless the user explicitly states otherwise). The argument for that is it is very rare for the hook function to exactly match the package name AND not end in -mode (I actually can't think of single example, but that is what use-package currently assumes). However it is very common for the "hook function" to be the package name with -mode appended (company, evil, outshine, anzu, lispy, to name a few). This change will handle the vast majority of packages better because it will follow that convention. The general package even added a new use-package keyword just to address this issue.

CeleritasCelery avatar Jul 16 '19 23:07 CeleritasCelery

@jwiegley any chance of this getting merged?

to recap with a TLDR, this PR lets us change this

(use-package company
  :hook (python-mode . company-mode))
(use-package lispy
  :hook (emacs-lisp-mode . lispy-mode))
(use-package outshine
  :hook (python-mode . outshine-mode))
(use-package anzu
  :hook (python-mode . anzu-mode))

to this

(use-package company
  :hook python-mode)
(use-package lispy
  :hook emacs-lisp-mode)
(use-package outshine
  :hook python-mode)
(use-package anzu
  :hook python-mode)

Note that the old versions still work, but you are not required to be as verbose. As a bonus this closes a couple of issues as well.

CeleritasCelery avatar May 18 '21 00:05 CeleritasCelery