Subject: Re: bug#68246: 30.0.50; Add non-TS mode as extra parent of TS modes

From: Stefan Monnier <[email-masked]>
Date: Wed, 17 Jan 2024 12:08:02 -0500

> @@ -3150,6 +3150,9 @@ auto-mode-alist
>  Visiting a file whose name matches REGEXP specifies FUNCTION as the
>  mode function to use.  FUNCTION will be called, unless it is nil.
>  
> +FUNCTION can also be a keyword denoting a language, to be looked
> +up in `major-mode-remap-alist'.

Side note: the intention is OK but `major-mode-remap-alist' is
a defcustom and should remain nil by default.  It's there for the users
to express which major modes they prefer.  So if we want a mapping
between some new language/type concept and major modes, it should be
stored elsewhere (could be a plain alist that's handled as a kind of
"implicit tail of `major-mode-remap-alist`").

> @@ -3206,10 +3209,10 @@ interpreter-mode-alist
>       ("emacs" . emacs-lisp-mode)))
>    "Alist mapping interpreter names to major modes.
>  This is used for files whose first lines match `auto-mode-interpreter-regexp'.
> -Each element looks like (REGEXP . MODE).
> +Each element looks like (REGEXP . MODE-OR-LANGUAGE).
>  If REGEXP matches the entire name (minus any directory part) of
>  the interpreter specified in the first line of a script, enable
> -major mode MODE.
> +MODE-OR-LANGUAGE.

There's a similar need for "content type" rather than "language".  If we
want to mention "language" we should also take the opportunity to
mention other related categorizations like "content type".

> -      (funcall (alist-get mode major-mode-remap-alist mode))
> +      ;; XXX: When there's no mapping for `:<language>', we could also
> +      ;; look for a function called `<language>-mode'.
> +      (funcall (alist-get mode major-mode-remap-alist (if (keywordp mode)
> +                                                          #'fundamental-mode
> +                                                        mode)))
> +      (when (keywordp mode)             ;Perhaps do that unconditionally.
> +        (run-hooks (intern (format "%s-language-hook" (buffer-language)))))

That seems wrong:
- Why should this hook run when `auto-mode-alist` says `:js` but not
  when doing `M-x javascript-mode` (or other ways to enable this mode)?
- Why run this hook *after* the mode's `:after-hook` and after
  things like `after-change-major-mode-hook`?

I think it should remain the major mode's responsibility to decide which
hooks it runs.

> +(defun buffer-language ()
> +  "Return the language of the current buffer.
> +A language is a lowercase keyword with the name of the language."
> +  ;; Alternatively, we could go through all the matchers in
> +  ;; auto-mode-alist, interpreter-mode-alist,
> +  ;; magic-fallback-mode-alist here, possibly using a cache keyed on
> +  ;; buffer-file-name.  But that's probably an overkill: if the user
> +  ;; changes the settings, they can call `M-x revert-buffer' at the end.
> +  (if (keywordp (car set-auto-mode--last))
> +      (car set-auto-mode--last)
> +    ;; Backward compatibility.
> +    (intern (format ":%s" (replace-regexp-in-string "\\(?:-ts\\)?-mode\\'" ""
> +                                                    (symbol-name major-mode))))))

I'm not comfortable enshrining the "-ts-mode" convention here.

Also I think if we want a `buffer-language` function, it should not rely
on how the mode was installed (e.g. `set-auto-mode--last`) but only on
the major mode itself, i.e. something like

    (defun buffer-language ()
      (or buffer-language
          (some heuristic based on major-mode and/or derived-modes)))

[ Of course, I already mentioned that I also suspect that there can/will
  be sometimes several languages (or none).  ]

> +(defun set-buffer-language (language)
> +  "Set the language of the current buffer.
> +And switch the major mode appropriately."
> +  (interactive
> +   (list (let* ((ct (mapcan
> +                     (lambda (pair) (and (keywordp (car pair))
> +                                    (list (symbol-name (car pair)))))
> +                     major-mode-remap-alist))
> +                (lang (completing-read "Language: " ct)))
> +           (and lang (intern lang)))))
> +  (set-auto-mode-0 language))

I see several issues with this function (name and implementation), but
I wonder when we'd ever need such a thing.

>  ;;;###autoload
>  (dolist (name (list "node" "nodejs" "gjs" "rhino"))
> -  (add-to-list 'interpreter-mode-alist (cons (purecopy name) 'js-mode)))
> +  (add-to-list 'interpreter-mode-alist (cons (purecopy name) :js)))

BTW, my suggested patch basically proposes to use `<LANG>-mode` instead
of `:LANG>` which saves us from those changes since that matches our
historical conventions.

Another issue I see if we don't use something like
`derived-mode-add-parents` is that all the various places where we use
mode-indexing, such as `.dir-locals.el`, `ffap`, YASnippet, etc... will
need to be extended with a way to use "languages" as well, and then we
also need to define a sane precedence between settings that apply to
a given mode and settings that apply to a given language (setting for
`js-ts-mode` should presumably take precedence over settings for
`:js` which should take precedence over settings for `prog-mode`).

        Stefan