GNU bug report logs - #64818
30.0.50; c++-ts-mode highlight does not work

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 64818 in the body.
You can then email your comments to 64818 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Wang Diancheng <dianchengwang <at> gmail.com>
To: bug-gnu-emacs <bug-gnu-emacs <at> gnu.org>
Subject: 30.0.50; c++-ts-mode highlight does not work
Date: Mon, 24 Jul 2023 12:49:06 +0800

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

Hi,

I use the latest tree-sitter and tree-sitter modules.
Only preprocessor instructions and comments are highlighted, please see attached
screenshot. BTW c-ts-mode highlight is OK.

Following are values of treesit-font-lock-feature-list and
treesit-font-lock-level:

treesit-font-lock-feature-list is a variable defined in ‘treesit.el’.

Its value is
((comment definition) (keyword preprocessor string type)
 (assignment constant escape-sequence label literal)
 (bracket delimiter error function operator property variable))
Local in buffer sql_select.cc; global value is nil

treesit-font-lock-level is a variable defined in ‘treesit.el’.

Its value is 3

Thanks.

In GNU Emacs 30.0.50 (build 11, x86_64-pc-linux-gnu, GTK+ Version
 3.22.30, cairo version 1.15.12) of 2023-07-24 built on h4
Repository revision: 865817633f6e4b548c9d138fdf792394d021e2f5
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12013000
System Description: TencentOS Server 3.2 (Final)

Configured using:
 'configure CC=/usr/bin/gcc 'CFLAGS=-g3 -O2' CXX=/usr/bin/g++
 'CXXFLAGS=-g3 -O2''

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LIBSELINUX LIBSYSTEMD LIBXML2 MODULES NOTIFY INOTIFY PDUMPER PNG
RSVG SECCOMP SOUND SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS TREE_SITTER
X11 XDBE XIM XINPUT2 XPM GTK3 ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: C++//

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message mailcap yank-media puny dired
dired-loaddefs rfc822 mml mml-sec password-cache epa derived epg rfc6068
epg-config gnus-util text-property-search time-date subr-x mm-decode
mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader
sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils
c-ts-mode c-ts-common treesit cl-seq vc-git diff-mode easy-mmode
vc-dispatcher cc-mode cc-fonts cc-guess cc-menus cc-cmds cc-styles
cc-align cc-engine cc-vars cc-defs cl-loaddefs cl-lib rmc iso-transl
tooltip cconv eldoc paren electric uniquify ediff-hook vc-hooks
lisp-float-type elisp-mode mwheel term/x-win x-win term/common-win x-dnd
tool-bar dnd fontset image regexp-opt fringe tabulated-list replace
newcomment text-mode lisp-mode prog-mode register page tab-bar menu-bar
rfn-eshadow isearch easymenu timer select scroll-bar mouse jit-lock
font-lock syntax font-core term/tty-colors frame minibuffer nadvice seq
simple cl-generic indonesian philippine cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms
cp51932 hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese composite emoji-zwj charscript charprop case-table
epa-hook jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button
loaddefs theme-loaddefs faces cus-face macroexp files window
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget keymap hashtable-print-readable backquote threads dbusbind
inotify dynamic-setting system-font-setting font-render-setting cairo
gtk x-toolkit xinput2 x multi-tty move-toolbar make-network-process
emacs)

Memory information:
((conses 16 76872 12520) (symbols 48 7835 0) (strings 32 22549 1631)
 (string-bytes 1 812511) (vectors 16 14075)
 (vector-slots 8 195709 11372) (floats 8 28 263) (intervals 56 3811 0)
 (buffers 984 14))

[c++-ts-mode.png (image/png, attachment)]

[c-ts-mode.png (image/png, attachment)]

Message #8 received at 64818 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Wang Diancheng <dianchengwang <at> gmail.com>, Yuan Fu <casouri <at> gmail.com>,
 Theodor Thornhill <theo <at> thornhill.no>
Cc: 64818 <at> debbugs.gnu.org
Subject: Re: bug#64818: 30.0.50; c++-ts-mode highlight does not work
Date: Mon, 24 Jul 2023 14:54:53 +0300

