unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#72814: 31.0.50; Add a variable controlling doxygen support  in C/C++/Java?
       [not found] <D6CB8DB6-0D4A-496D-8EFF-4C03BFCC6858@gmail.com>
@ 2024-08-27  3:13 ` Yuan Fu
  2024-08-27  7:36   ` Vincenzo Pupillo
  2024-08-27 12:22   ` Eli Zaretskii
  0 siblings, 2 replies; 12+ messages in thread
From: Yuan Fu @ 2024-08-27  3:13 UTC (permalink / raw)
  To: 72814; +Cc: vincenzo.pupillo, eliz


Yuan Fu <casouri@gmail.com> writes:

> X-Debbugs-CC: eliz@gnu.org <mailto:eliz@gnu.org>,
> vincenzo.pupillo@unimi.it
>
> Should we add a custom option that controls whether to enable doxygen
> support in c-ts-mode/c++-ts-mode/java-ts-mode?
>
> Technically this isn’t a problem, since the doxygen support is only
> enabled if there’s doxygen grammar on the system. But many people
> (me) might install a grammar bundle that includes dozens of
> grammars for convenience. Then the doxygen support would turn on when
> the user doesn’t really intend to use it.
>
> I propose we add custom options like c-ts-mode-enable-doxygen and set it
> to t by default, so the default behavior is still to enable doxygen
> support when the grammar for it exists, but people who don’t want to pay
> for it can turn it off by setting the option to nil.
>
> Yuan

Eli, WDYT?





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#72814: 31.0.50; Add a variable controlling doxygen support  in C/C++/Java?
  2024-08-27  3:13 ` bug#72814: 31.0.50; Add a variable controlling doxygen support in C/C++/Java? Yuan Fu
@ 2024-08-27  7:36   ` Vincenzo Pupillo
  2024-08-27 12:22   ` Eli Zaretskii
  1 sibling, 0 replies; 12+ messages in thread
From: Vincenzo Pupillo @ 2024-08-27  7:36 UTC (permalink / raw)
  To: 72814, casouri; +Cc: eliz

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

Sorry Yuan, the spam filter is aggressive on my work email and I didn't see your email. In early versions of php-ts-mode there were two variables to disable parser injection for html and phpdoc. When, thanks in part to your help, I was able to get indentation and font locking to work properly, I preferred to remove them because they seemed unnecessary.
c-ts-mode has font locking for comments itself, so it is fine to introduce this variable from my point of view.

Vincenzo 

Il 27 agosto 2024 05:13:52 CEST, Yuan Fu <casouri@gmail.com> ha scritto:
>
>Yuan Fu <casouri@gmail.com> writes:
>
>> X-Debbugs-CC: eliz@gnu.org <mailto:eliz@gnu.org>,
>> vincenzo.pupillo@unimi.it
>>
>> Should we add a custom option that controls whether to enable doxygen
>> support in c-ts-mode/c++-ts-mode/java-ts-mode?
>>
>> Technically this isn’t a problem, since the doxygen support is only
>> enabled if there’s doxygen grammar on the system. But many people
>> (me) might install a grammar bundle that includes dozens of
>> grammars for convenience. Then the doxygen support would turn on when
>> the user doesn’t really intend to use it.
>>
>> I propose we add custom options like c-ts-mode-enable-doxygen and set it
>> to t by default, so the default behavior is still to enable doxygen
>> support when the grammar for it exists, but people who don’t want to pay
>> for it can turn it off by setting the option to nil.
>>
>> Yuan
>
>Eli, WDYT?
>
>
>

