GNU bug report logs - #60623
30.0.50; Add forward-sentence with tree sitter support

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 60623 in the body.
You can then email your comments to 60623 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Theodor Thornhill <theo <at> thornhill.no>
To: bug-gnu-emacs <at> gnu.org
Subject: 30.0.50; Add forward-sentence with tree sitter support
Date: Sat, 07 Jan 2023 12:54:13 +0100

[Message part 1 (text/plain, inline)]

Hi all!

This patch tweaks the forward-sentence function to be usable with
tree-sitter.

It follows the same style as the recent change in transpose-sexps, so I
hope it isn't too controversial.

What exact node types do you consider useful for sentence movement?

I added an example value in java-ts-mode and c-ts-mode, but that is
merely a suggestion.  Let's decide on some heuristic to decide the
proper nodes to use.

What do you think?

Theo

[0001-Add-forward-sentence-with-tree-sitter-support.patch (text/x-patch, attachment)]

Message #8 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Daniel Martín <mardani29 <at> yahoo.es>
To: Theodor Thornhill via "Bug reports for GNU Emacs, the Swiss army knife
 of text editors" <bug-gnu-emacs <at> gnu.org>
Cc: 60623 <at> debbugs.gnu.org, Theodor Thornhill <theo <at> thornhill.no>,
 casouri <at> gmail.com, monnier <at> iro.umontreal.ca, eliz <at> gnu.org
Subject: Re: bug#60623: 30.0.50; Add forward-sentence with tree sitter support
Date: Sat, 07 Jan 2023 16:41:57 +0100

Theodor Thornhill via "Bug reports for GNU Emacs, the Swiss army knife
of text editors" <bug-gnu-emacs <at> gnu.org> writes:

> Hi all!
>
> This patch tweaks the forward-sentence function to be usable with
> tree-sitter.
>
> It follows the same style as the recent change in transpose-sexps, so I
> hope it isn't too controversial.

Thanks.

>
> What exact node types do you consider useful for sentence movement?
>

I haven't thought much about your proposed nodes, I initially thought
that sentences in a programming language are just "statements".

As a suggestion, treesit-forward-sentence could navigate by textual
sentences when point is inside comments or strings.

> +** New defvar-local forward-sentence-function.
> +The previous implementation of 'forward-sentence' is moved into this
> +variable, which can be set to customize the sentece movement behavior.
                                               ^^^^^^^
                                               sentence

Also, this feature probably needs an update to the Info documentation to
mention that Tree-sitter can specialize sentence commands in programming
modes.

Message #14 received at 60623 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Theodor Thornhill <theo <at> thornhill.no>
Cc: 60623 <at> debbugs.gnu.org, eliz <at> gnu.org, casouri <at> gmail.com,
 monnier <at> iro.umontreal.ca
Subject: Re: bug#60623: 30.0.50; Add forward-sentence with tree sitter support
Date: Sun, 08 Jan 2023 10:36:42 +0200

