unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#60511: 29.0.50; treesit-ready-p should not emit warning by default
@ 2023-01-03 11:19 Stefan Kangas
  2023-01-03 17:44 ` Juri Linkov
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Stefan Kangas @ 2023-01-03 11:19 UTC (permalink / raw)
  To: 60511; +Cc: Yuan Fu

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.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#60511: 29.0.50; treesit-ready-p should not emit warning by default
  2023-01-03 11:19 bug#60511: 29.0.50; treesit-ready-p should not emit warning by default Stefan Kangas
@ 2023-01-03 17:44 ` Juri Linkov
  2023-01-04  7:02 ` Yuan Fu
  2023-01-08  1:31 ` Yuan Fu
  2 siblings, 0 replies; 15+ messages in thread
From: Juri Linkov @ 2023-01-03 17:44 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: Yuan Fu, 60511

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





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#60511: 29.0.50; treesit-ready-p should not emit warning by  default
  2023-01-03 11:19 bug#60511: 29.0.50; treesit-ready-p should not emit warning by default Stefan Kangas
  2023-01-03 17:44 ` Juri Linkov
@ 2023-01-04  7:02 ` Yuan Fu
  2023-01-04  7:47   ` Juri Linkov
  2023-01-08  1:31 ` Yuan Fu
  2 siblings, 1 reply; 15+ messages in thread
From: Yuan Fu @ 2023-01-04  7:02 UTC (permalink / raw)
  To: Juri Linkov; +Cc: Stefan Kangas, 60511


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





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#60511: 29.0.50; treesit-ready-p should not emit warning by default
  2023-01-04  7:02 ` Yuan Fu
@ 2023-01-04  7:47   ` Juri Linkov
  0 siblings, 0 replies; 15+ messages in thread
From: Juri Linkov @ 2023-01-04  7:47 UTC (permalink / raw)
  To: Yuan Fu; +Cc: Stefan Kangas, 60511

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





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#60511: 29.0.50; treesit-ready-p should not emit warning by  default
  2023-01-03 11:19 bug#60511: 29.0.50; treesit-ready-p should not emit warning by default Stefan Kangas
  2023-01-03 17:44 ` Juri Linkov
  2023-01-04  7:02 ` Yuan Fu
@ 2023-01-08  1:31 ` Yuan Fu
  2023-01-08  5:53   ` Eli Zaretskii
  2 siblings, 1 reply; 15+ messages in thread
From: Yuan Fu @ 2023-01-08  1:31 UTC (permalink / raw)
  To: Juri Linkov; +Cc: Eli Zaretskii, Stefan Kangas, 60511


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





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#60511: 29.0.50; treesit-ready-p should not emit warning by  default
  2023-01-08  1:31 ` Yuan Fu
@ 2023-01-08  5:53   ` Eli Zaretskii
  2023-01-08  8:39     ` Juri Linkov
  2023-01-08 18:14     ` Stefan Kangas
  0 siblings, 2 replies; 15+ messages in thread
From: Eli Zaretskii @ 2023-01-08  5:53 UTC (permalink / raw)
  To: Yuan Fu; +Cc: stefankangas, 60511, juri

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





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#60511: 29.0.50; treesit-ready-p should not emit warning by default
  2023-01-08  5:53   ` Eli Zaretskii
@ 2023-01-08  8:39     ` Juri Linkov
  2023-01-08 11:03       ` Eli Zaretskii
  2023-01-08 18:14     ` Stefan Kangas
  1 sibling, 1 reply; 15+ messages in thread
From: Juri Linkov @ 2023-01-08  8:39 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Yuan Fu, stefankangas, 60511

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





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#60511: 29.0.50; treesit-ready-p should not emit warning by default
  2023-01-08  8:39     ` Juri Linkov
@ 2023-01-08 11:03       ` Eli Zaretskii
  2023-01-08 17:35         ` Juri Linkov
  0 siblings, 1 reply; 15+ messages in thread
From: Eli Zaretskii @ 2023-01-08 11:03 UTC (permalink / raw)
  To: Juri Linkov; +Cc: casouri, stefankangas, 60511

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





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#60511: 29.0.50; treesit-ready-p should not emit warning by default
  2023-01-08 11:03       ` Eli Zaretskii