> From: Wang Diancheng <dianchengwang <at> gmail.com>
> Date: Mon, 24 Jul 2023 12:49:06 +0800
> 
> I use the latest tree-sitter and tree-sitter modules.
> Only preprocessor instructions and comments are highlighted, please see attached
> screenshot. BTW c-ts-mode highlight is OK.
> 
> Following are values of treesit-font-lock-feature-list and
> treesit-font-lock-level:
> 
> treesit-font-lock-feature-list is a variable defined in ‘treesit.el’.
> 
> Its value is
> ((comment definition) (keyword preprocessor string type)
>  (assignment constant escape-sequence label literal)
>  (bracket delimiter error function operator property variable))
> Local in buffer sql_select.cc; global value is nil
> 
> treesit-font-lock-level is a variable defined in ‘treesit.el’.
> 
> Its value is 3

Thanks.

Yuan, Theo: can you please look into this?  I can confirm the above
behavior, and I also see it in the very first pretest 29.0.90.  So
either some change broke c++-ts-mode before April, or maybe the latest
changes in the C++ grammar library did that.  treesit-explore-mode
seems to show that the code is parsed correctly, so why aren't
fontifications working?

Message #13 received at 64818 <at> debbugs.gnu.org (full text, mbox):

From: Theodor Thornhill <theo <at> thornhill.no>
To: Eli Zaretskii <eliz <at> gnu.org>, Wang Diancheng <dianchengwang <at> gmail.com>,
 Yuan Fu <casouri <at> gmail.com>
Cc: 64818 <at> debbugs.gnu.org
Subject: Re: bug#64818: 30.0.50; c++-ts-mode highlight does not work
Date: Mon, 24 Jul 2023 15:10:29 +0200

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Wang Diancheng <dianchengwang <at> gmail.com>
>> Date: Mon, 24 Jul 2023 12:49:06 +0800
>> 
>> I use the latest tree-sitter and tree-sitter modules.
>> Only preprocessor instructions and comments are highlighted, please see attached
>> screenshot. BTW c-ts-mode highlight is OK.
>> 
>> Following are values of treesit-font-lock-feature-list and
>> treesit-font-lock-level:
>> 
>> treesit-font-lock-feature-list is a variable defined in ‘treesit.el’.
>> 
>> Its value is
>> ((comment definition) (keyword preprocessor string type)
>>  (assignment constant escape-sequence label literal)
>>  (bracket delimiter error function operator property variable))
>> Local in buffer sql_select.cc; global value is nil
>> 
>> treesit-font-lock-level is a variable defined in ‘treesit.el’.
>> 
>> Its value is 3
>
> Thanks.
>
> Yuan, Theo: can you please look into this?  I can confirm the above
> behavior, and I also see it in the very first pretest 29.0.90.  So
> either some change broke c++-ts-mode before April, or maybe the latest
> changes in the C++ grammar library did that.  treesit-explore-mode
> seems to show that the code is parsed correctly, so why aren't
> fontifications working?

I'll look into it :)

Theo

Message #16 received at 64818 <at> debbugs.gnu.org (full text, mbox):

From: Theodor Thornhill <theo <at> thornhill.no>
To: Eli Zaretskii <eliz <at> gnu.org>, Wang Diancheng <dianchengwang <at> gmail.com>,
 Yuan Fu <casouri <at> gmail.com>
Cc: 64818 <at> debbugs.gnu.org
Subject: Re: bug#64818: 30.0.50; c++-ts-mode highlight does not work
Date: Mon, 24 Jul 2023 15:26:36 +0200

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Wang Diancheng <dianchengwang <at> gmail.com>
>> Date: Mon, 24 Jul 2023 12:49:06 +0800
>> 
>> I use the latest tree-sitter and tree-sitter modules.
>> Only preprocessor instructions and comments are highlighted, please see attached
>> screenshot. BTW c-ts-mode highlight is OK.
>> 
>> Following are values of treesit-font-lock-feature-list and
>> treesit-font-lock-level:
>> 
>> treesit-font-lock-feature-list is a variable defined in ‘treesit.el’.
>> 
>> Its value is
>> ((comment definition) (keyword preprocessor string type)
>>  (assignment constant escape-sequence label literal)
>>  (bracket delimiter error function operator property variable))
>> Local in buffer sql_select.cc; global value is nil
>> 
>> treesit-font-lock-level is a variable defined in ‘treesit.el’.
>> 
>> Its value is 3
>
> Thanks.
>
> Yuan, Theo: can you please look into this?  I can confirm the above
> behavior, and I also see it in the very first pretest 29.0.90.  So
> either some change broke c++-ts-mode before April, or maybe the latest
> changes in the C++ grammar library did that.  treesit-explore-mode
> seems to show that the code is parsed correctly, so why aren't
> fontifications working?

