--- Begin Message ---
|
Subject: |
26.1; provided-mode-derived-p does not support parent modes set with defalias |
|
Date: |
Fri, 21 Sep 2018 00:20:48 -0700 |
The actual bug was that I expected was for `TODO` to be highlighted in a
`CMakeLists.txt` file. Normally this is done by `global-hl-todo-mode`,
which is activated for all modes derived from `prog-mode`. However,
`cmake-mode` is derived from `cmake--parent-mode` which is set like
this:
;; For compatibility with Emacs < 24
(defalias 'cmake--parent-mode
(if (fboundp 'prog-mode) 'prog-mode 'fundamental-mode))
After researching, I found multiple packages that set the parent mode
like this, using `prog-mode` when it's available. So with Emacs 26.1,
`cmake-mode` definitely derives from `prog-mode`, but the function that
`hl-todo-mode` uses to check this, `derived-mode-p`, does not handle
`defalias` correctly. The `hl-todo-mode` logic is:
(defcustom hl-todo-activate-in-modes '(prog-mode) ...)
(defun hl-todo--turn-on-mode-if-desired ()
(when (apply #'derived-mode-p hl-todo-activate-in-modes)
(hl-todo-mode 1)))
If this was working as expected, it would see `cmake-mode` as derived
from `prog-mode`, and therefore highlight the `TODO`, but it does not.
I was able to make a minimal repro as follows:
;; Broken behavior (seen in cmake-mode.el and groovy-mode.el):
(defalias 'alias-parent-mode
(if (fboundp 'prog-mode) 'prog-mode 'fundamental-mode))
(define-derived-mode alias-mode alias-parent-mode "Test")
;; Should print `prog-mode', but prints `alias-parent-mode'
(message "Aliased derived: %s"
(provided-mode-derived-p 'alias-mode 'prog-mode))
;; Existing working behavior:
(define-derived-mode test-mode prog-mode "Test")
;; Correctly prints `prog-mode'
(message "Directly derived: %s"
(provided-mode-derived-p 'test-mode 'prog-mode))
After looking at `derived-mode-p`, I followed it to
`provided-mode-derived-p`, and I would like to propose the following
patch to it (with an appropriate added commit message):
---
lisp/subr.el | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/lisp/subr.el b/lisp/subr.el
index 9e880bc88..a35734812 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -1922,7 +1922,11 @@ Only affects hooks run in the current buffer."
Uses the `derived-mode-parent' property of the symbol to trace backwards.
If you just want to check `major-mode', use `derived-mode-p'."
(while (and (not (memq mode modes))
- (setq mode (get mode 'derived-mode-parent))))
+ (let ((parent (get mode 'derived-mode-parent)))
+ (let ((parentfn (symbol-function parent)))
+ (setq mode (if (and parentfn (symbolp parentfn))
+ parentfn
+ parent))))))
mode)
(defun derived-mode-p (&rest modes)
--
This is kind of similar to the logic used by
https://github.com/Fanael/parent-mode/blob/master/parent-mode.el, except
(a) I wrote it originally, and (b) their implementation is recursive,
while I've kept `provided-mode-derived-p` iterative.
After testing, this fixes my original bug, and has not appeared to cause
unexpected behavior. It possibly fixes other bugs, anything that
expected to accurately deduce the parent of a derived mode.
Thanks,
Andy
P.S. Thanks molloy on #emacs for your help when looking at this.
Emacs Version(s):
In GNU Emacs 26.1 (build 1, x86_64-apple-darwin14.5.0, NS appkit-1348.17
Version 10.10.5 (Build 14F2511))
of 2018-05-30 built on builder10-10.porkrind.org
Also exhibited on Emacs 26.1 on Linux
--- End Message ---
--- Begin Message ---
|
Subject: |
Re: bug#32832: [PATCH] Fix 'provided-mode-derived-p' when the parent is an alias |
|
Date: |
Sat, 29 Sep 2018 10:01:01 +0300 |
> From: address@hidden
> Date: Mon, 24 Sep 2018 21:09:39 -0700
>
> From: Andrew Schwartzmeyer <address@hidden>
>
> Authors of Emacs packages often derive from an alias of a mode instead
> of the mode directly. This is especially the case when deriving from
> 'prog-mode' as it is relatively new. Unfortunately, using
> 'derived-mode-p' to check if some mode is derived from an alias of
> 'prog-mode' does not work as expected. The bug is that
> 'provided-mode-derived-p' should not only return non-nil when MODE is
> one of MODES, but also when MODE is an alias of one of MODES.
> * lisp/subr.el (provided-mode-derived-p):
> Return non-nil when MODE is an alias of any of MODES (Bug#32795).
> * test/lisp/subr-tests.el: Add tests for the above.
Thanks, pushed.
In the future please either format your patches with "git
format-patch" or make sure the first line of the commit log message is
a single sentence followed by an empty line (this is explained in
CONTRIBUTE), to make the patch application easier.
Also, you don't seem to have a copyright assignment on file, and this
contribution comes close to exhausting the amount of changes we can
accept without such an assignment. So I urge you to start your legal
paperwork rolling, to allow us to accept more contributions from you.
If you are interested, I will send you the assignment form off-list.
--- End Message ---