@ 2023-01-08 17:35         ` Juri Linkov
  2023-01-08 17:57           ` Eli Zaretskii
  0 siblings, 1 reply; 15+ messages in thread
From: Juri Linkov @ 2023-01-08 17:35 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: casouri, stefankangas, 60511

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





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#60511: 29.0.50; treesit-ready-p should not emit warning by default
  2023-01-08 17:35         ` Juri Linkov
@ 2023-01-08 17:57           ` Eli Zaretskii
  2023-01-08 18:11             ` Juri Linkov
  0 siblings, 1 reply; 15+ messages in thread
From: Eli Zaretskii @ 2023-01-08 17:57 UTC (permalink / raw)
  To: Juri Linkov; +Cc: casouri, stefankangas, 60511

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





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#60511: 29.0.50; treesit-ready-p should not emit warning by default
  2023-01-08 17:57           ` Eli Zaretskii
@ 2023-01-08 18:11             ` Juri Linkov
  2023-01-08 18:48               ` Eli Zaretskii
  0 siblings, 1 reply; 15+ messages in thread
From: Juri Linkov @ 2023-01-08 18:11 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: casouri, stefankangas, 60511

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





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#60511: 29.0.50; treesit-ready-p should not emit warning by default
  2023-01-08  5:53   ` Eli Zaretskii
  2023-01-08  8:39     ` Juri Linkov
@ 2023-01-08 18:14     ` Stefan Kangas
  2023-01-08 18:28       ` Juri Linkov
  1 sibling, 1 reply; 15+ messages in thread
From: Stefan Kangas @ 2023-01-08 18:14 UTC (permalink / raw)
  To: Eli Zaretskii, Yuan Fu; +Cc: 60511, juri

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.





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#60511: 29.0.50; treesit-ready-p should not emit warning by default
  2023-01-08 18:14     ` Stefan Kangas
@ 2023-01-08 18:28       ` Juri Linkov
  2023-01-08 18:49         ` Eli Zaretskii
  0 siblings, 1 reply; 15+ messages in thread
From: Juri Linkov @ 2023-01-08 18:28 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: Eli Zaretskii, Yuan Fu, 60511

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





^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#60511: 29.0.50; treesit-ready-p should not emit warning by default
  2023-01-08 18:11             ` Juri Linkov
@ 2023-01-08 18:48               ` Eli Zaretskii
  0 siblings, 0 replies; 15+ messages in thread
From: Eli Zaretskii @ 2023-01-08 18:48 UTC (permalink / raw)
  To: Juri Linkov; +Cc: casouri, stefankangas, 60511

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






^ permalink raw reply	[flat|nested] 15+ messages in thread

* bug#60511: 29.0.50; treesit-ready-p should not emit warning by default
  2023-01-08 18:28       ` Juri Linkov
@ 2023-01-08 18:49         ` Eli Zaretskii
  0 siblings, 0 replies; 15+ messages in thread
From: Eli Zaretskii @ 2023-01-08 18:49 UTC (permalink / raw)
  To: Juri Linkov; +Cc: casouri, stefankangas, 60511

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





^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2023-01-08 18:49 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-01-03 11:19 bug#60511: 29.0.50; treesit-ready-p should not emit warning by default Stefan Kangas
2023-01-03 17:44 ` Juri Linkov
2023-01-04  7:02 ` Yuan Fu
2023-01-04  7:47   ` Juri Linkov
2023-01-08  1:31 ` Yuan Fu
2023-01-08  5:53   ` Eli Zaretskii
2023-01-08  8:39     ` Juri Linkov
2023-01-08 11:03       ` Eli Zaretskii
2023-01-08 17:35         ` Juri Linkov
2023-01-08 17:57           ` Eli Zaretskii
2023-01-08 18:11             ` Juri Linkov
2023-01-08 18:48               ` Eli Zaretskii
2023-01-08 18:14     ` Stefan Kangas
2023-01-08 18:28       ` Juri Linkov
2023-01-08 18:49         ` Eli Zaretskii

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).