[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[debbugs-tracker] bug#32795: closed (26.1; provided-mode-derived-p does

From: GNU bug Tracking System
Subject: [debbugs-tracker] bug#32795: closed (26.1; provided-mode-derived-p does not support parent modes set with defalias)
Date: Sat, 29 Sep 2018 07:02:02 +0000

Your message dated Sat, 29 Sep 2018 10:01:01 +0300
with message-id <address@hidden>
and subject line Re: bug#32832: [PATCH] Fix 'provided-mode-derived-p' when the 
parent is an alias
has caused the debbugs.gnu.org bug report #32795,
regarding 26.1; provided-mode-derived-p does not support parent modes set with 
to be marked as done.

(If you believe you have received this mail in error, please contact

32795: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=32795
GNU Bug Tracking System
Contact address@hidden with problems
--- 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

    ;; 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))))))
 (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.



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 ---

reply via email to

[Prev in Thread] Current Thread [Next in Thread]