bug#60511: 29.0.50; treesit-ready-p should not emit warning by default

bug#60511: 29.0.50; treesit-ready-p should not emit warning by default

[Stefan Kangas]

Severity: wishlist

This currently emits a warning if the ruby grammar is not installed:

    (treesit-ready-p 'ruby)

I think it should *not* emit a warning, as no other predicates in Emacs
do (e.g. `featurep', `integerp', etc.).

It could have an optional flag to emit a warning, if there's a strong
need for that.  But personally, I'd rather see a new function for that.

I also don't see much need for the `message' symbol as the second
argument, so I'd simplify the API by dropping that part.  It's currently
unused in our tree.

bug#60511: 29.0.50; treesit-ready-p should not emit warning by default

[Juri Linkov]

> This currently emits a warning if the ruby grammar is not installed:
>
>     (treesit-ready-p 'ruby)
>
> I think it should *not* emit a warning, as no other predicates in Emacs
> do (e.g. `featurep', `integerp', etc.).
>
> It could have an optional flag to emit a warning, if there's a strong
> need for that.  But personally, I'd rather see a new function for that.
>
> I also don't see much need for the `message' symbol as the second
> argument, so I'd simplify the API by dropping that part.  It's currently
> unused in our tree.

Like the argument NOERROR of `require', the argument QUIET of `treesit-ready-p'
could do the same.

bug#60511: 29.0.50; treesit-ready-p should not emit warning by default

[Yuan Fu]

Juri Linkov <juri@linkov.net> writes:

>> This currently emits a warning if the ruby grammar is not installed:
>>
>>     (treesit-ready-p 'ruby)
>>
>> I think it should *not* emit a warning, as no other predicates in Emacs
>> do (e.g. `featurep', `integerp', etc.).
>>
>> It could have an optional flag to emit a warning, if there's a strong
>> need for that.  But personally, I'd rather see a new function for that.
>>
>> I also don't see much need for the `message' symbol as the second
>> argument, so I'd simplify the API by dropping that part.  It's currently
>> unused in our tree.
>
> Like the argument NOERROR of `require', the argument QUIET of `treesit-ready-p'
> could do the same.

Maybe rename it to treesit-check-readiness?

Yuan

bug#60511: 29.0.50; treesit-ready-p should not emit warning by default

[Juri Linkov]

>>> This currently emits a warning if the ruby grammar is not installed:
>>>
>>>     (treesit-ready-p 'ruby)
>>>
>>> I think it should *not* emit a warning, as no other predicates in Emacs
>>> do (e.g. `featurep', `integerp', etc.).
>>>
>>> It could have an optional flag to emit a warning, if there's a strong
>>> need for that.  But personally, I'd rather see a new function for that.
>>>
>>> I also don't see much need for the `message' symbol as the second
>>> argument, so I'd simplify the API by dropping that part.  It's currently
>>> unused in our tree.
>>
>> Like the argument NOERROR of `require', the argument QUIET of `treesit-ready-p'
>> could do the same.
>
> Maybe rename it to treesit-check-readiness?

I think treesit-ready-p already is a good name.
We just need to support more values in its argument QUIET,
with a new value that does nothing in case of an error,
and just returns nil.

bug#60511: 29.0.50; treesit-ready-p should not emit warning by default

[Yuan Fu]

Juri Linkov <juri@linkov.net> writes:

>>>> This currently emits a warning if the ruby grammar is not installed:
>>>>
>>>>     (treesit-ready-p 'ruby)
>>>>
>>>> I think it should *not* emit a warning, as no other predicates in Emacs
>>>> do (e.g. `featurep', `integerp', etc.).
>>>>
>>>> It could have an optional flag to emit a warning, if there's a strong
>>>> need for that.  But personally, I'd rather see a new function for that.
>>>>
>>>> I also don't see much need for the `message' symbol as the second
>>>> argument, so I'd simplify the API by dropping that part.  It's currently
>>>> unused in our tree.
>>>
>>> Like the argument NOERROR of `require', the argument QUIET of `treesit-ready-p'
>>> could do the same.
>>
>> Maybe rename it to treesit-check-readiness?
>
> I think treesit-ready-p already is a good name.
> We just need to support more values in its argument QUIET,
> with a new value that does nothing in case of an error,
> and just returns nil.

It already has such option: if QUIET is t, treesit-ready-p returns nil and don’t emit anything.

I can make treesit-ready-p not emit any warning by default, and change
the quiet parameter to WARN, and accept either 'warn or ‘message.

Basically:

(treesit-ready-p lang) => t/nil
(treesit-ready-p lang 'warn) => t/emit warning
(treesit-ready-p lang 'message) => t/message

Eli, WDYT?

Yuan

bug#60511: 29.0.50; treesit-ready-p should not emit warning by default

[Eli Zaretskii]

> From: Yuan Fu <casouri@gmail.com>
> Date: Sat, 7 Jan 2023 17:31:16 -0800
> Cc: Stefan Kangas <stefankangas@gmail.com>,
>  60511@debbugs.gnu.org,
>  Eli Zaretskii <eliz@gnu.org>
> 
> 
> Juri Linkov <juri@linkov.net> writes:
> 
> >>>> This currently emits a warning if the ruby grammar is not installed:
> >>>>
> >>>>     (treesit-ready-p 'ruby)
> >>>>
> >>>> I think it should *not* emit a warning, as no other predicates in Emacs
> >>>> do (e.g. `featurep', `integerp', etc.).
> >>>>
> >>>> It could have an optional flag to emit a warning, if there's a strong
> >>>> need for that.  But personally, I'd rather see a new function for that.
> >>>>
> >>>> I also don't see much need for the `message' symbol as the second
> >>>> argument, so I'd simplify the API by dropping that part.  It's currently
> >>>> unused in our tree.
> >>>
> >>> Like the argument NOERROR of `require', the argument QUIET of `treesit-ready-p'
> >>> could do the same.
> >>
> >> Maybe rename it to treesit-check-readiness?
> >
> > I think treesit-ready-p already is a good name.
> > We just need to support more values in its argument QUIET,
> > with a new value that does nothing in case of an error,
> > and just returns nil.
> 
> It already has such option: if QUIET is t, treesit-ready-p returns nil and don’t emit anything.
> 
> I can make treesit-ready-p not emit any warning by default, and change
> the quiet parameter to WARN, and accept either 'warn or ‘message.
> 
> Basically:
> 
> (treesit-ready-p lang) => t/nil
> (treesit-ready-p lang 'warn) => t/emit warning
> (treesit-ready-p lang 'message) => t/message
> 
> Eli, WDYT?

I think the default should be to emit a warning, like we do now.  We
should support the main use case of the user turning on a TS mode when
the required libraries are not installed or incompatible with our
requirements.  Silently doing nothing in that case is not TRT.

No objections from me to extend the QUIET argument other than that.

bug#60511: 29.0.50; treesit-ready-p should not emit warning by default

[Juri Linkov]

>> I can make treesit-ready-p not emit any warning by default, and change
>> the quiet parameter to WARN, and accept either 'warn or ‘message.
>> 
>> Basically:
>> 
>> (treesit-ready-p lang) => t/nil
>> (treesit-ready-p lang 'warn) => t/emit warning
>> (treesit-ready-p lang 'message) => t/message
>> 
>> Eli, WDYT?
>
> I think the default should be to emit a warning, like we do now.

Then how users could change this default?

bug#60511: 29.0.50; treesit-ready-p should not emit warning by default

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: Yuan Fu <casouri@gmail.com>,  stefankangas@gmail.com,
>   60511@debbugs.gnu.org
> Date: Sun, 08 Jan 2023 10:39:36 +0200
> 
> >> I can make treesit-ready-p not emit any warning by default, and change
> >> the quiet parameter to WARN, and accept either 'warn or ‘message.
> >> 
> >> Basically:
> >> 
> >> (treesit-ready-p lang) => t/nil
> >> (treesit-ready-p lang 'warn) => t/emit warning
> >> (treesit-ready-p lang 'message) => t/message
> >> 
> >> Eli, WDYT?
> >
> > I think the default should be to emit a warning, like we do now.
> 
> Then how users could change this default?

In what situation?

If they invoke the mode, they aren't supposed to disable the warning,
and they cannot.

bug#60511: 29.0.50; treesit-ready-p should not emit warning by default

[Juri Linkov]

>> >> I can make treesit-ready-p not emit any warning by default, and change
>> >> the quiet parameter to WARN, and accept either 'warn or ‘message.
>> >>
>> >> Basically:
>> >>
>> >> (treesit-ready-p lang) => t/nil
>> >> (treesit-ready-p lang 'warn) => t/emit warning
>> >> (treesit-ready-p lang 'message) => t/message
>> >>
>> >> Eli, WDYT?
>> >
>> > I think the default should be to emit a warning, like we do now.
>>
>> Then how users could change this default?
>
> In what situation?
>
> If they invoke the mode, they aren't supposed to disable the warning,
> and they cannot.

In a situation when they want simply to visit a file without using
tree-sitter features, and that visit won't require from them
customization with modifying auto-mode-alist, etc.

bug#60511: 29.0.50; treesit-ready-p should not emit warning by default

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: casouri@gmail.com,  stefankangas@gmail.com,  60511@debbugs.gnu.org
> Date: Sun, 08 Jan 2023 19:35:27 +0200
> 
> >> >> I can make treesit-ready-p not emit any warning by default, and change
> >> >> the quiet parameter to WARN, and accept either 'warn or ‘message.
> >> >>
> >> >> Basically:
> >> >>
> >> >> (treesit-ready-p lang) => t/nil
> >> >> (treesit-ready-p lang 'warn) => t/emit warning
> >> >> (treesit-ready-p lang 'message) => t/message
> >> >>
> >> >> Eli, WDYT?
> >> >
> >> > I think the default should be to emit a warning, like we do now.
> >>
> >> Then how users could change this default?
> >
> > In what situation?
> >
> > If they invoke the mode, they aren't supposed to disable the warning,
> > and they cannot.
> 
> In a situation when they want simply to visit a file without using
> tree-sitter features, and that visit won't require from them
> customization with modifying auto-mode-alist, etc.

This will soon become a non-issue, as I plan on removing all the TS
modes from auto-mode-alist.  Only loading the mode will add it back to
the alist.  So there will be no surprises, and no need to conceal the
warning.

bug#60511: 29.0.50; treesit-ready-p should not emit warning by default

[Juri Linkov]

>> >> > I think the default should be to emit a warning, like we do now.
>> >>
>> >> Then how users could change this default?
>> >
>> > In what situation?
>> >
>> > If they invoke the mode, they aren't supposed to disable the warning,
>> > and they cannot.
>>
>> In a situation when they want simply to visit a file without using
>> tree-sitter features, and that visit won't require from them
>> customization with modifying auto-mode-alist, etc.
>
> This will soon become a non-issue, as I plan on removing all the TS
> modes from auto-mode-alist.  Only loading the mode will add it back to
> the alist.  So there will be no surprises, and no need to conceal the
> warning.

Why users need to always load modes that they are using only occasionally?

bug#60511: 29.0.50; treesit-ready-p should not emit warning by default

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: casouri@gmail.com,  stefankangas@gmail.com,  60511@debbugs.gnu.org
> Date: Sun, 08 Jan 2023 20:11:42 +0200
> 
> >> In a situation when they want simply to visit a file without using
> >> tree-sitter features, and that visit won't require from them
> >> customization with modifying auto-mode-alist, etc.
> >
> > This will soon become a non-issue, as I plan on removing all the TS
> > modes from auto-mode-alist.  Only loading the mode will add it back to
> > the alist.  So there will be no surprises, and no need to conceal the
> > warning.
> 
> Why users need to always load modes that they are using only occasionally?

If they want to use it permanently, they can load them in their init
files.

The idea is not to surprise users who were editing these kinds of
files before and don't have tree-sitter installed/configured.  If
these new modes are completely opt-in, they cannot surprise anyone by
an unexpected warning.

bug#60511: 29.0.50; treesit-ready-p should not emit warning by default

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

>> I can make treesit-ready-p not emit any warning by default, and change
>> the quiet parameter to WARN, and accept either 'warn or ‘message.
>>
>> Basically:
>>
>> (treesit-ready-p lang) => t/nil
>> (treesit-ready-p lang 'warn) => t/emit warning
>> (treesit-ready-p lang 'message) => t/message

This makes sense to me.

>> Eli, WDYT?
>
> I think the default should be to emit a warning, like we do now.

In that case, it would be better to rename `treesit-ready-p' to reflect
that it's not a predicate function.  I believe Juri suggested the name
`treesit-check-readiness'.

Then there's the question if we want a predicate function for this too.
I think it would be useful.

> We should support the main use case of the user turning on a TS mode
> when the required libraries are not installed or incompatible with our
> requirements.  Silently doing nothing in that case is not TRT.

To be clear, I did not suggest changing that behavior.

bug#60511: 29.0.50; treesit-ready-p should not emit warning by default

[Juri Linkov]

>> I think the default should be to emit a warning, like we do now.
>
> In that case, it would be better to rename `treesit-ready-p' to reflect
> that it's not a predicate function.  I believe Juri suggested the name
> `treesit-check-readiness'.

Actually, I think `treesit-check-readiness' is a worse name.

bug#60511: 29.0.50; treesit-ready-p should not emit warning by default

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: Eli Zaretskii <eliz@gnu.org>,  Yuan Fu <casouri@gmail.com>,
>   60511@debbugs.gnu.org
> Date: Sun, 08 Jan 2023 20:28:45 +0200
> 
> >> I think the default should be to emit a warning, like we do now.
> >
> > In that case, it would be better to rename `treesit-ready-p' to reflect
> > that it's not a predicate function.  I believe Juri suggested the name
> > `treesit-check-readiness'.
> 
> Actually, I think `treesit-check-readiness' is a worse name.

I see no reason to rename the function, so let's not argue about its
name, okay?