master 9422a673744: (set-auto-mode): Streamline to fix bug#67795

[Stefan Monnier]

branch: master
commit 9422a6737447b186ca017929da79985cef7898a8
Author: Stefan Monnier <monnier@iro.umontreal.ca>
Commit: Stefan Monnier <monnier@iro.umontreal.ca>

    (set-auto-mode): Streamline to fix bug#67795
    
    The old code tested if the mode function is `fboundp` but in an
    inconsistent way and without paying attention to
    `major-mode-remap`.
    
    * lisp/files.el (set-auto-mode-0): Return `:keep` rather than nil if
    the mode was already set.  And emit a warning when the mode function
    doesn't exist.
    (set-auto-mode): Remove checks that the mode function exists now that
    `set-auto-mode-0` does it for us.  Adjust to the new return values of
    that function, and simplify the code using a big `or` instead of
    a sequence of steps each setting&testing `done`.
    (hack-local-variables--find-variables): Use `major-mode-remap`
    when skipping the "mode:" entries that specify modes we don't have.
    Also, when (eq handle-mode t), don't bother building a list of results
    only to return a single element in the end.
---
 lisp/files.el | 231 ++++++++++++++++++++++++++++++----------------------------
 1 file changed, 118 insertions(+), 113 deletions(-)

diff --git a/lisp/files.el b/lisp/files.el
index 3ca4f047144..766ed573392 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -3425,7 +3425,7 @@ set the major mode only if that would change it.  In 
other words
 we don't actually set it to the same mode the buffer already has."
   ;; Look for -*-MODENAME-*- or -*- ... mode: MODENAME; ... -*-
   (let ((try-locals (not (inhibit-local-variables-p)))
-       end done mode modes)
+       end modes)
     ;; Once we drop the deprecated feature where mode: is also allowed to
     ;; specify minor-modes (ie, there can be more than one "mode:"), we can
     ;; remove this section and just let (hack-local-variables t) handle it.
@@ -3456,100 +3456,96 @@ we don't actually set it to the same mode the buffer 
already has."
             (push (intern (concat (downcase (buffer-substring (point) end))
                                   "-mode"))
                   modes))))
