bug#74367: [PATCH] c-ts-mode.el: Adjust doc about how to change default modes

bug#74367: [PATCH] c-ts-mode.el: Adjust doc about how to change default modes

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1: Type: text/plain, Size: 1009 bytes --]

Tags: patch

The doc of `c-ts-mode.el` currently recommends things we deprecate.
The patch below fixes this.

I think this can go into `emacs-30`, but I believe Eli prefers we leave
this alone there, so I guess it would go to `master`.  If we can install
at least part of it into `emacs-30` (such as the removal of the
recommendation to `require` or the suggestion to use the obsolete
`c-or-c++-ts-mode`), I'll happily split it.


        Stefan


 In GNU Emacs 31.0.50 (build 1, i686-pc-linux-gnu, GTK+ Version 3.24.43,
 cairo version 1.18.2) of 2024-10-17 built on alfajor
Repository revision: 1df1a3f4a40aa6c49dd29db1b5a7ad47b6b2087c
Repository branch: work
Windowing system distributor 'The X.Org Foundation', version 11.0.12101013
System Description: Debian GNU/Linux trixie/sid

Configured using:
 'configure -C --enable-checking --enable-check-lisp-object-type --with-modules --with-cairo --with-tiff=ifavailable
 'CFLAGS=-Wall -g3 -Og -Wno-pointer-sign'
 PKG_CONFIG_PATH=/home/monnier/lib/pkgconfig'


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-c-ts-mode.el-Adjust-doc-about-how-to-change-default-.patch --]
[-- Type: text/patch, Size: 4041 bytes --]

From f6cb19d44fe2ff1abdf890c609264f86ee74e2d7 Mon Sep 17 00:00:00 2001
From: Stefan Monnier <monnier@iro.umontreal.ca>
Date: Fri, 15 Nov 2024 11:41:33 -0500
Subject: [PATCH] c-ts-mode.el: Adjust doc about how to change default modes

Don't recommend (require 'c-ts-mode) since it's not reliable (loading
`cc-mode.el` later on will override that preference) and also it
is incompatible with our policy that loading a package shouldn't
change Emacs's behavior.
Don't recommend customizing `auto-mode-alist` since it's more
complicated, only covers some of the ways a C mode can be selected,
and those who really want that kind of control probably know about
`auto-mode-alist` anyway.

* lisp/progmodes/c-ts-mode.el (Commentary:): Adjust
recommendation for how to use these modes by default:
(c-ts-mode, c++-ts-mode): Don't suggest remapping `c-or-c++-mode`
to `c-or-c++-ts-mode` since `c-or-c++-ts-mode` is deprecated
since `c-or-c++-mode` already obeys the remapping of `c/c++-mode`.
---
 lisp/progmodes/c-ts-mode.el | 35 +++++++----------------------------
 1 file changed, 7 insertions(+), 28 deletions(-)

diff --git a/lisp/progmodes/c-ts-mode.el b/lisp/progmodes/c-ts-mode.el
index 3823c553fda..19e511bbac3 100644
--- a/lisp/progmodes/c-ts-mode.el
+++ b/lisp/progmodes/c-ts-mode.el
@@ -28,36 +28,17 @@
 ;; functions that are useful generally to major modes for C-like
 ;; languages.
 ;;
-;; This package provides `c-ts-mode' for C, `c++-ts-mode' for C++, and
-;; `c-or-c++-ts-mode' which automatically chooses the right mode for
-;; C/C++ header files.
+;; This package provides `c-ts-mode' for C and `c++-ts-mode' for C++.
 ;;
 ;; To use these modes by default, assuming you have the respective
