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

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

[Wilhelm Kirschbaum]

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.

Wilhelm

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

[Eli Zaretskii]

> 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.

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

[Wilhelm Kirschbaum]

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.

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

[Eli Zaretskii]

> 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.

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

[Wilhelm Kirschbaum]

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.

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

[Yuan Fu]

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.

Yuan

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

[Yuan Fu]

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