With the version of the grammar on my system everything worked
perfectly, but I updated the grammar, and now I can reproduce. So it
seems something upstream broke it. I'll bisect tree-sitter-cpp and
figure out whats breaking it.

Thanks,
Theo

Message #19 received at 64818 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Theodor Thornhill <theo <at> thornhill.no>
Cc: 64818 <at> debbugs.gnu.org, dianchengwang <at> gmail.com, casouri <at> gmail.com
Subject: Re: bug#64818: 30.0.50; c++-ts-mode highlight does not work
Date: Mon, 24 Jul 2023 16:46:47 +0300

> From: Theodor Thornhill <theo <at> thornhill.no>
> Cc: 64818 <at> debbugs.gnu.org
> Date: Mon, 24 Jul 2023 15:26:36 +0200
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> >> From: Wang Diancheng <dianchengwang <at> gmail.com>
> >> Date: Mon, 24 Jul 2023 12:49:06 +0800
> >> 
> >> I use the latest tree-sitter and tree-sitter modules.
> >> Only preprocessor instructions and comments are highlighted, please see attached
> >> screenshot. BTW c-ts-mode highlight is OK.
> >> 
> >> Following are values of treesit-font-lock-feature-list and
> >> treesit-font-lock-level:
> >> 
> >> treesit-font-lock-feature-list is a variable defined in ‘treesit.el’.
> >> 
> >> Its value is
> >> ((comment definition) (keyword preprocessor string type)
> >>  (assignment constant escape-sequence label literal)
> >>  (bracket delimiter error function operator property variable))
> >> Local in buffer sql_select.cc; global value is nil
> >> 
> >> treesit-font-lock-level is a variable defined in ‘treesit.el’.
> >> 
> >> Its value is 3
> >
> > Thanks.
> >
> > Yuan, Theo: can you please look into this?  I can confirm the above
> > behavior, and I also see it in the very first pretest 29.0.90.  So
> > either some change broke c++-ts-mode before April, or maybe the latest
> > changes in the C++ grammar library did that.  treesit-explore-mode
> > seems to show that the code is parsed correctly, so why aren't
> > fontifications working?
> 
> With the version of the grammar on my system everything worked
> perfectly, but I updated the grammar, and now I can reproduce. So it
> seems something upstream broke it. I'll bisect tree-sitter-cpp and
> figure out whats breaking it.

Thanks.  The other report about this perhaps gives a hint:

> From: David Come <david.come <at> ageagle.com>
> Date: Mon, 24 Jul 2023 09:13:33 +0000
> msip_labels: 
> 
> When opening a C++ file with major mode c++-ts-mode, there is not 
> coloration 
> 
> In the Messsage buffer, I see 
> 
> Error during redisplay: (jit-lock-function 14) signaled (treesit-query-error "Node type error at" 99 "(true)
> @font-lock-constant-face (false) @font-lock-constant-face (null) @font-lock-constant-face (nullptr)
> @font-lock-constant-face" "Debug the query with `treesit-query-validate'") [2 times] 

Message #22 received at 64818 <at> debbugs.gnu.org (full text, mbox):

From: Theodor Thornhill <theo <at> thornhill.no>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 64818 <at> debbugs.gnu.org, dianchengwang <at> gmail.com, casouri <at> gmail.com
Subject: Re: bug#64818: 30.0.50; c++-ts-mode highlight does not work
Date: Mon, 24 Jul 2023 16:31:41 +0200

Eli Zaretskii <eliz <at> gnu.org> writes:


[...]

> Thanks.  The other report about this perhaps gives a hint:
>
>> From: David Come <david.come <at> ageagle.com>
>> Date: Mon, 24 Jul 2023 09:13:33 +0000
>> msip_labels: 
>> 
>> When opening a C++ file with major mode c++-ts-mode, there is not 
>> coloration 
>> 
>> In the Messsage buffer, I see 
>> 
>> Error during redisplay: (jit-lock-function 14) signaled (treesit-query-error "Node type error at" 99 "(true)
>> @font-lock-constant-face (false) @font-lock-constant-face (null) @font-lock-constant-face (nullptr)
>> @font-lock-constant-face" "Debug the query with `treesit-query-validate'") [2 times] 


Yep, nullptr was changed from named node to unnamed node last week [0].

I think we can live without a compat change and only target the node
as a normal keyword. I'll commit the fix if it is simple enough (the
simplest is just to remove the node altogether),
otherwise I'll send a patch for review. Sounds ok?