-;; tree-sitter grammars available, do one of the following:
-;;
-;; - If you have both C and C++ grammars installed, add
-;;
-;;    (require 'c-ts-mode)
-;;
-;;   to your init file.
-;;
-;; - Add one or mode of the following to your init file:
+;; tree-sitter grammars available, add one or more of the following to
+;; your init file:
 ;;
 ;;    (add-to-list 'major-mode-remap-alist '(c-mode . c-ts-mode))
 ;;    (add-to-list 'major-mode-remap-alist '(c++-mode . c++-ts-mode))
-;;    (add-to-list 'major-mode-remap-alist '(c-or-c++-mode . c-or-c++-ts-mode))
-;;
-;;   If you have only C grammar available, use only the first one; if
-;;   you have only the C++ grammar, use only the second one.
-;;
-;; - Customize 'auto-mode-alist' to turn one or more of the modes
-;;   automatically.  For example:
-;;
-;;     (add-to-list 'auto-mode-alist
-;;                  '("\\(\\.ii\\|\\.\\(CC?\\|HH?\\)\\|\\.[ch]\\(pp\\|xx\\|\\+\\+\\)\\|\\.\\(cc\\|hh\\)\\)\\'"
-;;                    . c++-ts-mode))
 ;;
-;;   will turn on the c++-ts-mode for C++ source files.
+;; If you have only C grammar available, use only the first one; if
+;; you have only the C++ grammar, use only the second one.
 ;;
 ;; You can also turn on these modes manually in a buffer.  Doing so
 ;; will set up Emacs to use the C/C++ modes defined here for other
@@ -1340,8 +1321,6 @@ c-ts-mode
 
     (add-to-list \\='major-mode-remap-alist \\='(c-mode . c-ts-mode))
     (add-to-list \\='major-mode-remap-alist \\='(c++-mode . c++-ts-mode))
-    (add-to-list \\='major-mode-remap-alist
-                 \\='(c-or-c++-mode . c-or-c++-ts-mode))
 
 in your init files."
   :group 'c
@@ -1410,8 +1389,6 @@ c++-ts-mode
 
     (add-to-list \\='major-mode-remap-alist \\='(c-mode . c-ts-mode))
     (add-to-list \\='major-mode-remap-alist \\='(c++-mode . c++-ts-mode))
-    (add-to-list \\='major-mode-remap-alist
-                 \\='(c-or-c++-mode . c-or-c++-ts-mode))
 
 in your init files.
 
@@ -1513,6 +1490,8 @@ c-ts-mode--c-or-c++-regexp
               "\\|" "#include"  ws-maybe "<" (regexp-opt headers) ">"
               "\\)")))
   "A regexp applied to C header files to check if they are really C++.")