> -(defun forward-sentence (&optional arg)
> +(defvar forward-sentence-function
> +  (lambda (&optional arg)

A good practice is to name such a function e.g. forward-sentence-default-function.

Message #17 received at 60623 <at> debbugs.gnu.org (full text, mbox):

From: Theodor Thornhill <theo <at> thornhill.no>
To: Juri Linkov <juri <at> linkov.net>
Cc: 60623 <at> debbugs.gnu.org, eliz <at> gnu.org, casouri <at> gmail.com,
 monnier <at> iro.umontreal.ca
Subject: Re: bug#60623: 30.0.50; Add forward-sentence with tree sitter support
Date: Sun, 08 Jan 2023 10:20:47 +0100

On 8 January 2023 09:36:42 CET, Juri Linkov <juri <at> linkov.net> wrote:
>> -(defun forward-sentence (&optional arg)
>> +(defvar forward-sentence-function
>> +  (lambda (&optional arg)
>
>A good practice is to name such a function e.g. forward-sentence-default-function.

Is this practice used anywhere else? Iirc forward-sexp-function doesn't follow that practice.

Thanks,
Theo

Message #20 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Theodor Thornhill <theo <at> thornhill.no>
To: Daniel Martín <mardani29 <at> yahoo.es>, "Theodor Thornhill
 via Bug reports for GNU Emacs, the Swiss army knife
 of text editors" <bug-gnu-emacs <at> gnu.org>
Cc: 60623 <at> debbugs.gnu.org, Juri Linkov <juri <at> linkov.net>, eliz <at> gnu.org,
 casouri <at> gmail.com, monnier <at> iro.umontreal.ca
Subject: Re: bug#60623: 30.0.50; Add forward-sentence with tree sitter support
Date: Sun, 08 Jan 2023 14:29:08 +0100

[Message part 1 (text/plain, inline)]

Daniel Martín <mardani29 <at> yahoo.es> writes:

> Theodor Thornhill via "Bug reports for GNU Emacs, the Swiss army knife
> of text editors" <bug-gnu-emacs <at> gnu.org> writes:
>
>> Hi all!
>>
>> This patch tweaks the forward-sentence function to be usable with
>> tree-sitter.
>>
>> It follows the same style as the recent change in transpose-sexps, so I
>> hope it isn't too controversial.
>
> Thanks.
>
>>
>> What exact node types do you consider useful for sentence movement?
>>
>
> I haven't thought much about your proposed nodes, I initially thought
> that sentences in a programming language are just "statements".

They aren't really proper propsals.  Mostly some example values to show
that the code works.  The problem with just stating "statements" is that
the names are different across parsers. So in java one would call

```
void foo() {
  var foo = 5;  // <-- This thing
}

```

A "local_variable_declaration" or something like that.  But it would
make sense for M-e to move across that whole line.  So this is language
dependent, I believe.

>
> As a suggestion, treesit-forward-sentence could navigate by textual
> sentences when point is inside comments or strings.
>

Yeah, this is a good idea - added in following patch.

>> +** New defvar-local forward-sentence-function.
>> +The previous implementation of 'forward-sentence' is moved into this
>> +variable, which can be set to customize the sentece movement behavior.
>                                                ^^^^^^^
>                                                sentence
>

Thanks - fixed.

> Also, this feature probably needs an update to the Info documentation to
> mention that Tree-sitter can specialize sentence commands in programming
> modes.

Yes, likely.  I will add this a bit later, when we agree on its behavior
fully :)

@Eli, what doc changes do you see as needed here?

@Juri: I added a change with how I understood what you meant.  Is that
in your line of reasoning?


Theo

[0001-Add-forward-sentence-with-tree-sitter-support-bug-60.patch (text/x-patch, attachment)]

Message #26 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Theodor Thornhill <theo <at> thornhill.no>
Cc: casouri <at> gmail.com, bug-gnu-emacs <at> gnu.org, 60623 <at> debbugs.gnu.org,
 juri <at> linkov.net, monnier <at> iro.umontreal.ca, mardani29 <at> yahoo.es
Subject: Re: bug#60623: 30.0.50; Add forward-sentence with tree sitter support
Date: Sun, 08 Jan 2023 16:53:47 +0200

> From: Theodor Thornhill <theo <at> thornhill.no>
> Cc: 60623 <at> debbugs.gnu.org, casouri <at> gmail.com, eliz <at> gnu.org,
>  monnier <at> iro.umontreal.ca,Juri Linkov <juri <at> linkov.net>
> Date: Sun, 08 Jan 2023 14:29:08 +0100
> 
> @Eli, what doc changes do you see as needed here?

More or less.  They need some polishing, like a few words about what
does "sentence" mean in the tree-sitter context.  But we can make
these changes after this is in the repository.

Thanks.

Message #32 received at 60623 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Theodor Thornhill <theo <at> thornhill.no>, Juri Linkov <juri <at> linkov.net>
Cc: "60623 <at> debbugs.gnu.org" <60623 <at> debbugs.gnu.org>,
 "casouri <at> gmail.com" <casouri <at> gmail.com>, "eliz <at> gnu.org" <eliz <at> gnu.org>,
 "monnier <at> iro.umontreal.ca" <monnier <at> iro.umontreal.ca>
Subject: RE: [External] : bug#60623: 30.0.50; Add forward-sentence with tree
 sitter support
Date: Sun, 8 Jan 2023 16:41:05 +0000

> >A good practice is to name such a function
> >e.g. forward-sentence-default-function.
> 
> Is this practice used anywhere else? Iirc
> forward-sexp-function doesn't follow that
> practice.

Exactly.  IMO, if the variable can have a
nonfunction value, especially nil, then
there's no need (nothing gained, and even
possible confusion/misunderstanding added)
by adding "-default-" to the name.

On the other hand, if the value must always
be a function, then having "-default-" in
the name makes sense.

Following this guideline helps users know
(guess) what the variable's value can be
even without consulting its doc.

Message #35 received at 60623 <at> debbugs.gnu.org (full text, mbox):

From: Theodor Thornhill <theo <at> thornhill.no>
To: Drew Adams <drew.adams <at> oracle.com>, Juri Linkov <juri <at> linkov.net>
Cc: "60623 <at> debbugs.gnu.org" <60623 <at> debbugs.gnu.org>,
 "casouri <at> gmail.com" <casouri <at> gmail.com>, "eliz <at> gnu.org" <eliz <at> gnu.org>,
 "monnier <at> iro.umontreal.ca" <monnier <at> iro.umontreal.ca>
Subject: RE: [External] : bug#60623: 30.0.50; Add forward-sentence with tree sitter support
Date: Sun, 08 Jan 2023 18:04:12 +0100

On 8 January 2023 17:41:05 CET, Drew Adams <drew.adams <at> oracle.com> wrote:
>> >A good practice is to name such a function
>> >e.g. forward-sentence-default-function.
>> 
>> Is this practice used anywhere else? Iirc
>> forward-sexp-function doesn't follow that
>> practice.
>
>Exactly.  IMO, if the variable can have a
>nonfunction value, especially nil, then
>there's no need (nothing gained, and even
>possible confusion/misunderstanding added)
>by adding "-default-" to the name.
>
>On the other hand, if the value must always
>be a function, then having "-default-" in
>the name makes sense.
>
>Following this guideline helps users know
>(guess) what the variable's value can be
>even without consulting its doc.

So is this to be considered an improvement to forward-sexp too, then?

Message #38 received at 60623 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Theodor Thornhill <theo <at> thornhill.no>, Juri Linkov <juri <at> linkov.net>
Cc: "60623 <at> debbugs.gnu.org" <60623 <at> debbugs.gnu.org>,
 "casouri <at> gmail.com" <casouri <at> gmail.com>, "eliz <at> gnu.org" <eliz <at> gnu.org>,
 "monnier <at> iro.umontreal.ca" <monnier <at> iro.umontreal.ca>
Subject: RE: [External] : bug#60623: 30.0.50; Add forward-sentence with tree
 sitter support
Date: Sun, 8 Jan 2023 17:42:07 +0000

> So is this to be considered an improvement to forward-sexp too, then?

I don't understand the question.  What is "this"?

Variable `forward-sexp-function' already has an
appropriate name, IMO, since its value can be nil
(not a function).

Message #41 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Theodor Thornhill <theo <at> thornhill.no>
Cc: casouri <at> gmail.com, "Theodor Thornhill via Bug reports
 for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>,
 60623 <at> debbugs.gnu.org, Daniel Martín <mardani29 <at> yahoo.es>,
 monnier <at> iro.umontreal.ca, eliz <at> gnu.org
Subject: Re: bug#60623: 30.0.50; Add forward-sentence with tree sitter support
Date: Sun, 08 Jan 2023 19:33:28 +0200

> @Juri: I added a change with how I understood what you meant.  Is that
> in your line of reasoning?

Thanks, now this is more maintainable.

Message #44 received at 60623 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Theodor Thornhill <theo <at> thornhill.no>
Cc: "60623 <at> debbugs.gnu.org" <60623 <at> debbugs.gnu.org>,
 "casouri <at> gmail.com" <casouri <at> gmail.com>, "eliz <at> gnu.org" <eliz <at> gnu.org>,
 Drew Adams <drew.adams <at> oracle.com>,
 "monnier <at> iro.umontreal.ca" <monnier <at> iro.umontreal.ca>
Subject: Re: [External] : bug#60623: 30.0.50; Add forward-sentence with tree
 sitter support
Date: Sun, 08 Jan 2023 19:30:48 +0200

>>> >A good practice is to name such a function
>>> >e.g. forward-sentence-default-function.
>>>
>>> Is this practice used anywhere else? Iirc
>>> forward-sexp-function doesn't follow that
>>> practice.
>>
>>On the other hand, if the value must always
>>be a function, then having "-default-" in
>>the name makes sense.
>
> So is this to be considered an improvement to forward-sexp too, then?

Sorry, I can't find where a lambda is set to forward-sexp-function.
I only see this:

  (defvar forward-sexp-function nil

But if it will be set to a function later, it would be nice
to define a default function as well.

Message #50 received at 60623 <at> debbugs.gnu.org (full text, mbox):

From: Theodor Thornhill <theo <at> thornhill.no>
To: Juri Linkov <juri <at> linkov.net>
Cc: "60623 <at> debbugs.gnu.org" <60623 <at> debbugs.gnu.org>,
 "casouri <at> gmail.com" <casouri <at> gmail.com>, "eliz <at> gnu.org" <eliz <at> gnu.org>,
 Drew Adams <drew.adams <at> oracle.com>,
 "monnier <at> iro.umontreal.ca" <monnier <at> iro.umontreal.ca>
Subject: Re: [External] : bug#60623: 30.0.50; Add forward-sentence with tree
 sitter support
Date: Sun, 08 Jan 2023 20:19:58 +0100

Juri Linkov <juri <at> linkov.net> writes:

>>>> >A good practice is to name such a function
>>>> >e.g. forward-sentence-default-function.
>>>>
>>>> Is this practice used anywhere else? Iirc
>>>> forward-sexp-function doesn't follow that
>>>> practice.
>>>
>>>On the other hand, if the value must always
>>>be a function, then having "-default-" in
>>>the name makes sense.
>>
>> So is this to be considered an improvement to forward-sexp too, then?
>
> Sorry, I can't find where a lambda is set to forward-sexp-function.
> I only see this:
>
>   (defvar forward-sexp-function nil
>
> But if it will be set to a function later, it would be nice
> to define a default function as well.

I meant the way we did with 'transpose-sexps', where there now is a
'transpose-sexps-function' variable containing the factored-out earlier
implementation.  And by if "this is an improvement" I meant declaring a
specific defun as the default value for the defvar in question.  Maybe I
should add the same change which is now developing here there too.

What do you think?

Theo

Message #53 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Theodor Thornhill <theo <at> thornhill.no>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: casouri <at> gmail.com, bug-gnu-emacs <at> gnu.org, 60623 <at> debbugs.gnu.org,
 juri <at> linkov.net, monnier <at> iro.umontreal.ca, mardani29 <at> yahoo.es
Subject: Re: bug#60623: 30.0.50; Add forward-sentence with tree sitter support
Date: Sun, 08 Jan 2023 20:35:58 +0100

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Theodor Thornhill <theo <at> thornhill.no>
>> Cc: 60623 <at> debbugs.gnu.org, casouri <at> gmail.com, eliz <at> gnu.org,
>>  monnier <at> iro.umontreal.ca,Juri Linkov <juri <at> linkov.net>
>> Date: Sun, 08 Jan 2023 14:29:08 +0100
>> 
>> @Eli, what doc changes do you see as needed here?
>
> More or less.  They need some polishing, like a few words about what
> does "sentence" mean in the tree-sitter context.  But we can make
> these changes after this is in the repository.
>
> Thanks.


Ok, so in other words, this patch is good to go?

I omitted the additions to java-ts-mode and c-ts-mode.  I can make a
separate commit to add some values that makes sense for multiple modes
after?  Will the changes to the manual lie in "26.2 Sentences"? in the
Emacs manual?

Theo

[0001-Add-forward-sentence-with-tree-sitter-support-bug-60.patch (text/x-patch, attachment)]

Message #59 received at 60623 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Theodor Thornhill <theo <at> thornhill.no>
Cc: 60623 <at> debbugs.gnu.org, juri <at> linkov.net, casouri <at> gmail.com,
 monnier <at> iro.umontreal.ca, mardani29 <at> yahoo.es
Subject: Re: bug#60623: 30.0.50; Add forward-sentence with tree sitter support
Date: Sun, 08 Jan 2023 21:57:51 +0200

> From: Theodor Thornhill <theo <at> thornhill.no>
> Cc: mardani29 <at> yahoo.es, bug-gnu-emacs <at> gnu.org, 60623 <at> debbugs.gnu.org,
>  casouri <at> gmail.com, monnier <at> iro.umontreal.ca, juri <at> linkov.net
> Date: Sun, 08 Jan 2023 20:35:58 +0100
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> >> From: Theodor Thornhill <theo <at> thornhill.no>
> >> Cc: 60623 <at> debbugs.gnu.org, casouri <at> gmail.com, eliz <at> gnu.org,
> >>  monnier <at> iro.umontreal.ca,Juri Linkov <juri <at> linkov.net>
> >> Date: Sun, 08 Jan 2023 14:29:08 +0100
> >> 
> >> @Eli, what doc changes do you see as needed here?
> >
> > More or less.  They need some polishing, like a few words about what
> > does "sentence" mean in the tree-sitter context.  But we can make
> > these changes after this is in the repository.
> >
> > Thanks.
> 
> 
> Ok, so in other words, this patch is good to go?

Yes, I think so.

> I omitted the additions to java-ts-mode and c-ts-mode.  I can make a
> separate commit to add some values that makes sense for multiple modes
> after?

SGTM.

> Will the changes to the manual lie in "26.2 Sentences"? in the Emacs
> manual?

No, because these are not really sentences in some human-readable
language, these are program parts.  As such they should be somewhere
under "27 Programs", possibly in "Defuns".

However, "Sentences" might mention that programming modes have their
own interpretation of "sentence" and corresponding movement commands.

Message #62 received at 60623 <at> debbugs.gnu.org (full text, mbox):

From: Theodor Thornhill <theo <at> thornhill.no>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 60623 <at> debbugs.gnu.org, juri <at> linkov.net, casouri <at> gmail.com,
 monnier <at> iro.umontreal.ca, mardani29 <at> yahoo.es
Subject: Re: bug#60623: 30.0.50; Add forward-sentence with tree sitter support
Date: Sun, 08 Jan 2023 21:07:21 +0100

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Theodor Thornhill <theo <at> thornhill.no>
>> Cc: mardani29 <at> yahoo.es, bug-gnu-emacs <at> gnu.org, 60623 <at> debbugs.gnu.org,
>>  casouri <at> gmail.com, monnier <at> iro.umontreal.ca, juri <at> linkov.net
>> Date: Sun, 08 Jan 2023 20:35:58 +0100
>> 
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>> 
>> >> From: Theodor Thornhill <theo <at> thornhill.no>
>> >> Cc: 60623 <at> debbugs.gnu.org, casouri <at> gmail.com, eliz <at> gnu.org,
>> >>  monnier <at> iro.umontreal.ca,Juri Linkov <juri <at> linkov.net>
>> >> Date: Sun, 08 Jan 2023 14:29:08 +0100
>> >> 
>> >> @Eli, what doc changes do you see as needed here?
>> >
>> > More or less.  They need some polishing, like a few words about what
>> > does "sentence" mean in the tree-sitter context.  But we can make
>> > these changes after this is in the repository.
>> >
>> > Thanks.
>> 
>> 
>> Ok, so in other words, this patch is good to go?
>
> Yes, I think so.
>

Great!

>> I omitted the additions to java-ts-mode and c-ts-mode.  I can make a
>> separate commit to add some values that makes sense for multiple modes
>> after?
>
> SGTM.
>

Nice.  Will you install this for me?

>> Will the changes to the manual lie in "26.2 Sentences"? in the Emacs
>> manual?
>
> No, because these are not really sentences in some human-readable
> language, these are program parts.  As such they should be somewhere
> under "27 Programs", possibly in "Defuns".
>
> However, "Sentences" might mention that programming modes have their
> own interpretation of "sentence" and corresponding movement commands.

Yeah, that makes sense.  Should I make an attempt at such formulations,
or will you do it at a later time?

Theo

Message #65 received at 60623 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: "casouri <at> gmail.com" <casouri <at> gmail.com>,
 "60623 <at> debbugs.gnu.org" <60623 <at> debbugs.gnu.org>,
 Theodor Thornhill <theo <at> thornhill.no>, "eliz <at> gnu.org" <eliz <at> gnu.org>,
 Juri Linkov <juri <at> linkov.net>
Subject: Re: [External] : bug#60623: 30.0.50; Add forward-sentence with tree
 sitter support
Date: Mon, 09 Jan 2023 01:20:50 -0500

> Exactly.  IMO, if the variable can have a
> nonfunction value, especially nil, then

Bad idea.  That precludes changing the value of the variable with
`add-function`, whereas `add-function` is often the best way for
a major/minor mode to change that variable (actually, the "only" way to
do it with some hope that it will interact correctly with other modes
that may change it as well).

> there's no need (nothing gained, and even possible
> confusion/misunderstanding added) by adding "-default-" to the name.

Instead we should strive to make the name of the default function simply
irrelevant.  If all goes well, nobody should ever need to explicitly
call "the default value" of that variable.  Instead the function they
added via `add-function` will receive the "previous" value as argument.

> On the other hand, if the value must always be a function, then having
> "-default-" in the name makes sense.

Agreed.  It's also helpful when you look at the var's value, it lets you
know that it hasn't been modified.


        Stefan

Message #68 received at 60623 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Theodor Thornhill <theo <at> thornhill.no>
Cc: "60623 <at> debbugs.gnu.org" <60623 <at> debbugs.gnu.org>,
 "casouri <at> gmail.com" <casouri <at> gmail.com>, "eliz <at> gnu.org" <eliz <at> gnu.org>,
 Drew Adams <drew.adams <at> oracle.com>,
 "monnier <at> iro.umontreal.ca" <monnier <at> iro.umontreal.ca>
Subject: Re: [External] : bug#60623: 30.0.50; Add forward-sentence with tree
 sitter support
Date: Mon, 09 Jan 2023 09:49:55 +0200

>>> So is this to be considered an improvement to forward-sexp too, then?
>>
>> Sorry, I can't find where a lambda is set to forward-sexp-function.
>> I only see this:
>>
>>   (defvar forward-sexp-function nil
>>
>> But if it will be set to a function later, it would be nice
>> to define a default function as well.
>
> I meant the way we did with 'transpose-sexps', where there now is a
> 'transpose-sexps-function' variable containing the factored-out earlier
> implementation.  And by if "this is an improvement" I meant declaring a
> specific defun as the default value for the defvar in question.  Maybe I
> should add the same change which is now developing here there too.
>
> What do you think?

I agree that 'transpose-sexps-function' could benefit from the same improvement.

Message #71 received at 60623 <at> debbugs.gnu.org (full text, mbox):

From: Theodor Thornhill <theo <at> thornhill.no>
To: Juri Linkov <juri <at> linkov.net>
Cc: "60623 <at> debbugs.gnu.org" <60623 <at> debbugs.gnu.org>,
 "casouri <at> gmail.com" <casouri <at> gmail.com>, "eliz <at> gnu.org" <eliz <at> gnu.org>,
 Drew Adams <drew.adams <at> oracle.com>,
 "monnier <at> iro.umontreal.ca" <monnier <at> iro.umontreal.ca>
Subject: Re: [External] : bug#60623: 30.0.50; Add forward-sentence with tree
 sitter support
Date: Mon, 09 Jan 2023 09:01:23 +0100

Juri Linkov <juri <at> linkov.net> writes:

>>>> So is this to be considered an improvement to forward-sexp too, then?
>>>
>>> Sorry, I can't find where a lambda is set to forward-sexp-function.
>>> I only see this:
>>>
>>>   (defvar forward-sexp-function nil
>>>
>>> But if it will be set to a function later, it would be nice
>>> to define a default function as well.
>>
>> I meant the way we did with 'transpose-sexps', where there now is a
>> 'transpose-sexps-function' variable containing the factored-out earlier
>> implementation.  And by if "this is an improvement" I meant declaring a
>> specific defun as the default value for the defvar in question.  Maybe I
>> should add the same change which is now developing here there too.
>>
>> What do you think?
>
> I agree that 'transpose-sexps-function' could benefit from the same improvement.

Yeah, added such a change in bug#60654, as it was already reported :-)

I appreciate your feedback!

Theo

Message #74 received at 60623 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Theodor Thornhill <theo <at> thornhill.no>
Cc: 60623 <at> debbugs.gnu.org, juri <at> linkov.net, casouri <at> gmail.com,
 monnier <at> iro.umontreal.ca, mardani29 <at> yahoo.es
Subject: Re: bug#60623: 30.0.50; Add forward-sentence with tree sitter support
Date: Mon, 09 Jan 2023 14:37:54 +0200

> From: Theodor Thornhill <theo <at> thornhill.no>
> Cc: mardani29 <at> yahoo.es, 60623 <at> debbugs.gnu.org, casouri <at> gmail.com,
>  monnier <at> iro.umontreal.ca, juri <at> linkov.net
> Date: Sun, 08 Jan 2023 21:07:21 +0100
> 
> >> Ok, so in other words, this patch is good to go?
> >
> > Yes, I think so.
> >
> 
> Great!
> 
> >> I omitted the additions to java-ts-mode and c-ts-mode.  I can make a
> >> separate commit to add some values that makes sense for multiple modes
> >> after?
> >
> > SGTM.
> >
> 
> Nice.  Will you install this for me?

I'm under the impression that this is still being discussed?

> >> Will the changes to the manual lie in "26.2 Sentences"? in the Emacs
> >> manual?
> >
> > No, because these are not really sentences in some human-readable
> > language, these are program parts.  As such they should be somewhere
> > under "27 Programs", possibly in "Defuns".
> >
> > However, "Sentences" might mention that programming modes have their
> > own interpretation of "sentence" and corresponding movement commands.
> 
> Yeah, that makes sense.  Should I make an attempt at such formulations,
> or will you do it at a later time?

It is better that you try, if only to gain experience ;-)

Message #77 received at 60623 <at> debbugs.gnu.org (full text, mbox):

From: Theodor Thornhill <theo <at> thornhill.no>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 60623 <at> debbugs.gnu.org, juri <at> linkov.net, casouri <at> gmail.com,
 monnier <at> iro.umontreal.ca, mardani29 <at> yahoo.es
Subject: Re: bug#60623: 30.0.50; Add forward-sentence with tree sitter support
Date: Mon, 09 Jan 2023 14:28:23 +0100

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Theodor Thornhill <theo <at> thornhill.no>
>> Cc: mardani29 <at> yahoo.es, 60623 <at> debbugs.gnu.org, casouri <at> gmail.com,
>>  monnier <at> iro.umontreal.ca, juri <at> linkov.net
>> Date: Sun, 08 Jan 2023 21:07:21 +0100
>> 
>> >> Ok, so in other words, this patch is good to go?
>> >
>> > Yes, I think so.
>> >
>> 
>> Great!
>> 
>> >> I omitted the additions to java-ts-mode and c-ts-mode.  I can make a
>> >> separate commit to add some values that makes sense for multiple modes
>> >> after?
>> >
>> > SGTM.
>> >
>> 
>> Nice.  Will you install this for me?
>
> I'm under the impression that this is still being discussed?
>

Hmm - I thought I'd addressed all comments.  I believe we discussed the
transpose-sexps-function equivalent change, but sure - no hurry :)

>> >> Will the changes to the manual lie in "26.2 Sentences"? in the Emacs
>> >> manual?
>> >
>> > No, because these are not really sentences in some human-readable
>> > language, these are program parts.  As such they should be somewhere
>> > under "27 Programs", possibly in "Defuns".
>> >
>> > However, "Sentences" might mention that programming modes have their
>> > own interpretation of "sentence" and corresponding movement commands.
>> 
>> Yeah, that makes sense.  Should I make an attempt at such formulations,
>> or will you do it at a later time?
>
> It is better that you try, if only to gain experience ;-)

Will do!

Thanks,
Theo

Message #80 received at 60623 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: "casouri <at> gmail.com" <casouri <at> gmail.com>,
 "60623 <at> debbugs.gnu.org" <60623 <at> debbugs.gnu.org>,
 Theodor Thornhill <theo <at> thornhill.no>, "eliz <at> gnu.org" <eliz <at> gnu.org>,
 Juri Linkov <juri <at> linkov.net>
Subject: RE: [External] : bug#60623: 30.0.50; Add forward-sentence with tree
 sitter support
Date: Mon, 9 Jan 2023 15:57:16 +0000

> > Exactly.  IMO, if the variable can have
                   ^^
> > a nonfunction value, especially nil, then
> 
> Bad idea.

I know you think so. ;-)

> That precludes changing the value of the
> variable with `add-function`,

Yes, it does, at least blindly and ignoring
its current value.  And that's _appropriate_
IF the var can have a non-function value.

But even in that case the var value can be
tested to see if it's a function, and if/when
so, advising it can make sense.

> whereas `add-function` is often the best way for
> a major/minor mode to change that variable (actually, the "only" way to
> do it with some hope that it will interact correctly with other modes
> that may change it as well).

A legitimate argument.  But it doesn't apply to
a variable already defined so that it "can have
a nonfunction value."  Did you perhaps miss that
"if"?

I also don't agree that that (_good_) reason you
give is all-deciding.

I'd say that _other things being equal_, yes,
you can take advantage of that good reason, IF
the variable's value can be ensured to always be
a function, or sometimes even if it just is
currently a function.

IOW, you give one (good) reason for one (good)
practice, which, yes, can sometimes make sense.

> > there's no need (nothing gained, and even
> > possible confusion/misunderstanding added) by
> > adding "-default-" to the name.
> >
> > On the other hand, if the value must always
> > be a function, then having "-default-" in the
> > name makes sense.
> 
> Agreed.  It's also helpful when you look at the var's
> value, it lets you know that it hasn't been modified.

Message #83 received at 60623 <at> debbugs.gnu.org (full text, mbox):

From: Theodor Thornhill <theo <at> thornhill.no>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 60623 <at> debbugs.gnu.org, juri <at> linkov.net, casouri <at> gmail.com,
 monnier <at> iro.umontreal.ca, mardani29 <at> yahoo.es
Subject: Re: bug#60623: 30.0.50; Add forward-sentence with tree sitter support
Date: Tue, 10 Jan 2023 09:37:26 +0100

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Theodor Thornhill <theo <at> thornhill.no>
>> Cc: mardani29 <at> yahoo.es, 60623 <at> debbugs.gnu.org, casouri <at> gmail.com,
>>  monnier <at> iro.umontreal.ca, juri <at> linkov.net
>> Date: Sun, 08 Jan 2023 21:07:21 +0100
>> 
>> >> Ok, so in other words, this patch is good to go?
>> >
>> > Yes, I think so.
>> >
>> 
>> Great!
>> 
>> >> I omitted the additions to java-ts-mode and c-ts-mode.  I can make a
>> >> separate commit to add some values that makes sense for multiple modes
>> >> after?
>> >
>> > SGTM.
>> >
>> 
>> Nice.  Will you install this for me?
>
> I'm under the impression that this is still being discussed?
>
>> >> Will the changes to the manual lie in "26.2 Sentences"? in the Emacs
>> >> manual?
>> >
>> > No, because these are not really sentences in some human-readable
>> > language, these are program parts.  As such they should be somewhere
>> > under "27 Programs", possibly in "Defuns".
>> >
>> > However, "Sentences" might mention that programming modes have their
>> > own interpretation of "sentence" and corresponding movement commands.
>> 
>> Yeah, that makes sense.  Should I make an attempt at such formulations,
>> or will you do it at a later time?
>
> It is better that you try, if only to gain experience ;-)

How about this for starter?

Theo

[0001-Add-forward-sentence-with-tree-sitter-support-bug-60.patch (text/x-patch, attachment)]

Message #86 received at 60623 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Theodor Thornhill <theo <at> thornhill.no>
Cc: 60623 <at> debbugs.gnu.org, juri <at> linkov.net, casouri <at> gmail.com,
 monnier <at> iro.umontreal.ca, mardani29 <at> yahoo.es
Subject: Re: bug#60623: 30.0.50; Add forward-sentence with tree sitter support
Date: Tue, 10 Jan 2023 17:07:14 +0200

> From: Theodor Thornhill <theo <at> thornhill.no>
> Cc: mardani29 <at> yahoo.es, 60623 <at> debbugs.gnu.org, casouri <at> gmail.com,
>  monnier <at> iro.umontreal.ca, juri <at> linkov.net
> Date: Tue, 10 Jan 2023 09:37:26 +0100
> 
> >> > No, because these are not really sentences in some human-readable
> >> > language, these are program parts.  As such they should be somewhere
> >> > under "27 Programs", possibly in "Defuns".
> >> >
> >> > However, "Sentences" might mention that programming modes have their
> >> > own interpretation of "sentence" and corresponding movement commands.
> >> 
> >> Yeah, that makes sense.  Should I make an attempt at such formulations,
> >> or will you do it at a later time?
> >
> > It is better that you try, if only to gain experience ;-)
> 
> How about this for starter?

Very good, thank you very much.  A few comments below.

> --- a/doc/emacs/programs.texi
> +++ b/doc/emacs/programs.texi
> @@ -163,6 +163,7 @@ Defuns
>  * Left Margin Paren::   An open-paren or similar opening delimiter
>                            starts a defun if it is at the left margin.
>  * Moving by Defuns::    Commands to move over or mark a major definition.
> +* Moving by Sentences:: Commands to move over certain definitions in code.
                                                         ^^^^^^^^^^^
I'd use "code units" or "units of code" here.

Also, should we perhaps name the section "Moving by Statements"? or
would it be too inaccurate?

> +  These commands move point or set up the region based on definitions,
> +also called @dfn{sentences}.  Even though sentences is usually

Each @dfn in a manual should have an index entry, so that readers
could easily find it.  in this case, the index entry should qualify
the "sentences" term by the fact that we are talking about units of
code.  So:

  @cindex sentences, in programming languages

> +considered when writing human languages, Emacs can use the same
> +commands to move over certain constructs in programming languages
> +(@pxref{Sentences}, @pxref{Moving by Defuns}).  In a programming
> +language a sentence is usually a complete language construct smaller
> +than defuns, but larger than sexps (@pxref{List Motion,,, elisp, The
> +Emacs Lisp Reference Manual}).

A couple of examples from two different languages could be a great
help here.  Otherwise this text sounds a bit too abstract.

> +@kindex M-a
> +@kindex M-e

Since we already have M-e elsewhere in the manual, I suggest to
qualify the key bindings here:

  @kindex M-a @r{(programming modes)}

and similarly for M-e.  The @r{..} thingy is necessary to reset to the
default typeface, since key index is implicitly typeset in @code.

> +@findex backward-sentence
> +@findex forward-sentence

Likewise with these two @findex entries: qualify them, since we have
the same commands documented elsewhere under "Sentences".

> --- a/doc/emacs/text.texi
> +++ b/doc/emacs/text.texi
> @@ -253,6 +253,14 @@ Sentences
>  of a sentence.  Set the variable @code{sentence-end-without-period} to
>  @code{t} in such cases.
>  
> +  Even though the above mentioned sentence movement commands are based
> +on human languages, other Emacs modes can set these command to get
> +similar functionality.  What exactly a sentence is in a non-human
> +language is dependent on the target language, but usually it is
> +complete statements, such as a variable definition and initialization,
> +or a conditional statement (@pxref{Moving by Sentences,,, emacs, The
> +extensible self-documenting text editor}).

The last sentence should be in "Moving by Sentences", since it
describes the commands documented there.  Also, please add a
cross-reference here to "Moving by Sentences", since you mention that
in the text (and rightfully so).

> +@defvar treesit-sentence-type-regexp
> +The value of this variable is a regexp matching the node type of sentence
> +nodes.  (For ``node'' and ``node type'', @pxref{Parsing Program Source}.)
> +
> +@findex treesit-forward-sentence
> +@findex forward-sentence
> +@findex backward-sentence
> +If Emacs is compiled with tree-sitter, it can use the tree-sitter
> +parser information to move across syntax constructs.  Since what
> +exactly is considered a sentence varies between languages, a major mode
> +should set @code{treesit-sentence-type-regexp} to determine that.  Then
> +the mode can get navigation-by-sentence functionality for free, by using
> +@code{forward-sentence} and @code{backward-sentence}.

Here please also add a cross-reference to the "Moving by Sentences"
node in the Emacs manual, so that people could understand what kind of
"sentences" this is talking about.

> +** New defvar forward-sentence-function.
      ^^^^^^^^^^
"New variable"

> +Emacs now can set this variable to customize the behavior of the
> +'forward-sentence' function.

Not "Emacs", but "major modes".

> +** New defun forward-sentence-default-function.
      ^^^^^^^^^
"New function"

> +The previous implementation of 'forward-sentence' is moved into its
> +own function, to be bound by 'forward-sentence-function'.
> +
> +** New defvar-local 'treesit-sentence-type-regexp.
> +Similarly to 'treesit-defun-type-regexp', this variable is used to
> +navigate sentences in Tree-sitter enabled modes.
> +
> +** New function 'treesit-forward-sentence'.
> +treesit.el now conditionally sets 'forward-sentence-function' for all
> +Tree-sitter modes that sets 'treesit-sentence-type-regexp'.

Please make these related items sub-headings of a common heading,
something like "Commands and variables to move by program statements".

> +
>  
>  * Changes in Specialized Modes and Packages in Emacs 30.1
>  ---
> diff --git a/lisp/textmodes/paragraphs.el b/lisp/textmodes/paragraphs.el
> index 73abb155aaa..fd2d83eeebf 100644
> --- a/lisp/textmodes/paragraphs.el
> +++ b/lisp/textmodes/paragraphs.el
> @@ -441,13 +441,12 @@ end-of-paragraph-text
>  	  (if (< (point) (point-max))
>  	      (end-of-paragraph-text))))))
>  
> -(defun forward-sentence (&optional arg)
> +(defun forward-sentence-default-function (&optional arg)
>    "Move forward to next end of sentence.  With argument, repeat.
>  When ARG is negative, move backward repeatedly to start of sentence.
>  
>  The variable `sentence-end' is a regular expression that matches ends of
>  sentences.  Also, every paragraph boundary terminates sentences as well."
> -  (interactive "^p")
>    (or arg (setq arg 1))
>    (let ((opoint (point))
>          (sentence-end (sentence-end)))
> @@ -480,6 +479,18 @@ forward-sentence
>      (let ((npoint (constrain-to-field nil opoint t)))
>        (not (= npoint opoint)))))
>  
> +(defvar forward-sentence-function #'forward-sentence-default-function
> +  "Function to be used to calculate sentence movements.
> +See `forward-sentence' for a description of its behavior.")
> +
> +(defun forward-sentence (&optional arg)
> +  "Move forward to next end of sentence.  With argument, repeat.
                                             ^^^^^^^^^^^^^^^^^^^^^
"With argument ARG, repeat."  The doc string should reference the
arguments where possible.

> +When ARG is negative, move backward repeatedly to start of sentence.
   ^^^^
"If", not "When".

> +(defvar-local treesit-sentence-type-regexp ""
> +  "A regexp that matches the node type of sentence nodes.

Why is the default an empty regexp? wouldn't nil be better?

Message #89 received at 60623 <at> debbugs.gnu.org (full text, mbox):

From: Theodor Thornhill <theo <at> thornhill.no>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 60623 <at> debbugs.gnu.org, juri <at> linkov.net, casouri <at> gmail.com,
 monnier <at> iro.umontreal.ca, mardani29 <at> yahoo.es
Subject: Re: bug#60623: 30.0.50; Add forward-sentence with tree sitter support
Date: Tue, 10 Jan 2023 20:33:52 +0100

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Theodor Thornhill <theo <at> thornhill.no>
>> Cc: mardani29 <at> yahoo.es, 60623 <at> debbugs.gnu.org, casouri <at> gmail.com,
>>  monnier <at> iro.umontreal.ca, juri <at> linkov.net
>> Date: Tue, 10 Jan 2023 09:37:26 +0100
>> 
>> >> > No, because these are not really sentences in some human-readable
>> >> > language, these are program parts.  As such they should be somewhere
>> >> > under "27 Programs", possibly in "Defuns".
>> >> >
>> >> > However, "Sentences" might mention that programming modes have their
>> >> > own interpretation of "sentence" and corresponding movement commands.
>> >> 
>> >> Yeah, that makes sense.  Should I make an attempt at such formulations,
>> >> or will you do it at a later time?
>> >
>> > It is better that you try, if only to gain experience ;-)
>> 
>> How about this for starter?
>
> Very good, thank you very much.  A few comments below.
>
>> --- a/doc/emacs/programs.texi
>> +++ b/doc/emacs/programs.texi
>> @@ -163,6 +163,7 @@ Defuns
>>  * Left Margin Paren::   An open-paren or similar opening delimiter
>>                            starts a defun if it is at the left margin.
>>  * Moving by Defuns::    Commands to move over or mark a major definition.
>> +* Moving by Sentences:: Commands to move over certain definitions in code.
>                                                          ^^^^^^^^^^^
> I'd use "code units" or "units of code" here.

Done.

>
> Also, should we perhaps name the section "Moving by Statements"? or
> would it be too inaccurate?
>

I'm not sure.  I think that maybe because the commands involved, and the
ones that implicitly will be impacted, such as kill-sentence and friends
it is best to stay with Sentences?  But a statement is the better term
wrt programming languages of course.  I hold no strong opinions here.

>> +  These commands move point or set up the region based on definitions,
>> +also called @dfn{sentences}.  Even though sentences is usually
>
> Each @dfn in a manual should have an index entry, so that readers
> could easily find it.  in this case, the index entry should qualify
> the "sentences" term by the fact that we are talking about units of
> code.  So:
>
>   @cindex sentences, in programming languages
>

Done.

>> +considered when writing human languages, Emacs can use the same
>> +commands to move over certain constructs in programming languages
>> +(@pxref{Sentences}, @pxref{Moving by Defuns}).  In a programming
>> +language a sentence is usually a complete language construct smaller
>> +than defuns, but larger than sexps (@pxref{List Motion,,, elisp, The
>> +Emacs Lisp Reference Manual}).
>
> A couple of examples from two different languages could be a great
> help here.  Otherwise this text sounds a bit too abstract.
>

Something like this?

>> +@kindex M-a
>> +@kindex M-e
>
> Since we already have M-e elsewhere in the manual, I suggest to
> qualify the key bindings here:
>
>   @kindex M-a @r{(programming modes)}
>
> and similarly for M-e.  The @r{..} thingy is necessary to reset to the
> default typeface, since key index is implicitly typeset in @code.
>
>> +@findex backward-sentence
>> +@findex forward-sentence
>
> Likewise with these two @findex entries: qualify them, since we have
> the same commands documented elsewhere under "Sentences".
>

Done.

>> --- a/doc/emacs/text.texi
>> +++ b/doc/emacs/text.texi
>> @@ -253,6 +253,14 @@ Sentences
>>  of a sentence.  Set the variable @code{sentence-end-without-period} to
>>  @code{t} in such cases.
>>  
>> +  Even though the above mentioned sentence movement commands are based
>> +on human languages, other Emacs modes can set these command to get
>> +similar functionality.  What exactly a sentence is in a non-human
>> +language is dependent on the target language, but usually it is
>> +complete statements, such as a variable definition and initialization,
>> +or a conditional statement (@pxref{Moving by Sentences,,, emacs, The
>> +extensible self-documenting text editor}).
>
> The last sentence should be in "Moving by Sentences", since it
> describes the commands documented there.  Also, please add a
> cross-reference here to "Moving by Sentences", since you mention that
> in the text (and rightfully so).
>

Is something like this what you meant?

>> +@defvar treesit-sentence-type-regexp
>> +The value of this variable is a regexp matching the node type of sentence
>> +nodes.  (For ``node'' and ``node type'', @pxref{Parsing Program Source}.)
>> +
>> +@findex treesit-forward-sentence
>> +@findex forward-sentence
>> +@findex backward-sentence
>> +If Emacs is compiled with tree-sitter, it can use the tree-sitter
>> +parser information to move across syntax constructs.  Since what
>> +exactly is considered a sentence varies between languages, a major mode
>> +should set @code{treesit-sentence-type-regexp} to determine that.  Then
>> +the mode can get navigation-by-sentence functionality for free, by using
>> +@code{forward-sentence} and @code{backward-sentence}.
>
> Here please also add a cross-reference to the "Moving by Sentences"
> node in the Emacs manual, so that people could understand what kind of
> "sentences" this is talking about.
>
>> +** New defvar forward-sentence-function.
>       ^^^^^^^^^^
> "New variable"
>
>> +Emacs now can set this variable to customize the behavior of the
>> +'forward-sentence' function.
>
> Not "Emacs", but "major modes".
>
>> +** New defun forward-sentence-default-function.
>       ^^^^^^^^^
> "New function"
>
>> +The previous implementation of 'forward-sentence' is moved into its
>> +own function, to be bound by 'forward-sentence-function'.
>> +
>> +** New defvar-local 'treesit-sentence-type-regexp.
>> +Similarly to 'treesit-defun-type-regexp', this variable is used to
>> +navigate sentences in Tree-sitter enabled modes.
>> +
>> +** New function 'treesit-forward-sentence'.
>> +treesit.el now conditionally sets 'forward-sentence-function' for all
>> +Tree-sitter modes that sets 'treesit-sentence-type-regexp'.
>
> Please make these related items sub-headings of a common heading,
> something like "Commands and variables to move by program statements".
>

Done.

>> +
>>  
>>  * Changes in Specialized Modes and Packages in Emacs 30.1
>>  ---
>> diff --git a/lisp/textmodes/paragraphs.el b/lisp/textmodes/paragraphs.el
>> index 73abb155aaa..fd2d83eeebf 100644
>> --- a/lisp/textmodes/paragraphs.el
>> +++ b/lisp/textmodes/paragraphs.el
>> @@ -441,13 +441,12 @@ end-of-paragraph-text
>>  	  (if (< (point) (point-max))
>>  	      (end-of-paragraph-text))))))
>>  
>> -(defun forward-sentence (&optional arg)
>> +(defun forward-sentence-default-function (&optional arg)
>>    "Move forward to next end of sentence.  With argument, repeat.
>>  When ARG is negative, move backward repeatedly to start of sentence.
>>  
>>  The variable `sentence-end' is a regular expression that matches ends of
>>  sentences.  Also, every paragraph boundary terminates sentences as well."
>> -  (interactive "^p")
>>    (or arg (setq arg 1))
>>    (let ((opoint (point))
>>          (sentence-end (sentence-end)))
>> @@ -480,6 +479,18 @@ forward-sentence
>>      (let ((npoint (constrain-to-field nil opoint t)))
>>        (not (= npoint opoint)))))
>>  
>> +(defvar forward-sentence-function #'forward-sentence-default-function
>> +  "Function to be used to calculate sentence movements.
>> +See `forward-sentence' for a description of its behavior.")
>> +
>> +(defun forward-sentence (&optional arg)
>> +  "Move forward to next end of sentence.  With argument, repeat.
>                                              ^^^^^^^^^^^^^^^^^^^^^
> "With argument ARG, repeat."  The doc string should reference the
> arguments where possible.
>

Thanks, done.

>> +When ARG is negative, move backward repeatedly to start of sentence.
>    ^^^^
> "If", not "When".
>

Done

>> +(defvar-local treesit-sentence-type-regexp ""
>> +  "A regexp that matches the node type of sentence nodes.
>
> Why is the default an empty regexp? wouldn't nil be better?

Indeed it will, done.

How about this?

Theo

[0001-Add-forward-sentence-with-tree-sitter-support-bug-60.patch (text/x-patch, attachment)]

Message #92 received at 60623 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Theodor Thornhill <theo <at> thornhill.no>
Cc: Eli Zaretskii <eliz <at> gnu.org>, juri <at> linkov.net, casouri <at> gmail.com,
 60623 <at> debbugs.gnu.org, mardani29 <at> yahoo.es
Subject: Re: bug#60623: 30.0.50; Add forward-sentence with tree sitter support
Date: Tue, 10 Jan 2023 15:03:07 -0500

>>> +* Moving by Sentences:: Commands to move over certain definitions in code.
>>                                                          ^^^^^^^^^^^
>> I'd use "code units" or "units of code" here.
>
> Done.
>
>>
>> Also, should we perhaps name the section "Moving by Statements"? or
>> would it be too inaccurate?
>>
>
> I'm not sure.  I think that maybe because the commands involved, and the
> ones that implicitly will be impacted, such as kill-sentence and friends
> it is best to stay with Sentences?  But a statement is the better term
> wrt programming languages of course.  I hold no strong opinions here.

FWIW, while it may correspond to "statements" for some languages, it
will correspond to other things in other languages (e.g. those that
don't have a notion of "statement"), so it's probably best to stick to
"sentence" here and then in the doc explain how that notion is expected
to be mapped to notions that make sense for a given language.


        Stefan

Message #95 received at 60623 <at> debbugs.gnu.org (full text, mbox):

From: Theodor Thornhill <theo <at> thornhill.no>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Eli Zaretskii <eliz <at> gnu.org>, juri <at> linkov.net, casouri <at> gmail.com,
 60623 <at> debbugs.gnu.org, mardani29 <at> yahoo.es
Subject: Re: bug#60623: 30.0.50; Add forward-sentence with tree sitter support
Date: Tue, 10 Jan 2023 21:22:56 +0100

Stefan Monnier <monnier <at> iro.umontreal.ca> writes:

>>>> +* Moving by Sentences:: Commands to move over certain definitions in code.
>>>                                                          ^^^^^^^^^^^
>>> I'd use "code units" or "units of code" here.
>>
>> Done.
>>
>>>
>>> Also, should we perhaps name the section "Moving by Statements"? or
>>> would it be too inaccurate?
>>>
>>
>> I'm not sure.  I think that maybe because the commands involved, and the
>> ones that implicitly will be impacted, such as kill-sentence and friends
>> it is best to stay with Sentences?  But a statement is the better term
>> wrt programming languages of course.  I hold no strong opinions here.
>
> FWIW, while it may correspond to "statements" for some languages, it
> will correspond to other things in other languages (e.g. those that
> don't have a notion of "statement"), so it's probably best to stick to
> "sentence" here and then in the doc explain how that notion is expected
> to be mapped to notions that make sense for a given language.
>
>
>         Stefan


Yeah, that makes sense.

Theo

Message #98 received at 60623 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Theodor Thornhill <theo <at> thornhill.no>
Cc: Eli Zaretskii <eliz <at> gnu.org>, juri <at> linkov.net, casouri <at> gmail.com,
 60623 <at> debbugs.gnu.org, mardani29 <at> yahoo.es
Subject: Re: bug#60623: 30.0.50; Add forward-sentence with tree sitter support
Date: Tue, 10 Jan 2023 15:28:38 -0500

>>>>> +* Moving by Sentences:: Commands to move over certain definitions in code.
[...]
>> FWIW, while it may correspond to "statements" for some languages, it
>> will correspond to other things in other languages (e.g. those that
>> don't have a notion of "statement"), so it's probably best to stick to
>> "sentence" here and then in the doc explain how that notion is expected
>> to be mapped to notions that make sense for a given language.
> Yeah, that makes sense.

Another option is to use a mix, as in:

    Moving by Sentences:: Commands to move over statement-like code chunks.


-- Stefan

Message #101 received at 60623 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>, Theodor Thornhill
 <theo <at> thornhill.no>
Cc: Eli Zaretskii <eliz <at> gnu.org>, "mardani29 <at> yahoo.es" <mardani29 <at> yahoo.es>,
 "casouri <at> gmail.com" <casouri <at> gmail.com>,
 "60623 <at> debbugs.gnu.org" <60623 <at> debbugs.gnu.org>,
 "juri <at> linkov.net" <juri <at> linkov.net>
Subject: RE: [External] : bug#60623: 30.0.50; Add forward-sentence with tree
 sitter support
Date: Tue, 10 Jan 2023 21:00:09 +0000

> > I'm not sure.  I think that maybe because the commands involved, and the
> > ones that implicitly will be impacted, such as kill-sentence and friends
> > it is best to stay with Sentences?  But a statement is the better term
> > wrt programming languages of course.  I hold no strong opinions here.
> 
> FWIW, while it may correspond to "statements" for some languages, it
> will correspond to other things in other languages (e.g. those that
> don't have a notion of "statement"), so it's probably best to stick to
> "sentence" here and then in the doc explain how that notion is expected
> to be mapped to notions that make sense for a given language.

I'm not following this thread.  But neither
"statement" nor "sentence" sounds appropriate for
what you're apparently talking about.  Both would
seem to be misleading.  Maybe you should add a new
THING type for just what you mean?

Message #104 received at 60623 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Theodor Thornhill <theo <at> thornhill.no>
Cc: 60623 <at> debbugs.gnu.org, juri <at> linkov.net, casouri <at> gmail.com,
 monnier <at> iro.umontreal.ca, mardani29 <at> yahoo.es
Subject: Re: bug#60623: 30.0.50; Add forward-sentence with tree sitter support
Date: Wed, 11 Jan 2023 16:08:08 +0200

> From: Theodor Thornhill <theo <at> thornhill.no>
> Cc: mardani29 <at> yahoo.es, 60623 <at> debbugs.gnu.org, casouri <at> gmail.com,
>  monnier <at> iro.umontreal.ca, juri <at> linkov.net
> Date: Tue, 10 Jan 2023 20:33:52 +0100
> 
> How about this?

LGTM, thanks.  Just one gotcha:

> --- a/doc/emacs/text.texi
> +++ b/doc/emacs/text.texi
> @@ -253,6 +253,11 @@ Sentences
>  of a sentence.  Set the variable @code{sentence-end-without-period} to
>  @code{t} in such cases.
>  
> +  Even though the above mentioned sentence movement commands are based
> +on human languages, other Emacs modes can set these command to get
> +similar functionality (@pxref{Moving by Sentences,,, emacs, The
> +extensible self-documenting text editor}).

This is a cross-reference to the same manual, so you don't need the
full 5-argument form of @pxref; just the node name will suffice.

I think I can give you write access to the tree now, so please request
access on the Savannah page (and create a user if you haven't
already), and then you can install this yourself.

Message #107 received at 60623 <at> debbugs.gnu.org (full text, mbox):

From: Theodor Thornhill <theo <at> thornhill.no>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 60623 <at> debbugs.gnu.org, juri <at> linkov.net, casouri <at> gmail.com,
 monnier <at> iro.umontreal.ca, mardani29 <at> yahoo.es
Subject: Re: bug#60623: 30.0.50; Add forward-sentence with tree sitter support
Date: Wed, 11 Jan 2023 15:41:48 +0100

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Theodor Thornhill <theo <at> thornhill.no>
>> Cc: mardani29 <at> yahoo.es, 60623 <at> debbugs.gnu.org, casouri <at> gmail.com,
>>  monnier <at> iro.umontreal.ca, juri <at> linkov.net
>> Date: Tue, 10 Jan 2023 20:33:52 +0100
>> 
>> How about this?
>
> LGTM, thanks.  Just one gotcha:
>
>> --- a/doc/emacs/text.texi
>> +++ b/doc/emacs/text.texi
>> @@ -253,6 +253,11 @@ Sentences
>>  of a sentence.  Set the variable @code{sentence-end-without-period} to
>>  @code{t} in such cases.
>>  
>> +  Even though the above mentioned sentence movement commands are based
>> +on human languages, other Emacs modes can set these command to get
>> +similar functionality (@pxref{Moving by Sentences,,, emacs, The
>> +extensible self-documenting text editor}).
>
> This is a cross-reference to the same manual, so you don't need the
> full 5-argument form of @pxref; just the node name will suffice.

Right, thanks!

>
> I think I can give you write access to the tree now, so please request
> access on the Savannah page (and create a user if you haven't
> already), and then you can install this yourself.

Wow thanks!  Request sent :-)

Theo

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.