bug#60453: 29.0.60; treesit-range-rules throw an error without tree-sitter

[Yuan Fu <casouri@gmail.com>]

Yuan Fu <casouri@gmail.com> writes:

> Wilhelm Kirschbaum <wkirschbaum@gmail.com> writes:
>
>> Eli Zaretskii <eliz@gnu.org> writes:
>>
>>>> From: Wilhelm Kirschbaum <wkirschbaum@gmail.com>
>>>> Cc: 60453@debbugs.gnu.org
>>>> Date: Sat, 31 Dec 2022 18:50:31 +0200
>>>> Eli Zaretskii <eliz@gnu.org> writes:
>>>>  >> From: Wilhelm Kirschbaum <wkirschbaum@gmail.com>
>>>> >> Date: Sat, 31 Dec 2022 16:53:08 +0200
>>>> >>  >>  >> With the following code without tree-sitter library:
>>>> >>  >> (defvar elixir-ts-mode--treesit-range-rules
>>>> >>   (treesit-range-rules
>>>> >>    :embed 'heex
>>>> >>    :host 'elixir
>>>> >>    '((sigil (sigil_name) @name (:match "^[H]$" @name)  >>
>>>> (quoted_content)
>>>> >>   @heex))))
>>>> >>  >> upon loading the mode I get the following error:
>>>> >>  >> treesit-range-rules: Symbol’s function definition is void:
>>>> >> treesit-query-compile
>>>> >>  >> This can easily be mitigated with (when  >>
>>>> (treesit-available-p)...)
>>>> >> but think it should function similar to how  >>
>>>> (treesit-font-lock-rules
>>>> >> work.
>>>> >
>>>> > Why does it make sense to protect treesit.el's code with
>>>> > treesit-available-p?  You aren't supposed to use treesit.el  >
>>>> functions
>>>> > when the tree-sitter library is not available.  IOW, Lisp  >
>>>> programs
>>>> > that want to use treesit-range-rules and other functions from
>>>> > treesit.el should make the treesit-available-p test _before_  >
>>>> that.
>>>> Okay, that makes sense.  I just saw this comment on
>>>> ;; treesit.el#618
>>>> (defun treesit-font-lock-rules (&rest query-specs)
>>>>   ...
>>>>   ;; Other tree-sitter function don't tend to be called unless
>>>>   ;; tree-sitter is enabled, which means tree-sitter must be
>>>> compiled.
>>>>   ;; But this function is usually call in `defvar' which runs
>>>>   ;; regardless whether tree-sitter is enabled.  So we need   this
>>>>   ;; guard.
>>>>   (when (treesit-available-p)
>>>> As treesit-range-rules also gets called with defvar and it is a
>>>> consistency issue.  I think the reason why this has not popped up
>>>> before is that no other modes I have seen uses treesit-range-rules
>>>> yet and think it will probably catch people off guard in the
>>>> future.
>>>
>>> It's up to Yuan: if he thinks this is a good idea, he should feel
>>> free
>>> to add that test.  But it's slippery slope, IMNSHO: we will very
>>> soon
>>> find ourselves adding such tests to every treesit.el function, just
>>> because some code somewhere calls that function without a prior
>>> test.
>>> IOW, IMO a single case of such callers is not enough to add a test.
>>> But that's me.
>>
>> Okay, I will add the checks before defvar anyways to keep things
>> consistent on my side.  It does make more sense to me just to not have
>> the
>> guards in the first place as it creates false expectation they will be
>> everywhere.
>
> I wonder if we should remove that guard in treesit-font-lock-rules... It
> looked like a good idea at the time, but now I can see it creating
> confusion going forward.

I think it’s too late to change treesit-font-lock-rules, maybe we should
add the guard to treesit-range-rules, just to be consistent? We can make
it an convention to add guards to all treesit-xxx-rules functions.

Yuan