Theo

[0]: https://github.com/tree-sitter/tree-sitter-c/commit/c75868f8b508ae32a0c8490da91bb31b2b96430e#diff-f6ccc66a5a64b1af49076514274f92e3d50daaa5d97bb1c0db000df04fdc946bR3979

Message #25 received at 64818 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Theodor Thornhill <theo <at> thornhill.no>
Cc: 64818 <at> debbugs.gnu.org, dianchengwang <at> gmail.com, casouri <at> gmail.com
Subject: Re: bug#64818: 30.0.50; c++-ts-mode highlight does not work
Date: Mon, 24 Jul 2023 19:08:44 +0300

> From: Theodor Thornhill <theo <at> thornhill.no>
> Cc: dianchengwang <at> gmail.com, casouri <at> gmail.com, 64818 <at> debbugs.gnu.org
> Date: Mon, 24 Jul 2023 16:31:41 +0200
> 
> >> Error during redisplay: (jit-lock-function 14) signaled (treesit-query-error "Node type error at" 99 "(true)
> >> @font-lock-constant-face (false) @font-lock-constant-face (null) @font-lock-constant-face (nullptr)
> >> @font-lock-constant-face" "Debug the query with `treesit-query-validate'") [2 times] 
> 
> 
> Yep, nullptr was changed from named node to unnamed node last week [0].
> 
> I think we can live without a compat change and only target the node
> as a normal keyword. I'll commit the fix if it is simple enough (the
> simplest is just to remove the node altogether),
> otherwise I'll send a patch for review. Sounds ok?

I'd prefer to see the patch.  Also, can you tell more about the effect
of the change you propose ("remove the node")?

More generally, I'm a bit worried by such incompatible changes in the
grammar libraries.  The developers must understand that they break
users of tree-sitter, right?  So why are they making such incompatible
changes?  And how do other editors cope with such changes, for example
this one?

I'm asking these questions because perhaps we are doing something we
shouldn't, or not doing something we should.  I don't think we can
tell our users to use only a specific commit from the grammar
libraries' repositories: a significant portion of Emacs users tend to
switch to a new version many moons after the release (e.g., I see
reports from people who only now upgrade from Emacs 27 to Emacs 28,
more than a year since Emacs 28 was released).  So a grammar library
which was the current one on the release date will be hopelessly
outdated by the time some users will switch to that Emacs version.

So we must look for some more robust way, if it exists.

Message #28 received at 64818 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Theodor Thornhill <theo <at> thornhill.no>, casouri <at> gmail.com
Cc: 64818 <at> debbugs.gnu.org, dianchengwang <at> gmail.com
Subject: Re: bug#64818: 30.0.50; c++-ts-mode highlight does not work
Date: Mon, 24 Jul 2023 19:56:05 +0300

> Date: Mon, 24 Jul 2023 18:40:44 +0200
> From: Theodor Thornhill <theo <at> thornhill.no>
> 
> >> Yep, nullptr was changed from named node to unnamed node last week [0].
> >> 
> >> I think we can live without a compat change and only target the node
> >> as a normal keyword. I'll commit the fix if it is simple enough (the
> >> simplest is just to remove the node altogether),
> >> otherwise I'll send a patch for review. Sounds ok?
> >
> >I'd prefer to see the patch.  Also, can you tell more about the effect
> >of the change you propose ("remove the node")?
> >
> 
> In this case it will only make the symbol "nullptr" get no font locking.

That's probably good enough.  And CC Mode doesn't fontify it, either.

Can you show the patch?

> >More generally, I'm a bit worried by such incompatible changes in the
> >grammar libraries.  The developers must understand that they break
> >users of tree-sitter, right?  So why are they making such incompatible
> >changes?  And how do other editors cope with such changes, for example
> >this one?
> 
> An example from nvim-treesitter: https://github.com/nvim-treesitter/nvim-treesitter/commit/823e67a1c9452075ec7f01e7aa05ac6e7b41fb1e
> 
> It seems most, if not all implementations use some sort of lockfile, where commits are frozen according to the current support. The consensus seems to be to do what I proposed some mails ago: show the last known commit the current file supports, and enable that one to be installed automatically.

I'm not sure how we would maintain this data.  Emacs is a large
project, and people come and go at will and without further notice.
We don't have people who will reliably track the development of the
grammar libraries and record the commits somewhere.  We'd basically
need this when a release is being tarred, and for that it should be
recorded somewhere in advance.

> >I'm asking these questions because perhaps we are doing something we
> >shouldn't, or not doing something we should.  I don't think we can
> >tell our users to use only a specific commit from the grammar
> >libraries' repositories: a significant portion of Emacs users tend to
> >switch to a new version many moons after the release (e.g., I see
> >reports from people who only now upgrade from Emacs 27 to Emacs 28,
> >more than a year since Emacs 28 was released).  So a grammar library
> >which was the current one on the release date will be hopelessly
> >outdated by the time some users will switch to that Emacs version.
> >
> >So we must look for some more robust way, if it exists.
> 
> I agree, but I'm not sure what that looks like.

What about catching errors inside treesit.c or treesit.el, so that the
features that disappeared and queries that fail don't fail the entire
font-lock?  Would that work, or at least make Emacs more robust in the
face of such changes?

Yuan, WDYT?

(This more robust approach is certainly not for Emacs 29.1, even if we
agree that it's a good idea.)

Message #31 received at 64818 <at> debbugs.gnu.org (full text, mbox):

From: Theodor Thornhill <theo <at> thornhill.no>
To: Eli Zaretskii <eliz <at> gnu.org>, casouri <at> gmail.com
Cc: 64818 <at> debbugs.gnu.org, dianchengwang <at> gmail.com
Subject: Re: bug#64818: 30.0.50; c++-ts-mode highlight does not work
Date: Mon, 24 Jul 2023 19:13:07 +0200

Eli Zaretskii <eliz <at> gnu.org> writes:

>> Date: Mon, 24 Jul 2023 18:40:44 +0200
>> From: Theodor Thornhill <theo <at> thornhill.no>
>> 
>> >> Yep, nullptr was changed from named node to unnamed node last week [0].
>> >> 
>> >> I think we can live without a compat change and only target the node
>> >> as a normal keyword. I'll commit the fix if it is simple enough (the
>> >> simplest is just to remove the node altogether),
>> >> otherwise I'll send a patch for review. Sounds ok?
>> >
>> >I'd prefer to see the patch.  Also, can you tell more about the effect
>> >of the change you propose ("remove the node")?
>> >
>> 
>> In this case it will only make the symbol "nullptr" get no font locking.
>
> That's probably good enough.  And CC Mode doesn't fontify it, either.
>
> Can you show the patch?
>

diff --git a/lisp/progmodes/c-ts-mode.el b/lisp/progmodes/c-ts-mode.el
index 7f4f6f11387..98797bf3ce7 100644
--- a/lisp/progmodes/c-ts-mode.el
+++ b/lisp/progmodes/c-ts-mode.el
@@ -574,9 +574,7 @@ c-ts-mode--font-lock-settings
    :feature 'constant
    `((true) @font-lock-constant-face
      (false) @font-lock-constant-face
-     (null) @font-lock-constant-face
-     ,@(when (eq mode 'cpp)
-         '((nullptr) @font-lock-constant-face)))
+     (null) @font-lock-constant-face)
 
    :language mode
    :feature 'keyword


>> >More generally, I'm a bit worried by such incompatible changes in the
>> >grammar libraries.  The developers must understand that they break
>> >users of tree-sitter, right?  So why are they making such incompatible
>> >changes?  And how do other editors cope with such changes, for example
>> >this one?
>> 
>> An example from nvim-treesitter: https://github.com/nvim-treesitter/nvim-treesitter/commit/823e67a1c9452075ec7f01e7aa05ac6e7b41fb1e
>> 
>> It seems most, if not all implementations use some sort of lockfile, where commits are frozen according to the current support. The consensus seems to be to do what I proposed some mails ago: show the last known commit the current file supports, and enable that one to be installed automatically.
>
> I'm not sure how we would maintain this data.  Emacs is a large
> project, and people come and go at will and without further notice.
> We don't have people who will reliably track the development of the
> grammar libraries and record the commits somewhere.  We'd basically
> need this when a release is being tarred, and for that it should be
> recorded somewhere in advance.
>

Yeah, it's not a super simple problem.

>> >I'm asking these questions because perhaps we are doing something we
>> >shouldn't, or not doing something we should.  I don't think we can
>> >tell our users to use only a specific commit from the grammar
>> >libraries' repositories: a significant portion of Emacs users tend to
>> >switch to a new version many moons after the release (e.g., I see
>> >reports from people who only now upgrade from Emacs 27 to Emacs 28,
>> >more than a year since Emacs 28 was released).  So a grammar library
>> >which was the current one on the release date will be hopelessly
>> >outdated by the time some users will switch to that Emacs version.
>> >
>> >So we must look for some more robust way, if it exists.
>> 
>> I agree, but I'm not sure what that looks like.
>
> What about catching errors inside treesit.c or treesit.el, so that the
> features that disappeared and queries that fail don't fail the entire
> font-lock?  Would that work, or at least make Emacs more robust in the
> face of such changes?
>
> Yuan, WDYT?
>
> (This more robust approach is certainly not for Emacs 29.1, even if we
> agree that it's a good idea.)

I'll defer that to Yuan, as I'm not 100% on where such errors can be
caught, and if it can make the parser enter some state it shouldn't be
in.

Theo

Message #34 received at 64818 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Theodor Thornhill <theo <at> thornhill.no>
Cc: dianchengwang <at> gmail.com, casouri <at> gmail.com, 64818 <at> debbugs.gnu.org
Subject: Re: bug#64818: 30.0.50; c++-ts-mode highlight does not work
Date: Mon, 24 Jul 2023 21:08:48 +0300

> From: Theodor Thornhill <theo <at> thornhill.no>
> Cc: 64818 <at> debbugs.gnu.org, dianchengwang <at> gmail.com
> Date: Mon, 24 Jul 2023 19:13:07 +0200
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> > Can you show the patch?
> >
> 
> diff --git a/lisp/progmodes/c-ts-mode.el b/lisp/progmodes/c-ts-mode.el
> index 7f4f6f11387..98797bf3ce7 100644
> --- a/lisp/progmodes/c-ts-mode.el
> +++ b/lisp/progmodes/c-ts-mode.el
> @@ -574,9 +574,7 @@ c-ts-mode--font-lock-settings
>     :feature 'constant
>     `((true) @font-lock-constant-face
>       (false) @font-lock-constant-face
> -     (null) @font-lock-constant-face
> -     ,@(when (eq mode 'cpp)
> -         '((nullptr) @font-lock-constant-face)))
> +     (null) @font-lock-constant-face)
>  
>     :language mode
>     :feature 'keyword

Thanks, please install this on the emacs-29 branch.

> > What about catching errors inside treesit.c or treesit.el, so that the
> > features that disappeared and queries that fail don't fail the entire
> > font-lock?  Would that work, or at least make Emacs more robust in the
> > face of such changes?
> >
> > Yuan, WDYT?
> >
> > (This more robust approach is certainly not for Emacs 29.1, even if we
> > agree that it's a good idea.)
> 
> I'll defer that to Yuan, as I'm not 100% on where such errors can be
> caught, and if it can make the parser enter some state it shouldn't be
> in.

AFAIU, it isn't the parser that signaled an error, it's our code which
queried tree-sitter about a node that doesn't exist.

Message #37 received at 64818 <at> debbugs.gnu.org (full text, mbox):

From: Theodor Thornhill <theo <at> thornhill.no>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: dianchengwang <at> gmail.com, casouri <at> gmail.com, 64818 <at> debbugs.gnu.org
Subject: Re: bug#64818: 30.0.50; c++-ts-mode highlight does not work
Date: Mon, 24 Jul 2023 20:22:48 +0200

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Theodor Thornhill <theo <at> thornhill.no>
>> Cc: 64818 <at> debbugs.gnu.org, dianchengwang <at> gmail.com
>> Date: Mon, 24 Jul 2023 19:13:07 +0200
>> 
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>> 
>> > Can you show the patch?
>> >
>> 
>> diff --git a/lisp/progmodes/c-ts-mode.el b/lisp/progmodes/c-ts-mode.el
>> index 7f4f6f11387..98797bf3ce7 100644
>> --- a/lisp/progmodes/c-ts-mode.el
>> +++ b/lisp/progmodes/c-ts-mode.el
>> @@ -574,9 +574,7 @@ c-ts-mode--font-lock-settings
>>     :feature 'constant
>>     `((true) @font-lock-constant-face
>>       (false) @font-lock-constant-face
>> -     (null) @font-lock-constant-face
>> -     ,@(when (eq mode 'cpp)
>> -         '((nullptr) @font-lock-constant-face)))
>> +     (null) @font-lock-constant-face)
>>  
>>     :language mode
>>     :feature 'keyword
>
> Thanks, please install this on the emacs-29 branch.
>

Now done

>> > What about catching errors inside treesit.c or treesit.el, so that the
>> > features that disappeared and queries that fail don't fail the entire
>> > font-lock?  Would that work, or at least make Emacs more robust in the
>> > face of such changes?
>> >
>> > Yuan, WDYT?
>> >
>> > (This more robust approach is certainly not for Emacs 29.1, even if we
>> > agree that it's a good idea.)
>> 
>> I'll defer that to Yuan, as I'm not 100% on where such errors can be
>> caught, and if it can make the parser enter some state it shouldn't be
>> in.
>
> AFAIU, it isn't the parser that signaled an error, it's our code which
> queried tree-sitter about a node that doesn't exist.

True, my bad.

Theo

Message #40 received at 64818 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Eli Zaretskii <eliz <at> gnu.org>, Theodor Thornhill <theo <at> thornhill.no>,
 casouri <at> gmail.com
Cc: 64818 <at> debbugs.gnu.org, dianchengwang <at> gmail.com
Subject: Re: bug#64818: 30.0.50; c++-ts-mode highlight does not work
Date: Mon, 24 Jul 2023 23:33:16 +0300

On 24/07/2023 19:56, Eli Zaretskii wrote:
>>> More generally, I'm a bit worried by such incompatible changes in the
>>> grammar libraries.  The developers must understand that they break
>>> users of tree-sitter, right?  So why are they making such incompatible
>>> changes?  And how do other editors cope with such changes, for example
>>> this one?
>> An example from nvim-treesitter:https://github.com/nvim-treesitter/nvim-treesitter/commit/823e67a1c9452075ec7f01e7aa05ac6e7b41fb1e
>>
>> It seems most, if not all implementations use some sort of lockfile, where commits are frozen according to the current support. The consensus seems to be to do what I proposed some mails ago: show the last known commit the current file supports, and enable that one to be installed automatically.
> I'm not sure how we would maintain this data.  Emacs is a large
> project, and people come and go at will and without further notice.
> We don't have people who will reliably track the development of the
> grammar libraries and record the commits somewhere.  We'd basically
> need this when a release is being tarred, and for that it should be
> recorded somewhere in advance.
> 

I'm guessing this info will have be tracked by the maintainers of the 
respective ts major modes.

The same people who have to be aware of what grammar nodes are available 
anyway, know about the changes in the grammar, and etc.

Message #43 received at 64818 <at> debbugs.gnu.org (full text, mbox):

From: Yuan Fu <casouri <at> gmail.com>
To: Theodor Thornhill <theo <at> thornhill.no>
Cc: dianchengwang <at> gmail.com, Eli Zaretskii <eliz <at> gnu.org>,
 64818 <at> debbugs.gnu.org
Subject: Re: bug#64818: 30.0.50; c++-ts-mode highlight does not work
Date: Tue, 15 Aug 2023 23:14:04 -0700

> On Jul 24, 2023, at 10:13 AM, Theodor Thornhill <theo <at> thornhill.no> wrote:
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
>>> Date: Mon, 24 Jul 2023 18:40:44 +0200
>>> From: Theodor Thornhill <theo <at> thornhill.no>
>>> 
>>>>> Yep, nullptr was changed from named node to unnamed node last week [0].
>>>>> 
>>>>> I think we can live without a compat change and only target the node
>>>>> as a normal keyword. I'll commit the fix if it is simple enough (the
>>>>> simplest is just to remove the node altogether),
>>>>> otherwise I'll send a patch for review. Sounds ok?
>>>> 
>>>> I'd prefer to see the patch.  Also, can you tell more about the effect
>>>> of the change you propose ("remove the node")?
>>>> 
>>> 
>>> In this case it will only make the symbol "nullptr" get no font locking.
>> 
>> That's probably good enough.  And CC Mode doesn't fontify it, either.
>> 
>> Can you show the patch?
>> 
> 
> diff --git a/lisp/progmodes/c-ts-mode.el b/lisp/progmodes/c-ts-mode.el
> index 7f4f6f11387..98797bf3ce7 100644
> --- a/lisp/progmodes/c-ts-mode.el
> +++ b/lisp/progmodes/c-ts-mode.el
> @@ -574,9 +574,7 @@ c-ts-mode--font-lock-settings
>    :feature 'constant
>    `((true) @font-lock-constant-face
>      (false) @font-lock-constant-face
> -     (null) @font-lock-constant-face
> -     ,@(when (eq mode 'cpp)
> -         '((nullptr) @font-lock-constant-face)))
> +     (null) @font-lock-constant-face)
> 
>    :language mode
>    :feature 'keyword
> 
> 
>>>> More generally, I'm a bit worried by such incompatible changes in the
>>>> grammar libraries.  The developers must understand that they break
>>>> users of tree-sitter, right?  So why are they making such incompatible
>>>> changes?  And how do other editors cope with such changes, for example
>>>> this one?
>>> 
>>> An example from nvim-treesitter: https://github.com/nvim-treesitter/nvim-treesitter/commit/823e67a1c9452075ec7f01e7aa05ac6e7b41fb1e
>>> 
>>> It seems most, if not all implementations use some sort of lockfile, where commits are frozen according to the current support. The consensus seems to be to do what I proposed some mails ago: show the last known commit the current file supports, and enable that one to be installed automatically.
>> 
>> I'm not sure how we would maintain this data.  Emacs is a large
>> project, and people come and go at will and without further notice.
>> We don't have people who will reliably track the development of the
>> grammar libraries and record the commits somewhere.  We'd basically
>> need this when a release is being tarred, and for that it should be
>> recorded somewhere in advance.
>> 
> 
> Yeah, it's not a super simple problem.
> 
>>>> I'm asking these questions because perhaps we are doing something we
>>>> shouldn't, or not doing something we should.  I don't think we can
>>>> tell our users to use only a specific commit from the grammar
>>>> libraries' repositories: a significant portion of Emacs users tend to
>>>> switch to a new version many moons after the release (e.g., I see
>>>> reports from people who only now upgrade from Emacs 27 to Emacs 28,
>>>> more than a year since Emacs 28 was released).  So a grammar library
>>>> which was the current one on the release date will be hopelessly
>>>> outdated by the time some users will switch to that Emacs version.
>>>> 
>>>> So we must look for some more robust way, if it exists.
>>> 
>>> I agree, but I'm not sure what that looks like.
>> 
>> What about catching errors inside treesit.c or treesit.el, so that the
>> features that disappeared and queries that fail don't fail the entire
>> font-lock?  Would that work, or at least make Emacs more robust in the
>> face of such changes?
>> 
>> Yuan, WDYT?
>> 
>> (This more robust approach is certainly not for Emacs 29.1, even if we
>> agree that it's a good idea.)
> 
> I'll defer that to Yuan, as I'm not 100% on where such errors can be
> caught, and if it can make the parser enter some state it shouldn't be
> in.

By default, queries are compiled lazily—only compiled when used in the first time. That’ll be in treesit-font-lock-fontify-region. We can catch the error and remote the bad query there. Though we should still have some warning displayed so the user knows something is wrong. I’ll work o this.

Yuan

Previous Next

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