[-- Attachment #2: Type: text/html, Size: 2027 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#72814: 31.0.50; Add a variable controlling doxygen support  in C/C++/Java?
  2024-08-27  3:13 ` bug#72814: 31.0.50; Add a variable controlling doxygen support in C/C++/Java? Yuan Fu
  2024-08-27  7:36   ` Vincenzo Pupillo
@ 2024-08-27 12:22   ` Eli Zaretskii
  2024-09-09  9:50     ` Vincenzo Pupillo
  1 sibling, 1 reply; 12+ messages in thread
From: Eli Zaretskii @ 2024-08-27 12:22 UTC (permalink / raw)
  To: Yuan Fu; +Cc: 72814, vincenzo.pupillo

> From: Yuan Fu <casouri@gmail.com>
> Date: Mon, 26 Aug 2024 20:13:52 -0700
> Cc: eliz@gnu.org,
>  vincenzo.pupillo@unimi.it
> 
> 
> Yuan Fu <casouri@gmail.com> writes:
> 
> > X-Debbugs-CC: eliz@gnu.org <mailto:eliz@gnu.org>,
> > vincenzo.pupillo@unimi.it
> >
> > Should we add a custom option that controls whether to enable doxygen
> > support in c-ts-mode/c++-ts-mode/java-ts-mode?
> >
> > Technically this isn’t a problem, since the doxygen support is only
> > enabled if there’s doxygen grammar on the system. But many people
> > (me) might install a grammar bundle that includes dozens of
> > grammars for convenience. Then the doxygen support would turn on when
> > the user doesn’t really intend to use it.
> >
> > I propose we add custom options like c-ts-mode-enable-doxygen and set it
> > to t by default, so the default behavior is still to enable doxygen
> > support when the grammar for it exists, but people who don’t want to pay
> > for it can turn it off by setting the option to nil.
> >
> > Yuan
> 
> Eli, WDYT?

I'm okay with such an option, but it should be careful to check
whether the corresponding grammar library is available, and offer a
user-friendly diagnostic if not.





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#72814: 31.0.50; Add a variable controlling doxygen support  in C/C++/Java?
  2024-08-27 12:22   ` Eli Zaretskii
@ 2024-09-09  9:50     ` Vincenzo Pupillo
  2024-09-11  4:53       ` Yuan Fu
  0 siblings, 1 reply; 12+ messages in thread
From: Vincenzo Pupillo @ 2024-09-09  9:50 UTC (permalink / raw)
  To: casouri, 72814; +Cc: eliz, vincenzo.pupillo

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

In data martedì 27 agosto 2024 14:22:16 CEST, Eli Zaretskii ha scritto:
> > From: Yuan Fu <casouri@gmail.com>
> > Date: Mon, 26 Aug 2024 20:13:52 -0700
> > Cc: eliz@gnu.org,
> > 
> >  vincenzo.pupillo@unimi.it
> > 
> > Yuan Fu <casouri@gmail.com> writes:
> > > X-Debbugs-CC: eliz@gnu.org <mailto:eliz@gnu.org>,
> > > vincenzo.pupillo@unimi.it
> > > 
> > > Should we add a custom option that controls whether to enable doxygen
> > > support in c-ts-mode/c++-ts-mode/java-ts-mode?
> > > 
> > > Technically this isn’t a problem, since the doxygen support is only
> > > enabled if there’s doxygen grammar on the system. But many people
> > > (me) might install a grammar bundle that includes dozens of
> > > grammars for convenience. Then the doxygen support would turn on when
> > > the user doesn’t really intend to use it.
> > > 
> > > I propose we add custom options like c-ts-mode-enable-doxygen and set it
> > > to t by default, so the default behavior is still to enable doxygen
> > > support when the grammar for it exists, but people who don’t want to pay
> > > for it can turn it off by setting the option to nil.
> > > 
> > > Yuan
> > 
> > Eli, WDYT?
> 
> I'm okay with such an option, but it should be careful to check
> whether the corresponding grammar library is available, and offer a
> user-friendly diagnostic if not.

Try to take a look at this patch. Is it okay?

Vincenzo

[-- Attachment #2: 0001-Add-a-user-option-to-disable-Doxygen-syntax-highligh.patch --]
[-- Type: text/x-patch, Size: 8147 bytes --]

From 0a6d90f25dfddc1da88b5e47a97bc09fe50a6d34 Mon Sep 17 00:00:00 2001
From: Vincenzo Pupillo <v.pupillo@gmail.com>
Date: Mon, 9 Sep 2024 09:20:42 +0200
Subject: [PATCH] Add a user option to disable Doxygen syntax highlighting
 (bug#72814).

Both 'c-ts-mode' and 'java-ts-mode' have a new user option,
'c-ts-mode-enable-doxygen' and 'java-ts-mode-enable-doxygen'
(defaults to t) which allow to disable syntax highlighting of comment
blocks based on the Doxygen grammar.

* lisp/progmodes/c-ts-mode.el: Add the 'c-ts-mode-enable-doxygen' user
option variable to disable doxygen grammar. (bug#72814)

* lisp/progmodes/c-ts-mode.el (c-ts-mode, c++-ts-mode): Use the new
'c-ts-mode-enable-dodygen' option.

* lisp/progmodes/java-ts-mode.el: Add the 'java-ts-mode-enable-doxygen'
user option variable to disable doxygen grammar. (bug#72814)

* lisp/progmodes/java-ts-mode.el (java-ts-mode): Use the new
'java-ts-mode-enable-dodygen' option.

* etc/NEWS: Document the change.
---
 etc/NEWS                       | 16 ++++++++
 lisp/progmodes/c-ts-mode.el    | 71 ++++++++++++++++++++--------------
 lisp/progmodes/java-ts-mode.el | 35 +++++++++++------
 3 files changed, 83 insertions(+), 39 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index c6f8b0062e4..5762bc22c54 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -224,6 +224,22 @@ package of the current buffer.  It is bound to 'C-c C-t p' in 'go-ts-mode'.
 The 'go-ts-mode-build-tags' user option is available to set a list of
 build tags for the test commands.
 
+** C-ts mode
+
++++
+*** New user option 'c-ts-mode-enable-doxygen'.
+By default, 'c-ts-mode-enable-doxygen' is t, and doxygen comment blocks
+are syntax-highlighted if the doxygen grammar is available.  If Non-nil,
+the comment blocks are highlighted like other comments.
+
+** Java-ts mode
+
++++
+*** New user option 'java-ts-mode-enable-doxygen'.
+By default, 'java-ts-mode-enable-doxygen' is t, and doxygen comment blocks
+are syntax-highlighted if the doxygen grammar is available.  If Non-nil,
+the comment blocks are highlighted like other comments.
+
 ** Emacs Lisp mode
 
 ---
diff --git a/lisp/progmodes/c-ts-mode.el b/lisp/progmodes/c-ts-mode.el
index 7f23b30a88a..3adf5673e67 100644
--- a/lisp/progmodes/c-ts-mode.el
+++ b/lisp/progmodes/c-ts-mode.el
@@ -216,6 +216,17 @@ c-ts-mode-emacs-sources-support
   :safe 'booleanp
   :group 'c)
 
+(defcustom c-ts-mode-enable-doxygen t
+  "Enable doxygen syntax highlighting.
+If nil, disable doxygen based font lock for comment.
+This needs to be set before enabling `c-ts-mode'; if you change
+the value after enabling `c-ts-mode', toggle the mode off and on
+again."
+  :version "31.1"
+  :type 'boolean
+  :safe 'booleanp
+  :group 'c)
+
 ;;; Syntax table
 
 (defvar c-ts-mode--syntax-table
@@ -1354,20 +1365,22 @@ c-ts-mode
         (treesit-font-lock-recompute-features '(emacs-devel)))
 
       ;; Inject doxygen parser for comment.
-      (when (treesit-ready-p 'doxygen t)
-        (setq-local treesit-primary-parser primary-parser)
-        (setq-local treesit-font-lock-settings
-                    (append
-                     treesit-font-lock-settings
-                     c-ts-mode-doxygen-comment-font-lock-settings))
-        (setq-local treesit-range-settings
-                    (treesit-range-rules
-                     :embed 'doxygen
-                     :host 'c
-                     :local t
-                     `(((comment) @cap
-                        (:match
-                         ,c-ts-mode--doxygen-comment-regex @cap)))))))))
+      (when c-ts-mode-enable-doxygen
+        (if (not (treesit-ready-p 'doxygen t))
+            (warn "Doxygen language grammar is unavailable, please install it.")
+          (setq-local treesit-primary-parser primary-parser)
+          (setq-local treesit-font-lock-settings
+                      (append
+                       treesit-font-lock-settings
+                       c-ts-mode-doxygen-comment-font-lock-settings))
+          (setq-local treesit-range-settings
+                      (treesit-range-rules
+                       :embed 'doxygen
+                       :host 'c
+                       :local t
+                       `(((comment) @cap
+                          (:match
+                           ,c-ts-mode--doxygen-comment-regex @cap))))))))))
 
 (derived-mode-add-parents 'c-ts-mode '(c-mode))
 
@@ -1415,20 +1428,22 @@ c++-ts-mode
                     #'c-ts-mode--emacs-current-defun-name))
 
       ;; Inject doxygen parser for comment.
-      (when (treesit-ready-p 'doxygen t)
-        (setq-local treesit-primary-parser primary-parser)
-        (setq-local treesit-font-lock-settings
-                    (append
-                     treesit-font-lock-settings
-                     c-ts-mode-doxygen-comment-font-lock-settings))
-        (setq-local treesit-range-settings
-                    (treesit-range-rules
-                     :embed 'doxygen
-                     :host 'cpp
-                     :local t
-                     `(((comment) @cap
-                        (:match
-                         ,c-ts-mode--doxygen-comment-regex @cap)))))))))
+      (when c-ts-mode-enable-doxygen
+        (if (not (treesit-ready-p 'doxygen t))
+            (warn "Doxygen language grammar is unavailable, please install it.")
+          (setq-local treesit-primary-parser primary-parser)
+          (setq-local treesit-font-lock-settings
+                      (append
+                       treesit-font-lock-settings
+                       c-ts-mode-doxygen-comment-font-lock-settings))
+          (setq-local treesit-range-settings
+                      (treesit-range-rules
+                       :embed 'doxygen
+                       :host 'cpp
+                       :local t
+                       `(((comment) @cap
+                          (:match
+                           ,c-ts-mode--doxygen-comment-regex @cap))))))))))
 
 (derived-mode-add-parents 'c++-ts-mode '(c++-mode))
 
diff --git a/lisp/progmodes/java-ts-mode.el b/lisp/progmodes/java-ts-mode.el
index ac104534734..f12c7ec81ef 100644
--- a/lisp/progmodes/java-ts-mode.el
+++ b/lisp/progmodes/java-ts-mode.el
@@ -48,6 +48,17 @@ java-ts-mode-indent-offset
   :safe 'integerp
   :group 'java)
 
+(defcustom java-ts-mode-enable-doxygen t
+  "Enable doxygen syntax highlighting.
+If nil, disable doxygen based font lock for comment.
+This needs to be set before enabling `java-ts-mode'; if you change
+the value after enabling `java-ts-mode', toggle the mode off and on
+again."
+  :version "31.1"
+  :type 'boolean
+  :safe 'booleanp
+  :group 'java)
+
 (defvar java-ts-mode--syntax-table
   (let ((table (make-syntax-table)))
     ;; Taken from the cc-langs version
@@ -401,17 +412,19 @@ java-ts-mode
                 java-ts-mode--font-lock-settings)
 
     ;; Inject doxygen parser for comment.
-    (when (treesit-ready-p 'doxygen t)
-      (setq-local treesit-primary-parser primary-parser)
-      (setq-local treesit-font-lock-settings
-                  (append treesit-font-lock-settings
-                          c-ts-mode-doxygen-comment-font-lock-settings))
-      (setq-local treesit-range-settings
-                  (treesit-range-rules
-                   :embed 'doxygen
-                   :host 'java
-                   :local t
-                   `(((block_comment) @cap (:match "/\\*\\*" @cap)))))))
+    (when java-ts-mode-enable-doxygen
+      (if (not (treesit-ready-p 'doxygen t))
+          (warn "Doxygen language grammar is unavailable, please install it.")
+        (setq-local treesit-primary-parser primary-parser)
+        (setq-local treesit-font-lock-settings
+                    (append treesit-font-lock-settings
+                            c-ts-mode-doxygen-comment-font-lock-settings))
+        (setq-local treesit-range-settings
+                    (treesit-range-rules
+                     :embed 'doxygen
+                     :host 'java
+                     :local t
+                     `(((block_comment) @cap (:match "/\\*\\*" @cap))))))))
 
   (setq-local treesit-font-lock-feature-list java-ts-mode--feature-list)
 
-- 
2.46.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* bug#72814: 31.0.50; Add a variable controlling doxygen support  in C/C++/Java?
  2024-09-09  9:50     ` Vincenzo Pupillo
@ 2024-09-11  4:53       ` Yuan Fu
  2024-09-11  9:40         ` Vincenzo Pupillo
  0 siblings, 1 reply; 12+ messages in thread
From: Yuan Fu @ 2024-09-11  4:53 UTC (permalink / raw)
  To: Vincenzo Pupillo; +Cc: 72814, eliz, vincenzo.pupillo



> On Sep 9, 2024, at 2:50 AM, Vincenzo Pupillo <v.pupillo@gmail.com> wrote:
> 
> In data martedì 27 agosto 2024 14:22:16 CEST, Eli Zaretskii ha scritto:
>>> From: Yuan Fu <casouri@gmail.com>
>>> Date: Mon, 26 Aug 2024 20:13:52 -0700
>>> Cc: eliz@gnu.org,
>>> 
>>> vincenzo.pupillo@unimi.it
>>> 
>>> Yuan Fu <casouri@gmail.com> writes:
>>>> X-Debbugs-CC: eliz@gnu.org <mailto:eliz@gnu.org>,
>>>> vincenzo.pupillo@unimi.it
>>>> 
>>>> Should we add a custom option that controls whether to enable doxygen
>>>> support in c-ts-mode/c++-ts-mode/java-ts-mode?
>>>> 
>>>> Technically this isn’t a problem, since the doxygen support is only
>>>> enabled if there’s doxygen grammar on the system. But many people
>>>> (me) might install a grammar bundle that includes dozens of
>>>> grammars for convenience. Then the doxygen support would turn on when
>>>> the user doesn’t really intend to use it.
>>>> 
>>>> I propose we add custom options like c-ts-mode-enable-doxygen and set it
>>>> to t by default, so the default behavior is still to enable doxygen
>>>> support when the grammar for it exists, but people who don’t want to pay
>>>> for it can turn it off by setting the option to nil.
>>>> 
>>>> Yuan
>>> 
>>> Eli, WDYT?
>> 
>> I'm okay with such an option, but it should be careful to check
>> whether the corresponding grammar library is available, and offer a
>> user-friendly diagnostic if not.
> 
> Try to take a look at this patch. Is it okay?
> 
> Vincenzo
> <0001-Add-a-user-option-to-disable-Doxygen-syntax-highligh.patch>

Thanks for taking this up! I won’t signal a warning if doxygen grammar isn’t found. Imagine a user without doxygen grammar, and din’t change c-ts-mode-enable-doxygen: they’ll get a warning whenever they open a C file. We should either set c-ts-mode-enable-doxygen to nil by default, or not warn when doxygen grammar doesn’t exist. Otherwise, the patch looks good to me.

Yuan




^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#72814: 31.0.50; Add a variable controlling doxygen support in C/C++/Java?
  2024-09-11  4:53       ` Yuan Fu
@ 2024-09-11  9:40         ` Vincenzo Pupillo
  2024-09-11 12:05           ` Eli Zaretskii
  0 siblings, 1 reply; 12+ messages in thread
From: Vincenzo Pupillo @ 2024-09-11  9:40 UTC (permalink / raw)
  To: Yuan Fu, Eli Zaretskii; +Cc: 72814

In data mercoledì 11 settembre 2024 06:53:32 CEST, Yuan Fu ha scritto:
> 
> > On Sep 9, 2024, at 2:50 AM, Vincenzo Pupillo <v.pupillo@gmail.com> wrote:
> > 
> > In data martedì 27 agosto 2024 14:22:16 CEST, Eli Zaretskii ha scritto:
> >>> From: Yuan Fu <casouri@gmail.com>
> >>> Date: Mon, 26 Aug 2024 20:13:52 -0700
> >>> Cc: eliz@gnu.org,
> >>> 
> >>> vincenzo.pupillo@unimi.it
> >>> 
> >>> Yuan Fu <casouri@gmail.com> writes:
> >>>> X-Debbugs-CC: eliz@gnu.org <mailto:eliz@gnu.org>,
> >>>> vincenzo.pupillo@unimi.it
> >>>> 
> >>>> Should we add a custom option that controls whether to enable doxygen
> >>>> support in c-ts-mode/c++-ts-mode/java-ts-mode?
> >>>> 
> >>>> Technically this isn’t a problem, since the doxygen support is only
> >>>> enabled if there’s doxygen grammar on the system. But many people
> >>>> (me) might install a grammar bundle that includes dozens of
> >>>> grammars for convenience. Then the doxygen support would turn on when
> >>>> the user doesn’t really intend to use it.
> >>>> 
> >>>> I propose we add custom options like c-ts-mode-enable-doxygen and set it
> >>>> to t by default, so the default behavior is still to enable doxygen
> >>>> support when the grammar for it exists, but people who don’t want to pay
> >>>> for it can turn it off by setting the option to nil.
> >>>> 
> >>>> Yuan
> >>> 
> >>> Eli, WDYT?
> >> 
> >> I'm okay with such an option, but it should be careful to check
> >> whether the corresponding grammar library is available, and offer a
> >> user-friendly diagnostic if not.
> > 
> > Try to take a look at this patch. Is it okay?
> > 
> > Vincenzo
> > <0001-Add-a-user-option-to-disable-Doxygen-syntax-highligh.patch>
> 
> Thanks for taking this up! I won’t signal a warning if doxygen grammar isn’t found. Imagine a user without doxygen grammar, and din’t change c-ts-mode-enable-doxygen: they’ll get a warning whenever they open a C file. 
> We should either set c-ts-mode-enable-doxygen to nil by default, or not warn when doxygen grammar doesn’t exist. Otherwise, the patch looks good to me.

Okay, fine. But Eli said something different...
What do we do?

Vincenzo
> 
> Yuan
> 








^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#72814: 31.0.50; Add a variable controlling doxygen support in C/C++/Java?
  2024-09-11  9:40         ` Vincenzo Pupillo
@ 2024-09-11 12:05           ` Eli Zaretskii
  2024-09-12  7:51             ` Yuan Fu
  0 siblings, 1 reply; 12+ messages in thread
From: Eli Zaretskii @ 2024-09-11 12:05 UTC (permalink / raw)
  To: Vincenzo Pupillo; +Cc: 72814, casouri

> From: Vincenzo Pupillo <v.pupillo@gmail.com>
> Cc: Bug Report Emacs <bug-gnu-emacs@gnu.org>, 72814@debbugs.gnu.org
> Date: Wed, 11 Sep 2024 11:40:17 +0200
> 
> > Thanks for taking this up! I won’t signal a warning if doxygen grammar isn’t found. Imagine a user without doxygen grammar, and din’t change c-ts-mode-enable-doxygen: they’ll get a warning whenever they open a C file. 
> > We should either set c-ts-mode-enable-doxygen to nil by default, or not warn when doxygen grammar doesn’t exist. Otherwise, the patch looks good to me.
> 
> Okay, fine. But Eli said something different...

Not necessarily.  All I said was "issue a user-friendly diagnostic".
We could output the message into *Messages*, and do it only once per
Emacs session.





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#72814: 31.0.50; Add a variable controlling doxygen support in C/C++/Java?
  2024-09-11 12:05           ` Eli Zaretskii
@ 2024-09-12  7:51             ` Yuan Fu
  2024-09-12  8:31               ` Eli Zaretskii
  0 siblings, 1 reply; 12+ messages in thread
From: Yuan Fu @ 2024-09-12  7:51 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 72814, v.pupillo



> On Sep 11, 2024, at 5:05 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> 
>> From: Vincenzo Pupillo <v.pupillo@gmail.com>
>> Cc: Bug Report Emacs <bug-gnu-emacs@gnu.org>, 72814@debbugs.gnu.org
>> Date: Wed, 11 Sep 2024 11:40:17 +0200
>> 
>>> Thanks for taking this up! I won’t signal a warning if doxygen grammar isn’t found. Imagine a user without doxygen grammar, and din’t change c-ts-mode-enable-doxygen: they’ll get a warning whenever they open a C file. 
>>> We should either set c-ts-mode-enable-doxygen to nil by default, or not warn when doxygen grammar doesn’t exist. Otherwise, the patch looks good to me.
>> 
>> Okay, fine. But Eli said something different...
> 
> Not necessarily.  All I said was "issue a user-friendly diagnostic".
> We could output the message into *Messages*, and do it only once per
> Emacs session.

Another idea: use tertiary value for c-ts-mode-enable-doxygen, t for enable (and issue warning if doxygen isn’t present), nil for enable if doxygen exists but don’t issue any warning, ‘disabled for disable. (I don’t think there’s a convention for the three values of tertiary toggle, is that right Eli?)

Yuan




^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#72814: 31.0.50; Add a variable controlling doxygen support in C/C++/Java?
  2024-09-12  7:51             ` Yuan Fu
@ 2024-09-12  8:31               ` Eli Zaretskii
  2024-09-14 17:26                 ` Vincenzo Pupillo
  0 siblings, 1 reply; 12+ messages in thread
From: Eli Zaretskii @ 2024-09-12  8:31 UTC (permalink / raw)
  To: Yuan Fu; +Cc: 72814, v.pupillo

> From: Yuan Fu <casouri@gmail.com>
> Date: Thu, 12 Sep 2024 00:51:20 -0700
> Cc: Vincenzo Pupillo <v.pupillo@gmail.com>,
>  Bug Report Emacs <bug-gnu-emacs@gnu.org>,
>  72814@debbugs.gnu.org
> 
> 
> 
> > On Sep 11, 2024, at 5:05 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> > 
> >> From: Vincenzo Pupillo <v.pupillo@gmail.com>
> >> Cc: Bug Report Emacs <bug-gnu-emacs@gnu.org>, 72814@debbugs.gnu.org
> >> Date: Wed, 11 Sep 2024 11:40:17 +0200
> >> 
> >>> Thanks for taking this up! I won’t signal a warning if doxygen grammar isn’t found. Imagine a user without doxygen grammar, and din’t change c-ts-mode-enable-doxygen: they’ll get a warning whenever they open a C file. 
> >>> We should either set c-ts-mode-enable-doxygen to nil by default, or not warn when doxygen grammar doesn’t exist. Otherwise, the patch looks good to me.
> >> 
> >> Okay, fine. But Eli said something different...
> > 
> > Not necessarily.  All I said was "issue a user-friendly diagnostic".
> > We could output the message into *Messages*, and do it only once per
> > Emacs session.
> 
> Another idea: use tertiary value for c-ts-mode-enable-doxygen, t for enable (and issue warning if doxygen isn’t present), nil for enable if doxygen exists but don’t issue any warning, ‘disabled for disable. (I don’t think there’s a convention for the three values of tertiary toggle, is that right Eli?)

That could also be OK, but I prefer something that does TRT by
default.





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#72814: 31.0.50; Add a variable controlling doxygen support in C/C++/Java?
  2024-09-12  8:31               ` Eli Zaretskii
@ 2024-09-14 17:26                 ` Vincenzo Pupillo
  2024-09-21  9:43                   ` Eli Zaretskii
  0 siblings, 1 reply; 12+ messages in thread
From: Vincenzo Pupillo @ 2024-09-14 17:26 UTC (permalink / raw)
  To: Yuan Fu, Eli Zaretskii; +Cc: 72814

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

In data giovedì 12 settembre 2024 10:31:56 CEST, Eli Zaretskii ha scritto:
> > From: Yuan Fu <casouri@gmail.com>
> > Date: Thu, 12 Sep 2024 00:51:20 -0700
> > Cc: Vincenzo Pupillo <v.pupillo@gmail.com>,
> > 
> >  Bug Report Emacs <bug-gnu-emacs@gnu.org>,
> >  72814@debbugs.gnu.org
> >  
> > > On Sep 11, 2024, at 5:05 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> > >> From: Vincenzo Pupillo <v.pupillo@gmail.com>
> > >> Cc: Bug Report Emacs <bug-gnu-emacs@gnu.org>, 72814@debbugs.gnu.org
> > >> Date: Wed, 11 Sep 2024 11:40:17 +0200
> > >> 
> > >>> Thanks for taking this up! I won’t signal a warning if doxygen grammar
> > >>> isn’t found. Imagine a user without doxygen grammar, and din’t change
> > >>> c-ts-mode-enable-doxygen: they’ll get a warning whenever they open a
> > >>> C file. We should either set c-ts-mode-enable-doxygen to nil by
> > >>> default, or not warn when doxygen grammar doesn’t exist. Otherwise,
> > >>> the patch looks good to me.> >> 
> > >> Okay, fine. But Eli said something different...
> > > 
> > > Not necessarily.  All I said was "issue a user-friendly diagnostic".
> > > We could output the message into *Messages*, and do it only once per
> > > Emacs session.
> > 
> > Another idea: use tertiary value for c-ts-mode-enable-doxygen, t for
> > enable (and issue warning if doxygen isn’t present), nil for enable if
> > doxygen exists but don’t issue any warning, ‘disabled for disable. (I
> > don’t think there’s a convention for the three values of tertiary toggle,
> > is that right Eli?)
> That could also be OK, but I prefer something that does TRT by
> default.

Ciao,
in this new version of the patch, as default, the *-ts-mode-enable-doxygen are 
set to nil, and issue a diagnostic (one per session) when Not-nil and if the  
grammar isn't present. It is OK?

Vincenzo

[-- Attachment #2: 0001-Add-a-user-option-to-enable-Doxygen-syntax-highlight.patch --]
[-- Type: text/x-patch, Size: 6068 bytes --]

From 71de6870d6b341e09fa6c79e38ca73e60fda8805 Mon Sep 17 00:00:00 2001
From: Vincenzo Pupillo <v.pupillo@gmail.com>
Date: Mon, 9 Sep 2024 09:20:42 +0200
Subject: [PATCH] Add a user option to enable Doxygen syntax highlighting
 (bug#72814).

Both 'c-ts-mode' and 'java-ts-mode' have a new user option,
'c-ts-mode-enable-doxygen' and 'java-ts-mode-enable-doxygen'
(defaults to nil) which allow to enable syntax highlighting of comment
blocks based on the Doxygen grammar.

* lisp/progmodes/c-ts-mode.el: Add the 'c-ts-mode-enable-doxygen' user
option variable to disable doxygen grammar. (bug#72814)
Notifies the user if doxygen grammar is not available.

* lisp/progmodes/c-ts-mode.el (c-ts-mode, c++-ts-mode): Use the new
'c-ts-mode-enable-doxygen' option.

* lisp/progmodes/java-ts-mode.el: Add the 'java-ts-mode-enable-doxygen'
user option variable to disable doxygen grammar. (bug#72814)
Notifies the user if doxygen grammar is not available.

* lisp/progmodes/java-ts-mode.el (java-ts-mode): Use the new
'java-ts-mode-enable-doxygen' option.

* etc/NEWS: Document the change.
---
 etc/NEWS                       | 16 ++++++++++++++++
 lisp/progmodes/c-ts-mode.el    | 20 +++++++++++++++++---
 lisp/progmodes/java-ts-mode.el | 18 ++++++++++++++++--
 3 files changed, 49 insertions(+), 5 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index c6f8b0062e4..8de4f886d67 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -224,6 +224,22 @@ package of the current buffer.  It is bound to 'C-c C-t p' in 'go-ts-mode'.
 The 'go-ts-mode-build-tags' user option is available to set a list of
 build tags for the test commands.
 
+** C-ts mode
+
++++
+*** New user option 'c-ts-mode-enable-doxygen'.
+By default, 'c-ts-mode-enable-doxygen' is nil, and the comment blocks
+are highlighted like other comments.  When Non-nil doxygen comment
+blocks are syntax-highlighted if the doxygen grammar is available.
+
+** Java-ts mode
+
++++
+*** New user option 'java-ts-mode-enable-doxygen'.
+By default, 'java-ts-mode-enable-doxygen' is nil, and the comment blocks
+are highlighted like other comments.  When Non-nil doxygen comment
+blocks are syntax-highlighted if the doxygen grammar is available.
+
 ** Emacs Lisp mode
 
 ---
diff --git a/lisp/progmodes/c-ts-mode.el b/lisp/progmodes/c-ts-mode.el
index 7f23b30a88a..10fa3bc3bb3 100644
--- a/lisp/progmodes/c-ts-mode.el
+++ b/lisp/progmodes/c-ts-mode.el
@@ -65,7 +65,7 @@
 ;; libraries installed.
 ;;
 ;; If the tree-sitter doxygen grammar is available, then the comment
-;; blocks will be highlighted according to this grammar.
+;; blocks can be highlighted according to this grammar.
 
 ;;; Code:
 
@@ -216,6 +216,17 @@ c-ts-mode-emacs-sources-support
   :safe 'booleanp
   :group 'c)
 
+(defcustom c-ts-mode-enable-doxygen nil
+  "Enable doxygen syntax highlighting.
+If Non-nil, enable doxygen based font lock for comment blocks.
+This needs to be set before enabling `c-ts-mode'; if you change
+the value after enabling `c-ts-mode', toggle the mode off and on
+again."
+  :version "31.1"
+  :type 'boolean
+  :safe 'booleanp
+  :group 'c)
+
 ;;; Syntax table
 
 (defvar c-ts-mode--syntax-table
@@ -1354,7 +1365,7 @@ c-ts-mode
         (treesit-font-lock-recompute-features '(emacs-devel)))
 
       ;; Inject doxygen parser for comment.
-      (when (treesit-ready-p 'doxygen t)
+      (when (and c-ts-mode-enable-doxygen (treesit-ready-p 'doxygen t))
         (setq-local treesit-primary-parser primary-parser)
         (setq-local treesit-font-lock-settings
                     (append
@@ -1415,7 +1426,7 @@ c++-ts-mode
                     #'c-ts-mode--emacs-current-defun-name))
 
       ;; Inject doxygen parser for comment.
-      (when (treesit-ready-p 'doxygen t)
+      (when (and c-ts-mode-enable-doxygen (treesit-ready-p 'doxygen t))
         (setq-local treesit-primary-parser primary-parser)
         (setq-local treesit-font-lock-settings
                     (append
@@ -1527,6 +1538,9 @@ c-or-c++-ts-mode
            (treesit-ready-p 'c))
   (add-to-list 'major-mode-remap-defaults '(c-or-c++-mode . c-or-c++-ts-mode)))
 
+(when (and c-ts-mode-enable-doxygen (not (treesit-ready-p 'doxygen t)))
+  (message "Doxygen syntax highlighting can't be enabled, please install the language grammar."))
+
 (provide 'c-ts-mode)
 (provide 'c++-ts-mode)
 
diff --git a/lisp/progmodes/java-ts-mode.el b/lisp/progmodes/java-ts-mode.el
index ac104534734..177f914160c 100644
--- a/lisp/progmodes/java-ts-mode.el
+++ b/lisp/progmodes/java-ts-mode.el
@@ -25,7 +25,7 @@
 ;;; Commentary:
 ;;
 ;; If the tree-sitter doxygen grammar is available, then the comment
-;; blocks will be highlighted according to this grammar.
+;; blocks can be highlighted according to this grammar.
 
 ;;; Code:
 
@@ -48,6 +48,17 @@ java-ts-mode-indent-offset
   :safe 'integerp
   :group 'java)
 
+(defcustom java-ts-mode-enable-doxygen nil
+  "Enable doxygen syntax highlighting.
+If Non-nil, enable doxygen based font lock for comment blocks.
+This needs to be set before enabling `java-ts-mode'; if you change
+the value after enabling `java-ts-mode', toggle the mode off and on
+again."
+  :version "31.1"
+  :type 'boolean
+  :safe 'booleanp
+  :group 'java)
+
 (defvar java-ts-mode--syntax-table
   (let ((table (make-syntax-table)))
     ;; Taken from the cc-langs version
@@ -401,7 +412,7 @@ java-ts-mode
                 java-ts-mode--font-lock-settings)
 
     ;; Inject doxygen parser for comment.
-    (when (treesit-ready-p 'doxygen t)
+    (when (and java-ts-mode-enable-doxygen (treesit-ready-p 'doxygen t))
       (setq-local treesit-primary-parser primary-parser)
       (setq-local treesit-font-lock-settings
                   (append treesit-font-lock-settings
@@ -428,6 +439,9 @@ java-ts-mode
 (if (treesit-ready-p 'java)
     (add-to-list 'auto-mode-alist '("\\.java\\'" . java-ts-mode)))
 
+(when (and java-ts-mode-enable-doxygen (not (treesit-ready-p 'doxygen t)))
+  (message "Doxygen syntax highlighting can't be enabled, please install the language grammar."))
+
 (provide 'java-ts-mode)
 
 ;;; java-ts-mode.el ends here
-- 
2.46.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* bug#72814: 31.0.50; Add a variable controlling doxygen support in C/C++/Java?
  2024-09-14 17:26                 ` Vincenzo Pupillo
@ 2024-09-21  9:43                   ` Eli Zaretskii
  2024-09-22  6:45                     ` Yuan Fu
  0 siblings, 1 reply; 12+ messages in thread
From: Eli Zaretskii @ 2024-09-21  9:43 UTC (permalink / raw)
  To: Vincenzo Pupillo; +Cc: casouri, 72814-done, 72814

> From: Vincenzo Pupillo <v.pupillo@gmail.com>
> Cc: bug-gnu-emacs@gnu.org, 72814@debbugs.gnu.org
> Date: Sat, 14 Sep 2024 19:26:07 +0200
> 
> > > Another idea: use tertiary value for c-ts-mode-enable-doxygen, t for
> > > enable (and issue warning if doxygen isn’t present), nil for enable if
> > > doxygen exists but don’t issue any warning, ‘disabled for disable. (I
> > > don’t think there’s a convention for the three values of tertiary toggle,
> > > is that right Eli?)
> > That could also be OK, but I prefer something that does TRT by
> > default.
> 
> Ciao,
> in this new version of the patch, as default, the *-ts-mode-enable-doxygen are 
> set to nil, and issue a diagnostic (one per session) when Not-nil and if the  
> grammar isn't present. It is OK?

Thanks, installed on master, and closing the bug.





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#72814: 31.0.50; Add a variable controlling doxygen support in C/C++/Java?
  2024-09-21  9:43                   ` Eli Zaretskii
@ 2024-09-22  6:45                     ` Yuan Fu
  0 siblings, 0 replies; 12+ messages in thread
From: Yuan Fu @ 2024-09-22  6:45 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 72814, v.pupillo, 72814-done



> On Sep 21, 2024, at 2:43 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> 
>> From: Vincenzo Pupillo <v.pupillo@gmail.com>
>> Cc: bug-gnu-emacs@gnu.org, 72814@debbugs.gnu.org
>> Date: Sat, 14 Sep 2024 19:26:07 +0200
>> 
>>>> Another idea: use tertiary value for c-ts-mode-enable-doxygen, t for
>>>> enable (and issue warning if doxygen isn’t present), nil for enable if
>>>> doxygen exists but don’t issue any warning, ‘disabled for disable. (I
>>>> don’t think there’s a convention for the three values of tertiary toggle,
>>>> is that right Eli?)
>>> That could also be OK, but I prefer something that does TRT by
>>> default.
>> 
>> Ciao,
>> in this new version of the patch, as default, the *-ts-mode-enable-doxygen are 
>> set to nil, and issue a diagnostic (one per session) when Not-nil and if the  
>> grammar isn't present. It is OK?
> 
> Thanks, installed on master, and closing the bug.

Thanks to you both!






^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2024-09-22  6:45 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <D6CB8DB6-0D4A-496D-8EFF-4C03BFCC6858@gmail.com>
2024-08-27  3:13 ` bug#72814: 31.0.50; Add a variable controlling doxygen support in C/C++/Java? Yuan Fu
2024-08-27  7:36   ` Vincenzo Pupillo
2024-08-27 12:22   ` Eli Zaretskii
2024-09-09  9:50     ` Vincenzo Pupillo
2024-09-11  4:53       ` Yuan Fu
2024-09-11  9:40         ` Vincenzo Pupillo
2024-09-11 12:05           ` Eli Zaretskii
2024-09-12  7:51             ` Yuan Fu
2024-09-12  8:31               ` Eli Zaretskii
2024-09-14 17:26                 ` Vincenzo Pupillo
2024-09-21  9:43                   ` Eli Zaretskii
2024-09-22  6:45                     ` Yuan Fu

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).