bug#64818: 30.0.50; c++-ts-mode highlight does not work

[Yuan Fu <casouri@gmail.com>]

> On Jul 24, 2023, at 10:13 AM, Theodor Thornhill <theo@thornhill.no> wrote:
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
>>> Date: Mon, 24 Jul 2023 18:40:44 +0200
>>> From: Theodor Thornhill <theo@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