+(make-obsolete-variable 'c-ts-mode--c-or-c++-regexp
+                        'c-or-c++-mode--regexp "30.1")
 
 ;;;###autoload
 (defun c-or-c++-ts-mode ()
-- 
2.45.2

bug#74367: [PATCH] c-ts-mode.el: Adjust doc about how to change default modes

[Stefan Kangas]

Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of
text editors" <bug-gnu-emacs@gnu.org> writes:

> Tags: patch
>
> The doc of `c-ts-mode.el` currently recommends things we deprecate.
> The patch below fixes this.
>
> I think this can go into `emacs-30`, but I believe Eli prefers we leave
> this alone there, so I guess it would go to `master`.  If we can install
> at least part of it into `emacs-30` (such as the removal of the
> recommendation to `require` or the suggestion to use the obsolete
> `c-or-c++-ts-mode`), I'll happily split it.

FWIW, I agree that we should at least remove the recommendation to use
`require` on emacs-30, as it encourages bad habits and contributes to an
already poor understanding of autoloading among users.

bug#74367: [PATCH] c-ts-mode.el: Adjust doc about how to change default modes

[Eli Zaretskii]

> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Fri, 15 Nov 2024 13:15:38 -0800
> 
> Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of
> text editors" <bug-gnu-emacs@gnu.org> writes:
> 
> > Tags: patch
> >
> > The doc of `c-ts-mode.el` currently recommends things we deprecate.
> > The patch below fixes this.
> >
> > I think this can go into `emacs-30`, but I believe Eli prefers we leave
> > this alone there, so I guess it would go to `master`.  If we can install
> > at least part of it into `emacs-30` (such as the removal of the
> > recommendation to `require` or the suggestion to use the obsolete
> > `c-or-c++-ts-mode`), I'll happily split it.
> 
> FWIW, I agree that we should at least remove the recommendation to use
> `require` on emacs-30, as it encourages bad habits and contributes to an
> already poor understanding of autoloading among users.

This all should have been discussed when major-mode-remap-defaults was
added and c-ts-mode was modified to use it.  Not now, sigh.

Anyway, I don't want to remove the 'require' part from the commentary,
but I think we should modify it to use 'load', not 'require' (given
the discussion in bug#74339), and we should make it the second
alternative, after the one that talks about using
major-mode-remap-alist.

I also don't think I agree with these removals:

> @@ -1340,8 +1321,6 @@ c-ts-mode
>  
>      (add-to-list \\='major-mode-remap-alist \\='(c-mode . c-ts-mode))
>      (add-to-list \\='major-mode-remap-alist \\='(c++-mode . c++-ts-mode))
> -    (add-to-list \\='major-mode-remap-alist
> -                 \\='(c-or-c++-mode . c-or-c++-ts-mode))
>  
>  in your init files."
>    :group 'c
> @@ -1410,8 +1389,6 @@ c++-ts-mode
>  
>      (add-to-list \\='major-mode-remap-alist \\='(c-mode . c-ts-mode))
>      (add-to-list \\='major-mode-remap-alist \\='(c++-mode . c++-ts-mode))
> -    (add-to-list \\='major-mode-remap-alist
> -                 \\='(c-or-c++-mode . c-or-c++-ts-mode))
>  
>  in your init files.

The commit log message says

> (c-ts-mode, c++-ts-mode): Don't suggest remapping `c-or-c++-mode`
> to `c-or-c++-ts-mode` since `c-or-c++-ts-mode` is deprecated
> since `c-or-c++-mode` already obeys the remapping of `c/c++-mode`.

However, if we don't remap c-or-c++-mode, it will load cc-mode.el,
which loads a bunch of other cc-*.el files, which is not clean, since
all we need is a single regexp.  (And note that c-ts-mode doesn't
limit the search for the regexp, whereas cc-mode.el does.)  So I think
if we want to _really_ deprecate c-or-c++-ts-mode, we should make this
variable defined on a separate file, common to CC Mode and c-ts-mode,
which doesn't load anything else.  Perhaps in subr.el or files.el.

P.S. As an aside, I find our handling of this and related issues quite
appalling.  Variables introduced (or should I say "sneaked in"?),
without properly discussing their effects on the users and without
properly documenting those effects; unclean tricks to "fight off"
changes installed by others, without properly describing those changes
and triggering a discussion; etc.  This is unbecoming, and almost
feels like some kind of court intrigues.  We should know better how to
deal with controversial changes.

bug#74367: [PATCH] c-ts-mode.el: Adjust doc about how to change default modes

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> FWIW, I agree that we should at least remove the recommendation to use
>> `require` on emacs-30, as it encourages bad habits and contributes to an
>> already poor understanding of autoloading among users.
> This all should have been discussed when major-mode-remap-defaults was
> added and c-ts-mode was modified to use it.  Not now, sigh.

Yup.  I too often focus on the code and skip the doc.  🙁

> Anyway, I don't want to remove the 'require' part from the commentary,
> but I think we should modify it to use 'load', not 'require' (given
> the discussion in bug#74339), and we should make it the second
> alternative, after the one that talks about using
> major-mode-remap-alist.

`load`?  Yuck!
But well, better than nothing (tho it likely won't make much difference
in an init file: the problem of later loading `cc-mode` will still bite
just as much).

I don't understand why you insist on suggesting such a "setting" since
it's unreliable now and probably even more so in the future.
What advantage do you see in it for the user over the
`major-mode-remap-alist` option?

> The commit log message says
>
>> (c-ts-mode, c++-ts-mode): Don't suggest remapping `c-or-c++-mode`
>> to `c-or-c++-ts-mode` since `c-or-c++-ts-mode` is deprecated
>> since `c-or-c++-mode` already obeys the remapping of `c/c++-mode`.
>
> However, if we don't remap c-or-c++-mode, it will load cc-mode.el,
> which loads a bunch of other cc-*.el files, which is not clean, since
> all we need is a single regexp.

...and the `c-or-c++-mode` function itself.

Good point.

> (And note that c-ts-mode doesn't limit the search for the regexp,
> whereas cc-mode.el does.)

Should we harmonize the two?  Do we know if there's a reason other than
accidental for the difference?  Presumably the `cc-mode.el` one has been
more widely tested.

> So I think if we want to _really_ deprecate
> c-or-c++-ts-mode, we should make this variable defined on a separate
> file, common to CC Mode and c-ts-mode, which doesn't load anything
> else.  Perhaps in subr.el or files.el.

BTW, there's a similar issue with the `c-string-list-p` and
`c-string-or-string-list-p` functions which are used for
`safe-local-variable` which would cause `cc-vars.el` to be loaded for
`c-ts-mode` users when a `dir-locals.el` sets some `c-mode` vars.

Maybe we should move those things to a small `cc-utils.el` file which
doesn't load other CC-mode files?

[ Currently, this problem is hidden by the fact that we eagerly load
  `cc-mode.el` anyway (which also loads `cc-vars.el`) even before we try
  to look at those vars's `safe-local-variable` setting.  ]


        Stefan

bug#74367: [PATCH] c-ts-mode.el: Adjust doc about how to change default modes

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Stefan Kangas <stefankangas@gmail.com>,  74367@debbugs.gnu.org, Alan
>  Mackenzie <acm@muc.de>
> Date: Sat, 16 Nov 2024 11:52:48 -0500
> 
> > Anyway, I don't want to remove the 'require' part from the commentary,
> > but I think we should modify it to use 'load', not 'require' (given
> > the discussion in bug#74339), and we should make it the second
> > alternative, after the one that talks about using
> > major-mode-remap-alist.
> 
> `load`?  Yuck!
> But well, better than nothing (tho it likely won't make much difference
> in an init file: the problem of later loading `cc-mode` will still bite
> just as much).
> 
> I don't understand why you insist on suggesting such a "setting" since
> it's unreliable now and probably even more so in the future.

Because that's what users did in Emacs 29.  I don't want us to make an
impression it no longer works.

> What advantage do you see in it for the user over the
> `major-mode-remap-alist` option?

Since I agreed to make the major-mode-remap-alist the first item, is
there still a question of the advantages of 'load' wrt
major-mode-remap-alist?

> > The commit log message says
> >
> >> (c-ts-mode, c++-ts-mode): Don't suggest remapping `c-or-c++-mode`
> >> to `c-or-c++-ts-mode` since `c-or-c++-ts-mode` is deprecated
> >> since `c-or-c++-mode` already obeys the remapping of `c/c++-mode`.
> >
> > However, if we don't remap c-or-c++-mode, it will load cc-mode.el,
> > which loads a bunch of other cc-*.el files, which is not clean, since
> > all we need is a single regexp.
> 
> ...and the `c-or-c++-mode` function itself.
> 
> Good point.
> 
> > (And note that c-ts-mode doesn't limit the search for the regexp,
> > whereas cc-mode.el does.)
> 
> Should we harmonize the two?  Do we know if there's a reason other than
> accidental for the difference?  Presumably the `cc-mode.el` one has been
> more widely tested.

I don't mind limiting the search in c-ts-mode, but then we need two
common variables.

> > So I think if we want to _really_ deprecate
> > c-or-c++-ts-mode, we should make this variable defined on a separate
> > file, common to CC Mode and c-ts-mode, which doesn't load anything
> > else.  Perhaps in subr.el or files.el.
> 
> BTW, there's a similar issue with the `c-string-list-p` and
> `c-string-or-string-list-p` functions which are used for
> `safe-local-variable` which would cause `cc-vars.el` to be loaded for
> `c-ts-mode` users when a `dir-locals.el` sets some `c-mode` vars.

I think we should have a string-or-list-p function in subr.el, like
string-or-null-p we already have there.

> Maybe we should move those things to a small `cc-utils.el` file which
> doesn't load other CC-mode files?

That would also work for me.

bug#74367: [PATCH] c-ts-mode.el: Adjust doc about how to change default modes

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> I don't understand why you insist on suggesting such a "setting" since
>> it's unreliable now and probably even more so in the future.
> Because that's what users did in Emacs 29.  I don't want us to make an
> impression it no longer works.

In Emacs-29, it did work, because it modified `auto-mode-alist` and
CC-mode never modified it back.

In `emacs-30`, it doesn't work reliably any more because loading
`cc-mode.el` after `c-ts-mode.el` causes the default mode to revert to
`c-mode`.  Loading `cc-mode.el` happens when you do `C-h f c-mode RET`,
or when you load a file that requires it, e.g.:

    lisp/cedet/semantic/bovine/c.el:(eval-when-compile (require 'cc-mode))
    lisp/generic-x.el:(require 'cc-mode)
    lisp/org/ob-C.el:(require 'cc-mode)
    lisp/org/ob-fortran.el:(require 'cc-mode)
    lisp/progmodes/antlr-mode.el:(require 'cc-mode)
    lisp/progmodes/cc-mode.el:;; (require 'cc-mode)
    lisp/progmodes/cmacexp.el:(require 'cc-mode)
    lisp/progmodes/csharp-mode.el:(require 'cc-mode)
    lisp/progmodes/cwarn.el:(require 'cc-mode)
    lisp/progmodes/hideif.el:(require 'cc-mode)
    lisp/progmodes/js.el:(require 'cc-mode)

or:

    ../elpa/packages/csharp-mode/csharp-mode.el:(require 'cc-mode)
    ../elpa/packages/electric-spacing/electric-spacing.el:(require 'cc-mode)
    ../elpa/packages/gnome-c-style/gnome-c-align.el:(require 'cc-mode)
    ../elpa/packages/javaimp/javaimp-parse.el:(require 'cc-mode) ;for java-mode-syntax-table
    ../elpa/packages/vcl-mode/vcl-mode.el:(require 'cc-mode)
    ../nongnu/packages/arduino-mode/arduino-mode.el:(require 'cc-mode)
    ../nongnu/packages/bison-mode/bison-mode.el:(require 'cc-mode)
    ../nongnu/packages/d-mode/d-mode.el:(require 'cc-mode)
    ../nongnu/packages/php-mode/lisp/php.el:  (require 'cc-mode)
    ../nongnu/packages/php-mode/lisp/php-mode-debug.el:(require 'cc-mode)
    ../nongnu/packages/php-mode/lisp/php-mode.el:(require 'cc-mode)
    ../nongnu/packages/scad-mode/scad-mode.el:(require 'cc-mode)
    ../nongnu/packages/typescript-mode/typescript-mode.el:  (require 'cc-mode)

Also we're discussing changing `c-ts-mode.el` so loading it doesn't have
that effect at all, which means that there's a good chance it will work
even less well in the future.  So to me it's clear that suggesting the use of
`require` or `load` to change the default C mode is doing a disservice
to our users.


        Stefan

bug#74367: [PATCH] c-ts-mode.el: Adjust doc about how to change default modes

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: stefankangas@gmail.com,  74367@debbugs.gnu.org,  acm@muc.de
> Date: Sat, 16 Nov 2024 15:26:31 -0500
> 
> >> I don't understand why you insist on suggesting such a "setting" since
> >> it's unreliable now and probably even more so in the future.
> > Because that's what users did in Emacs 29.  I don't want us to make an
> > impression it no longer works.
> 
> In Emacs-29, it did work, because it modified `auto-mode-alist` and
> CC-mode never modified it back.
> 
> In `emacs-30`, it doesn't work reliably any more because loading
> `cc-mode.el` after `c-ts-mode.el` causes the default mode to revert to
> `c-mode`.  Loading `cc-mode.el` happens when you do `C-h f c-mode RET`,
> or when you load a file that requires it, e.g.:

This will be better once we fix the major-mode-remap-defaults are
handled by the modes involved in this.  But yes, the effect of your
introduction of major-mode-remap-defaults and the subsequent changes
in cc-mode is that loading the mode works less reliably now.  Which is
why the advice to use major-mode-remap-alist should now be the first
one.

>     lisp/cedet/semantic/bovine/c.el:(eval-when-compile (require 'cc-mode))
>     lisp/generic-x.el:(require 'cc-mode)
>     lisp/org/ob-C.el:(require 'cc-mode)
>     lisp/org/ob-fortran.el:(require 'cc-mode)
>     lisp/progmodes/antlr-mode.el:(require 'cc-mode)
>     lisp/progmodes/cc-mode.el:;; (require 'cc-mode)
>     lisp/progmodes/cmacexp.el:(require 'cc-mode)
>     lisp/progmodes/csharp-mode.el:(require 'cc-mode)
>     lisp/progmodes/cwarn.el:(require 'cc-mode)
>     lisp/progmodes/hideif.el:(require 'cc-mode)
>     lisp/progmodes/js.el:(require 'cc-mode)

See https://debbugs.gnu.org/cgi/bugreport.cgi?bug=74339#38

> Also we're discussing changing `c-ts-mode.el` so loading it doesn't have
> that effect at all

That's not the change I am discussing in bug#74339.

> which means that there's a good chance it will work even less well
> in the future.  So to me it's clear that suggesting the use of
> `require` or `load` to change the default C mode is doing a
> disservice to our users.

Noted.

bug#74367: [PATCH] c-ts-mode.el: Adjust doc about how to change default modes

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

So, for `emacs-30` I propose the patch below.
Any objection?


        Stefan


diff --git a/lisp/progmodes/c-ts-mode.el b/lisp/progmodes/c-ts-mode.el
index 2785e9a6e68..af23cd66d98 100644
--- a/lisp/progmodes/c-ts-mode.el
+++ b/lisp/progmodes/c-ts-mode.el
@@ -35,12 +35,6 @@
 ;; To use these modes by default, assuming you have the respective
 ;; tree-sitter grammars available, do one of the following:
 ;;
-;; - If you have both C and C++ grammars installed, add
-;;
-;;    (require 'c-ts-mode)
-;;
-;;   to your init file.
-;;
 ;; - Add one or mode of the following to your init file:
 ;;
 ;;    (add-to-list 'major-mode-remap-alist '(c-mode . c-ts-mode))
@@ -59,6 +53,12 @@
 ;;
 ;;   will turn on the c++-ts-mode for C++ source files.
 ;;
+;; - If you have both C and C++ grammars installed, add
+;;
+;;    (require 'c-ts-mode)
+;;
+;;   to your init file.
+;;
 ;; You can also turn on these modes manually in a buffer.  Doing so
 ;; will set up Emacs to use the C/C++ modes defined here for other
 ;; files, provided that you have the corresponding parser grammar
@@ -1496,7 +1496,6 @@ c-or-c++-ts-mode
                  '(c++-mode . c++-ts-mode)))
 
 (when (treesit-ready-p 'c)
-  (add-to-list 'major-mode-remap-defaults '(c++-mode . c++-ts-mode))
   (add-to-list 'major-mode-remap-defaults '(c-mode . c-ts-mode)))
 
 (when (and (treesit-ready-p 'cpp)

bug#74367: [PATCH] c-ts-mode.el: Adjust doc about how to change default modes

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: stefankangas@gmail.com,  74367@debbugs.gnu.org,  acm@muc.de
> Date: Sun, 17 Nov 2024 17:19:28 -0500
> 
> So, for `emacs-30` I propose the patch below.
> Any objection?

No objections, provided that you replace

  (require 'c-ts-mode)

with

  (load "c-ts-mode")

since, as we now understand, just 'require' might not do anything.

> @@ -1496,7 +1496,6 @@ c-or-c++-ts-mode
>                   '(c++-mode . c++-ts-mode)))
>  
>  (when (treesit-ready-p 'c)
> -  (add-to-list 'major-mode-remap-defaults '(c++-mode . c++-ts-mode))
>    (add-to-list 'major-mode-remap-defaults '(c-mode . c-ts-mode)))
>  
>  (when (and (treesit-ready-p 'cpp)

Please leave this part alone in the meantime, as long as bug#74339 is
being discussed and is not resolved.  The resolution of that bug will
also handle this nit.