-    ;; If we found modes to use, invoke them now, outside the save-excursion.
-    (if modes
-       (catch 'nop
-         (dolist (mode (nreverse modes))
-           (if (not (functionp mode))
-               (message "Ignoring unknown mode `%s'" mode)
-             (setq done t)
-             (or (set-auto-mode-0 mode keep-mode-if-same)
-                 ;; continuing would call minor modes again, toggling them off
-                 (throw 'nop nil))))))
-    ;; Check for auto-mode-alist entry in dir-locals.
-    (unless done
-      (with-demoted-errors "Directory-local variables error: %s"
-       ;; Note this is a no-op if enable-local-variables is nil.
-        (let* ((mode-alist (cdr (hack-dir-local--get-variables
-                                 (lambda (key) (eq key 'auto-mode-alist))))))
-          (setq done (set-auto-mode--apply-alist mode-alist
-                                                 keep-mode-if-same t)))))
-    (and (not done)
-        (setq mode (hack-local-variables t (not try-locals)))
-        (not (memq mode modes))        ; already tried and failed
-        (if (not (functionp mode))
-            (message "Ignoring unknown mode `%s'" mode)
-          (setq done t)
-          (set-auto-mode-0 mode keep-mode-if-same)))
-    ;; If we didn't, look for an interpreter specified in the first line.
-    ;; As a special case, allow for things like "#!/bin/env perl", which
-    ;; finds the interpreter anywhere in $PATH.
-    (and (not done)
-        (setq mode (save-excursion
-                     (goto-char (point-min))
-                     (if (looking-at auto-mode-interpreter-regexp)
-                         (match-string 2))))
-        ;; Map interpreter name to a mode, signaling we're done at the
-        ;; same time.
-        (setq done (assoc-default
-                    (file-name-nondirectory mode)
-                    (mapcar (lambda (e)
-                               (cons
-                                (format "\\`%s\\'" (car e))
-                                (cdr e)))
-                            interpreter-mode-alist)
-                    #'string-match-p))
-        ;; If we found an interpreter mode to use, invoke it now.
-        (set-auto-mode-0 done keep-mode-if-same))
-    ;; Next try matching the buffer beginning against magic-mode-alist.
-    (unless done
-      (if (setq done (save-excursion
-                      (goto-char (point-min))
-                      (save-restriction
-                        (narrow-to-region (point-min)
-                                          (min (point-max)
-                                               (+ (point-min) 
magic-mode-regexp-match-limit)))
-                         (assoc-default
-                          nil magic-mode-alist
-                          (lambda (re _dummy)
-                            (cond
-                             ((functionp re)
-                              (funcall re))
-                             ((stringp re)
-                              (let ((case-fold-search nil))
-                                (looking-at re)))
-                             (t
-                              (error
-                               "Problem in magic-mode-alist with element %s"
-                               re))))))))
-         (set-auto-mode-0 done keep-mode-if-same)))
-    ;; Next compare the filename against the entries in auto-mode-alist.
-    (unless done
-      (setq done (set-auto-mode--apply-alist auto-mode-alist
-                                             keep-mode-if-same nil)))
-    ;; Next try matching the buffer beginning against 
magic-fallback-mode-alist.
-    (unless done
-      (if (setq done (save-excursion
-                      (goto-char (point-min))
-                      (save-restriction
-                        (narrow-to-region (point-min)
-                                          (min (point-max)
-                                               (+ (point-min) 
magic-mode-regexp-match-limit)))
-                        (assoc-default nil magic-fallback-mode-alist
-                                        (lambda (re _dummy)
-                                          (cond
-                                           ((functionp re)
-                                            (funcall re))
-                                           ((stringp re)
-                                            (let ((case-fold-search nil))
-                                              (looking-at re)))
-                                           (t
-                                            (error
-                                             "Problem with 
magic-fallback-mode-alist element: %s"
-                                             re))))))))
-         (set-auto-mode-0 done keep-mode-if-same)))
-    (unless done
-      (set-buffer-major-mode (current-buffer)))))
+    (or
+     ;; If we found modes to use, invoke them now, outside the save-excursion.
+     ;; Presume `modes' holds a major mode followed by minor modes.
+     (let ((done ()))
+       (dolist (mode (nreverse modes))
+        (if (eq done :keep)
+            ;; `keep-mode-if-same' is set and the (major) mode
+            ;; was already set.  Refrain from calling the following
+            ;; minor modes since they have already been set.
+            ;; It was especially important in the past when calling
+            ;; minor modes without an arg would toggle them, but it's
+             ;; still preferable to avoid re-enabling them,
+            nil
+          (let ((res (set-auto-mode-0 mode keep-mode-if-same)))
+            (setq done (or res done)))))
+       done)
+     ;; Check for auto-mode-alist entry in dir-locals.
+     (with-demoted-errors "Directory-local variables error: %s"
+       ;; Note this is a no-op if enable-local-variables is nil.
+       (let* ((mode-alist (cdr (hack-dir-local--get-variables
+                                (lambda (key) (eq key 'auto-mode-alist))))))
+         (set-auto-mode--apply-alist mode-alist keep-mode-if-same t)))
+     (let ((mode (hack-local-variables t (not try-locals))))
+       (unless (memq mode modes)       ; already tried and failed
+         (set-auto-mode-0 mode keep-mode-if-same)))
+     ;; If we didn't, look for an interpreter specified in the first line.
+     ;; As a special case, allow for things like "#!/bin/env perl", which
+     ;; finds the interpreter anywhere in $PATH.
+     (when-let
+        ((interp (save-excursion
+                   (goto-char (point-min))
+                   (if (looking-at auto-mode-interpreter-regexp)
+                       (match-string 2))))
+         ;; Map interpreter name to a mode, signaling we're done at the
+         ;; same time.
+         (mode (assoc-default
+                (file-name-nondirectory interp)
+                (mapcar (lambda (e)
+                           (cons
+                            (format "\\`%s\\'" (car e))
+                            (cdr e)))
+                        interpreter-mode-alist)
+                #'string-match-p)))
+       ;; If we found an interpreter mode to use, invoke it now.
+       (set-auto-mode-0 mode keep-mode-if-same))
+     ;; Next try matching the buffer beginning against magic-mode-alist.
+     (let ((mode (save-excursion
+                  (goto-char (point-min))
+                  (save-restriction
+                    (narrow-to-region (point-min)
+                                      (min (point-max)
+                                           (+ (point-min) 
magic-mode-regexp-match-limit)))
+                     (assoc-default
+                      nil magic-mode-alist
+                      (lambda (re _dummy)
+                        (cond
+                         ((functionp re)
+                          (funcall re))
+                         ((stringp re)
+                          (let ((case-fold-search nil))
+                            (looking-at re)))
+                         (t
+                          (error
+                           "Problem in magic-mode-alist with element %s"
+                           re)))))))))
+       (set-auto-mode-0 mode keep-mode-if-same))
+     ;; Next compare the filename against the entries in auto-mode-alist.
+     (set-auto-mode--apply-alist auto-mode-alist
+                                 keep-mode-if-same nil)
+     ;; Next try matching the buffer beginning against 
magic-fallback-mode-alist.
+     (let ((mode (save-excursion
+                  (goto-char (point-min))
+                  (save-restriction
+                    (narrow-to-region (point-min)
+                                      (min (point-max)
+                                           (+ (point-min) 
magic-mode-regexp-match-limit)))
+                    (assoc-default nil magic-fallback-mode-alist
+                                    (lambda (re _dummy)
+                                      (cond
+                                       ((functionp re)
+                                        (funcall re))
+                                       ((stringp re)
+                                        (let ((case-fold-search nil))
+                                          (looking-at re)))
+                                       (t
+                                        (error
+                                         "Problem with 
magic-fallback-mode-alist element: %s"
+                                         re)))))))))
+       (set-auto-mode-0 mode keep-mode-if-same))
+     (set-buffer-major-mode (current-buffer)))))
 
 (defvar-local set-auto-mode--last nil
   "Remember the mode we have set via `set-auto-mode-0'.")
@@ -3583,18 +3579,29 @@ and it is meant to be modified by packages rather than 
users.")
   "Apply MODE and return it.
 If optional arg KEEP-MODE-IF-SAME is non-nil, MODE is chased of
 any aliases and compared to current major mode.  If they are the
-same, do nothing and return nil."
-  (unless (and keep-mode-if-same
-              (or (eq (indirect-function mode)
-                      (indirect-function major-mode))
-                  (and set-auto-mode--last
-                       (eq mode (car set-auto-mode--last))
-                       (eq major-mode (cdr set-auto-mode--last)))))
-    (when mode
-      (funcall (major-mode-remap mode))
-      (unless (eq mode major-mode)
-        (setq set-auto-mode--last (cons mode major-mode)))
-      mode)))
+same, do nothing and return `:keep'.
+Return nil if MODE could not be applied."
+  (when mode
+    (if (and keep-mode-if-same
+            (or (eq (indirect-function mode)
+                    (indirect-function major-mode))
+                (and set-auto-mode--last
+                     (eq mode (car set-auto-mode--last))
+                     (eq major-mode (cdr set-auto-mode--last)))))
+       :keep
+      (let ((modefun (major-mode-remap mode)))
+        (if (not (functionp modefun))
+            (progn
+              (message "Ignoring unknown mode `%s'%s" mode
+                       (if (eq mode modefun) ""
+                         (format " (remapped to `%S')" modefun)))
+              nil)
+          (funcall modefun)
+          (unless (or (eq mode major-mode) ;`set-auto-mode--last' is overkill.
+                      ;; `modefun' is something like a minor mode.
+                      (local-variable-p 'set-auto-mode--last))
+            (setq set-auto-mode--last (cons mode major-mode)))
+          mode)))))
 
 (defvar file-auto-mode-skip "^\\(#!\\|'\\\\\"\\)"
   "Regexp of lines to skip when looking for file-local settings.
@@ -4201,8 +4208,9 @@ major-mode."
                           (not (string-match
                                 "-minor\\'"
                                 (setq val2 (downcase (symbol-name val)))))
-                           ;; Allow several mode: elements.
-                           (push (intern (concat val2 "-mode")) result))
+                          (let ((mode (intern (concat val2 "-mode"))))
+                             (when (fboundp (major-mode-remap mode))
+                               (setq result mode))))
                    (cond ((eq var 'coding))
                          ((eq var 'lexical-binding)
                           (unless hack-local-variables--warned-lexical
@@ -4233,10 +4241,7 @@ major-mode."
                                         val)
                                    result))))))
                (forward-line 1)))))))
-    (if (eq handle-mode t)
-        ;; Return the final mode: setting that's defined.
-        (car (seq-filter #'fboundp result))
-      result)))
+    result))
 
 (defun hack-local-variables-apply ()
   "Apply the elements of `file-local-variables-alist'.