GNU bug report logs - #32795
26.1; provided-mode-derived-p does not support parent modes set with defalias

Previous Next

Full log

View this message in rfc822 format

From: help-debbugs <at> gnu.org (GNU bug Tracking System)
To: Andrew Schwartzmeyer <andrew <at> schwartzmeyer.com>
Subject: bug#32795: closed (Re: bug#32832: [PATCH] Fix 'provided-mode-derived-p'
 when the parent is an alias)
Date: Sat, 29 Sep 2018 07:02:02 +0000

[Message part 1 (text/plain, inline)]

Your bug report

#32795: 26.1; provided-mode-derived-p does not support parent modes set with defalias

which was filed against the emacs package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to 32795 <at> debbugs.gnu.org.

-- 
32795: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=32795
GNU Bug Tracking System
Contact help-debbugs <at> gnu.org with problems

[Message part 2 (message/rfc822, inline)]

From: Eli Zaretskii <eliz <at> gnu.org>
To: andrew <at> schwartzmeyer.com
Cc: 32795-done <at> debbugs.gnu.org
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: andrew <at> schwartzmeyer.com
> Date: Mon, 24 Sep 2018 21:09:39 -0700
> 
> From: Andrew Schwartzmeyer <andrew <at> schwartzmeyer.com>
> 
> 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.

[Message part 3 (message/rfc822, inline)]

From: Andrew Schwartzmeyer <andrew <at> schwartzmeyer.com>
To: bug-gnu-emacs <at> gnu.org
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

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.