GNU bug report logs -
#59662
29.0.50; [PATCH] Add treesit--indent-defun
Previous Next
Reported by: Theodor Thornhill <theo <at> thornhill.no>
Date: Mon, 28 Nov 2022 19:33:01 UTC
Severity: wishlist
Tags: patch
Found in version 29.0.50
Done: Yuan Fu <casouri <at> gmail.com>
Bug is archived. No further changes may be made.
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 59662 in the body.
You can then email your comments to 59662 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
eliz <at> gnu.org, casouri <at> gmail.org, bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Mon, 28 Nov 2022 19:33:01 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Theodor Thornhill <theo <at> thornhill.no>:
New bug report received and forwarded. Copy sent to
eliz <at> gnu.org, casouri <at> gmail.org, bug-gnu-emacs <at> gnu.org.
(Mon, 28 Nov 2022 19:33:01 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Hi Eli and Yuan!
I've added a function to treesit.el, for use in
treesit-major-mode-setup. Because the treesit-defun-type-regexp gives
us "defuns" for free, we can set fill-paragraph-function to this new
function, thus enabling quick formatting, and some sensible default for
fill-paragraph. It aims to mirror c-indent-defun.
I think this is a nice way to get this functionality for free, but I'm
not 100% whether this is considered ok or not.
An alternative could be to add a 'treesit-mode-map' where we can
auto-enable such constructs.
Feel free to apply if it looks ok.
Theo
[0001-Add-treesit-indent-defun.patch (text/x-diff, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Tue, 29 Nov 2022 06:11:01 GMT)
Full text and
rfc822 format available.
Message #8 received at 59662 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Rebased on newest emacs-29 and added bug number in commit.
Theo
[0001-Add-treesit-indent-defun.patch (text/x-diff, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Tue, 29 Nov 2022 12:02:02 GMT)
Full text and
rfc822 format available.
Message #11 received at 59662 <at> debbugs.gnu.org (full text, mbox):
> Cc: eliz <at> gnu.org, casouri <at> gmail.org
> From: Theodor Thornhill <theo <at> thornhill.no>
> Date: Mon, 28 Nov 2022 20:32:01 +0100
>
> I've added a function to treesit.el, for use in
> treesit-major-mode-setup. Because the treesit-defun-type-regexp gives
> us "defuns" for free, we can set fill-paragraph-function to this new
> function, thus enabling quick formatting, and some sensible default for
> fill-paragraph. It aims to mirror c-indent-defun.
>
> I think this is a nice way to get this functionality for free, but I'm
> not 100% whether this is considered ok or not.
Sounds good, but why did you think it wouldn't be OK? Anything here that
doesn't meet the eye?
> An alternative could be to add a 'treesit-mode-map' where we can
> auto-enable such constructs.
I think this is less desirable.
Yuan, WDYT?
> @@ -1698,7 +1713,10 @@ treesit-major-mode-setup
> ;; Navigation.
> (when treesit-defun-type-regexp
> (setq-local beginning-of-defun-function #'treesit-beginning-of-defun)
> - (setq-local end-of-defun-function #'treesit-end-of-defun)))
> + (setq-local end-of-defun-function #'treesit-end-of-defun))
> + ;; Filling
> + (when (and treesit-defun-type-regexp treesit-simple-indent-rules)
> + (setq-local fill-paragraph-function #'treesit--indent-defun)))
I'm a bit confused: if the function's name is treesit--indent-defun, and it
uses treesit-indent-region to do its job, why do we assign it to
fill-paragraph-function, which is supposed to _fill_, not to _indent_?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Tue, 29 Nov 2022 12:15:03 GMT)
Full text and
rfc822 format available.
Message #14 received at 59662 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
>> Cc: eliz <at> gnu.org, casouri <at> gmail.org
>> From: Theodor Thornhill <theo <at> thornhill.no>
>> Date: Mon, 28 Nov 2022 20:32:01 +0100
>>
>> I've added a function to treesit.el, for use in
>> treesit-major-mode-setup. Because the treesit-defun-type-regexp gives
>> us "defuns" for free, we can set fill-paragraph-function to this new
>> function, thus enabling quick formatting, and some sensible default for
>> fill-paragraph. It aims to mirror c-indent-defun.
>>
>> I think this is a nice way to get this functionality for free, but I'm
>> not 100% whether this is considered ok or not.
>
> Sounds good, but why did you think it wouldn't be OK? Anything here that
> doesn't meet the eye?
>
Not really, but see below answer.
>> An alternative could be to add a 'treesit-mode-map' where we can
>> auto-enable such constructs.
>
> I think this is less desirable.
>
> Yuan, WDYT?
>
>> @@ -1698,7 +1713,10 @@ treesit-major-mode-setup
>> ;; Navigation.
>> (when treesit-defun-type-regexp
>> (setq-local beginning-of-defun-function #'treesit-beginning-of-defun)
>> - (setq-local end-of-defun-function #'treesit-end-of-defun)))
>> + (setq-local end-of-defun-function #'treesit-end-of-defun))
>> + ;; Filling
>> + (when (and treesit-defun-type-regexp treesit-simple-indent-rules)
>> + (setq-local fill-paragraph-function #'treesit--indent-defun)))
>
> I'm a bit confused: if the function's name is treesit--indent-defun, and it
> uses treesit-indent-region to do its job, why do we assign it to
> fill-paragraph-function, which is supposed to _fill_, not to _indent_?
This is why I was thinking it would maybe be better to put it into a
treesit-mode-map that major-modes can inherit from, thus binding it to
things such as C-c C-q. The reason I put it in filling was because that
is a common key to press in everything _but_ prog-modes. And prog-modes
seem to mostly just turn it off if not inside of comments etc. This
would behave just like that, except we would reformat/reindent/refill
code.
In a way filling _is_ formatting/reindenting, at least that's how I look
at it.
Theo
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Tue, 29 Nov 2022 14:15:02 GMT)
Full text and
rfc822 format available.
Message #17 received at 59662 <at> debbugs.gnu.org (full text, mbox):
Resending with correct address of Yuan.
> From: Theodor Thornhill <theo <at> thornhill.no>
> Cc: 59662 <at> debbugs.gnu.org, casouri <at> gmail.org
> Date: Tue, 29 Nov 2022 13:14:21 +0100
>
> Eli Zaretskii <eliz <at> gnu.org> writes:
>
> >> Cc: eliz <at> gnu.org, casouri <at> gmail.org
> >> From: Theodor Thornhill <theo <at> thornhill.no>
> >> Date: Mon, 28 Nov 2022 20:32:01 +0100
> >>
> >> I've added a function to treesit.el, for use in
> >> treesit-major-mode-setup. Because the treesit-defun-type-regexp gives
> >> us "defuns" for free, we can set fill-paragraph-function to this new
> >> function, thus enabling quick formatting, and some sensible default for
> >> fill-paragraph. It aims to mirror c-indent-defun.
> >>
> >> I think this is a nice way to get this functionality for free, but I'm
> >> not 100% whether this is considered ok or not.
> >
> > Sounds good, but why did you think it wouldn't be OK? Anything here that
> > doesn't meet the eye?
> >
>
> Not really, but see below answer.
>
> >> An alternative could be to add a 'treesit-mode-map' where we can
> >> auto-enable such constructs.
> >
> > I think this is less desirable.
> >
> > Yuan, WDYT?
> >
> >> @@ -1698,7 +1713,10 @@ treesit-major-mode-setup
> >> ;; Navigation.
> >> (when treesit-defun-type-regexp
> >> (setq-local beginning-of-defun-function #'treesit-beginning-of-defun)
> >> - (setq-local end-of-defun-function #'treesit-end-of-defun)))
> >> + (setq-local end-of-defun-function #'treesit-end-of-defun))
> >> + ;; Filling
> >> + (when (and treesit-defun-type-regexp treesit-simple-indent-rules)
> >> + (setq-local fill-paragraph-function #'treesit--indent-defun)))
> >
> > I'm a bit confused: if the function's name is treesit--indent-defun, and it
> > uses treesit-indent-region to do its job, why do we assign it to
> > fill-paragraph-function, which is supposed to _fill_, not to _indent_?
>
> This is why I was thinking it would maybe be better to put it into a
> treesit-mode-map that major-modes can inherit from, thus binding it to
> things such as C-c C-q. The reason I put it in filling was because that
> is a common key to press in everything _but_ prog-modes. And prog-modes
> seem to mostly just turn it off if not inside of comments etc. This
> would behave just like that, except we would reformat/reindent/refill
> code.
>
> In a way filling _is_ formatting/reindenting, at least that's how I look
> at it.
>
> Theo
>
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Wed, 30 Nov 2022 10:34:01 GMT)
Full text and
rfc822 format available.
Message #20 received at 59662 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
> Resending with correct address of Yuan.
>
>> From: Theodor Thornhill <theo <at> thornhill.no>
>> Cc: 59662 <at> debbugs.gnu.org, casouri <at> gmail.org
>> Date: Tue, 29 Nov 2022 13:14:21 +0100
>>
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>>
>> >> Cc: eliz <at> gnu.org, casouri <at> gmail.org
>> >> From: Theodor Thornhill <theo <at> thornhill.no>
>> >> Date: Mon, 28 Nov 2022 20:32:01 +0100
>> >>
>> >> I've added a function to treesit.el, for use in
>> >> treesit-major-mode-setup. Because the treesit-defun-type-regexp gives
>> >> us "defuns" for free, we can set fill-paragraph-function to this new
>> >> function, thus enabling quick formatting, and some sensible default for
>> >> fill-paragraph. It aims to mirror c-indent-defun.
>> >>
>> >> I think this is a nice way to get this functionality for free, but I'm
>> >> not 100% whether this is considered ok or not.
>> >
>> > Sounds good, but why did you think it wouldn't be OK? Anything here that
>> > doesn't meet the eye?
>> >
>>
>> Not really, but see below answer.
>>
>> >> An alternative could be to add a 'treesit-mode-map' where we can
>> >> auto-enable such constructs.
>> >
>> > I think this is less desirable.
>> >
>> > Yuan, WDYT?
>> >
>> >> @@ -1698,7 +1713,10 @@ treesit-major-mode-setup
>> >> ;; Navigation.
>> >> (when treesit-defun-type-regexp
>> >> (setq-local beginning-of-defun-function #'treesit-beginning-of-defun)
>> >> - (setq-local end-of-defun-function #'treesit-end-of-defun)))
>> >> + (setq-local end-of-defun-function #'treesit-end-of-defun))
>> >> + ;; Filling
>> >> + (when (and treesit-defun-type-regexp treesit-simple-indent-rules)
>> >> + (setq-local fill-paragraph-function #'treesit--indent-defun)))
>> >
>> > I'm a bit confused: if the function's name is treesit--indent-defun, and it
>> > uses treesit-indent-region to do its job, why do we assign it to
>> > fill-paragraph-function, which is supposed to _fill_, not to _indent_?
>>
>> This is why I was thinking it would maybe be better to put it into a
>> treesit-mode-map that major-modes can inherit from, thus binding it to
>> things such as C-c C-q. The reason I put it in filling was because that
>> is a common key to press in everything _but_ prog-modes. And prog-modes
>> seem to mostly just turn it off if not inside of comments etc. This
>> would behave just like that, except we would reformat/reindent/refill
>> code.
>>
>> In a way filling _is_ formatting/reindenting, at least that's how I look
>> at it.
>>
>> Theo
>>
I was caught up in some stuff the past two days, sorry about that :-)
I’ll have a look tomorrow!
Yuan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Wed, 30 Nov 2022 23:24:02 GMT)
Full text and
rfc822 format available.
Message #23 received at 59662 <at> debbugs.gnu.org (full text, mbox):
Theodor Thornhill <theo <at> thornhill.no> writes:
> Eli Zaretskii <eliz <at> gnu.org> writes:
>
>>> Cc: eliz <at> gnu.org, casouri <at> gmail.org
>>> From: Theodor Thornhill <theo <at> thornhill.no>
>>> Date: Mon, 28 Nov 2022 20:32:01 +0100
>>>
>>> I've added a function to treesit.el, for use in
>>> treesit-major-mode-setup. Because the treesit-defun-type-regexp gives
>>> us "defuns" for free, we can set fill-paragraph-function to this new
>>> function, thus enabling quick formatting, and some sensible default for
>>> fill-paragraph. It aims to mirror c-indent-defun.
>>>
>>> I think this is a nice way to get this functionality for free, but I'm
>>> not 100% whether this is considered ok or not.
>>
>> Sounds good, but why did you think it wouldn't be OK? Anything here that
>> doesn't meet the eye?
>>
>
> Not really, but see below answer.
>
>>> An alternative could be to add a 'treesit-mode-map' where we can
>>> auto-enable such constructs.
>>
>> I think this is less desirable.
>>
>> Yuan, WDYT?
>>
>>> @@ -1698,7 +1713,10 @@ treesit-major-mode-setup
>>> ;; Navigation.
>>> (when treesit-defun-type-regexp
>>> (setq-local beginning-of-defun-function #'treesit-beginning-of-defun)
>>> - (setq-local end-of-defun-function #'treesit-end-of-defun)))
>>> + (setq-local end-of-defun-function #'treesit-end-of-defun))
>>> + ;; Filling
>>> + (when (and treesit-defun-type-regexp treesit-simple-indent-rules)
>>> + (setq-local fill-paragraph-function #'treesit--indent-defun)))
>>
>> I'm a bit confused: if the function's name is treesit--indent-defun, and it
>> uses treesit-indent-region to do its job, why do we assign it to
>> fill-paragraph-function, which is supposed to _fill_, not to _indent_?
>
> This is why I was thinking it would maybe be better to put it into a
> treesit-mode-map that major-modes can inherit from, thus binding it to
> things such as C-c C-q. The reason I put it in filling was because that
> is a common key to press in everything _but_ prog-modes. And prog-modes
> seem to mostly just turn it off if not inside of comments etc. This
> would behave just like that, except we would reformat/reindent/refill
> code.
>
> In a way filling _is_ formatting/reindenting, at least that's how I look
> at it.
I see, so you want to implement C-c C-q in c-mode. But why don’t we
make treesit--indent-defun a command and bind it in C-c C-q in major modes?
Filling is not indent IMO. If you fill a list in elisp code it wraps long lines
instead of indent (IIRC).
Also, this is really not tree-sitter specific, it doesn’t require any
tree-sitter feature to accomplish: indent-defun only needs mark-defun
and indent-region, both are supported by practically any major mode.
Normally this kind of thing goes into lisp.el, alongside commands like
fill-paragraph, indent-region, beginning-of-defun, etc, and claim a
global keybinding. But maybe we only want it to live under C-c prefix,
in that case I guess we can bind it in prog-mode-map, under C-c C-q?
I’m thinking something like
(defun prog-mode-indent-defun ()
(interactive)
(mark-defun)
(indent-region (region-beginning) (region-end)))
(defvar prog-mode-map
(...
(define-key map (kbd "C-c C-q") #'prog-mode-indent-defun)))
Yuan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Thu, 01 Dec 2022 01:08:02 GMT)
Full text and
rfc822 format available.
Message #26 received at 59662 <at> debbugs.gnu.org (full text, mbox):
> On Nov 30, 2022, at 3:23 PM, Yuan Fu <casouri <at> gmail.com> wrote:
>
>
> Theodor Thornhill <theo <at> thornhill.no> writes:
>
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>>
>>>> Cc: eliz <at> gnu.org, casouri <at> gmail.org
>>>> From: Theodor Thornhill <theo <at> thornhill.no>
>>>> Date: Mon, 28 Nov 2022 20:32:01 +0100
>>>>
>>>> I've added a function to treesit.el, for use in
>>>> treesit-major-mode-setup. Because the treesit-defun-type-regexp gives
>>>> us "defuns" for free, we can set fill-paragraph-function to this new
>>>> function, thus enabling quick formatting, and some sensible default for
>>>> fill-paragraph. It aims to mirror c-indent-defun.
>>>>
>>>> I think this is a nice way to get this functionality for free, but I'm
>>>> not 100% whether this is considered ok or not.
>>>
>>> Sounds good, but why did you think it wouldn't be OK? Anything here that
>>> doesn't meet the eye?
>>>
>>
>> Not really, but see below answer.
>>
>>>> An alternative could be to add a 'treesit-mode-map' where we can
>>>> auto-enable such constructs.
>>>
>>> I think this is less desirable.
>>>
>>> Yuan, WDYT?
>>>
>>>> @@ -1698,7 +1713,10 @@ treesit-major-mode-setup
>>>> ;; Navigation.
>>>> (when treesit-defun-type-regexp
>>>> (setq-local beginning-of-defun-function #'treesit-beginning-of-defun)
>>>> - (setq-local end-of-defun-function #'treesit-end-of-defun)))
>>>> + (setq-local end-of-defun-function #'treesit-end-of-defun))
>>>> + ;; Filling
>>>> + (when (and treesit-defun-type-regexp treesit-simple-indent-rules)
>>>> + (setq-local fill-paragraph-function #'treesit--indent-defun)))
>>>
>>> I'm a bit confused: if the function's name is treesit--indent-defun, and it
>>> uses treesit-indent-region to do its job, why do we assign it to
>>> fill-paragraph-function, which is supposed to _fill_, not to _indent_?
>>
>> This is why I was thinking it would maybe be better to put it into a
>> treesit-mode-map that major-modes can inherit from, thus binding it to
>> things such as C-c C-q. The reason I put it in filling was because that
>> is a common key to press in everything _but_ prog-modes. And prog-modes
>> seem to mostly just turn it off if not inside of comments etc. This
>> would behave just like that, except we would reformat/reindent/refill
>> code.
>>
>> In a way filling _is_ formatting/reindenting, at least that's how I look
>> at it.
>
> I see, so you want to implement C-c C-q in c-mode. But why don’t we
> make treesit--indent-defun a command and bind it in C-c C-q in major modes?
> Filling is not indent IMO. If you fill a list in elisp code it wraps long lines
> instead of indent (IIRC).
>
> Also, this is really not tree-sitter specific, it doesn’t require any
> tree-sitter feature to accomplish: indent-defun only needs mark-defun
> and indent-region, both are supported by practically any major mode.
>
> Normally this kind of thing goes into lisp.el, alongside commands like
> fill-paragraph, indent-region, beginning-of-defun, etc, and claim a
> global keybinding. But maybe we only want it to live under C-c prefix,
> in that case I guess we can bind it in prog-mode-map, under C-c C-q?
>
> I’m thinking something like
>
> (defun prog-mode-indent-defun ()
> (interactive)
> (mark-defun)
> (indent-region (region-beginning) (region-end)))
>
> (defvar prog-mode-map
> (...
> (define-key map (kbd "C-c C-q") #'prog-mode-indent-defun)))
Tho I’d prefer we bind it in major mode maps first. Adding anything to prog-mode-map should go through emacs-devel (and I don’t know if we want to add any bindings to prog-mode-map), of course.
Yuan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Thu, 01 Dec 2022 06:11:02 GMT)
Full text and
rfc822 format available.
Message #29 received at 59662 <at> debbugs.gnu.org (full text, mbox):
Yuan Fu <casouri <at> gmail.com> writes:
> Theodor Thornhill <theo <at> thornhill.no> writes:
>
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>>
>>>> Cc: eliz <at> gnu.org, casouri <at> gmail.org
>>>> From: Theodor Thornhill <theo <at> thornhill.no>
>>>> Date: Mon, 28 Nov 2022 20:32:01 +0100
[...]
>>
>>>> An alternative could be to add a 'treesit-mode-map' where we can
>>>> auto-enable such constructs.
>>>
>>> I think this is less desirable.
>>>
>>> Yuan, WDYT?
>>>
> I see, so you want to implement C-c C-q in c-mode. But why don’t we
> make treesit--indent-defun a command and bind it in C-c C-q in major modes?
> Filling is not indent IMO. If you fill a list in elisp code it wraps long lines
> instead of indent (IIRC).
>
> Also, this is really not tree-sitter specific, it doesn’t require any
> tree-sitter feature to accomplish: indent-defun only needs mark-defun
> and indent-region, both are supported by practically any major mode.
>
> Normally this kind of thing goes into lisp.el, alongside commands like
> fill-paragraph, indent-region, beginning-of-defun, etc, and claim a
> global keybinding. But maybe we only want it to live under C-c prefix,
> in that case I guess we can bind it in prog-mode-map, under C-c C-q?
>
Yeah, that's smart - thanks for the tip! Perhaps adding it to prog-mode
is a bit premature, but maybe Eli has an opinion here?
My guess is that a change in lisp.el is a little too big for the release
branch? So we could let this live in treesit.el if it's too big a
change?
Theo
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Thu, 01 Dec 2022 07:14:01 GMT)
Full text and
rfc822 format available.
Message #32 received at 59662 <at> debbugs.gnu.org (full text, mbox):
> From: Yuan Fu <casouri <at> gmail.com>
> Date: Wed, 30 Nov 2022 15:23:17 -0800
> Cc: Eli Zaretskii <eliz <at> gnu.org>,
> 59662 <at> debbugs.gnu.org,
> casouri <at> gmail.org
>
> Also, this is really not tree-sitter specific, it doesn’t require any
> tree-sitter feature to accomplish: indent-defun only needs mark-defun
> and indent-region, both are supported by practically any major mode.
>
> Normally this kind of thing goes into lisp.el, alongside commands like
> fill-paragraph, indent-region, beginning-of-defun, etc, and claim a
> global keybinding. But maybe we only want it to live under C-c prefix,
> in that case I guess we can bind it in prog-mode-map, under C-c C-q?
>
> I’m thinking something like
>
> (defun prog-mode-indent-defun ()
> (interactive)
> (mark-defun)
> (indent-region (region-beginning) (region-end)))
>
> (defvar prog-mode-map
> (...
> (define-key map (kbd "C-c C-q") #'prog-mode-indent-defun)))
SGTM, but I'd begin by doing this only in c-ts-mode.el (and maybe also in
csharp-ts-mode). After all, until now this functionality and the "C-c C-q"
binding was only in CC Mode. (My original question about this was due to
lack of enthusiasm to add the "C Indent" node of the Emacs manual text that
explains this feature is only available in CC Mode, not in the tree-sitter
based C/C++ modes.)
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Thu, 01 Dec 2022 07:21:01 GMT)
Full text and
rfc822 format available.
Message #35 received at 59662 <at> debbugs.gnu.org (full text, mbox):
> From: Yuan Fu <casouri <at> gmail.com>
> Date: Wed, 30 Nov 2022 17:07:48 -0800
> Cc: Eli Zaretskii <eliz <at> gnu.org>,
> 59662 <at> debbugs.gnu.org,
> casouri <at> gmail.org
>
> > I’m thinking something like
> >
> > (defun prog-mode-indent-defun ()
> > (interactive)
> > (mark-defun)
> > (indent-region (region-beginning) (region-end)))
> >
> > (defvar prog-mode-map
> > (...
> > (define-key map (kbd "C-c C-q") #'prog-mode-indent-defun)))
>
> Tho I’d prefer we bind it in major mode maps first. Adding anything to prog-mode-map should go through emacs-devel (and I don’t know if we want to add any bindings to prog-mode-map), of course.
I agree and think we should first do this only in C and related modes.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Thu, 01 Dec 2022 07:24:01 GMT)
Full text and
rfc822 format available.
Message #38 received at 59662 <at> debbugs.gnu.org (full text, mbox):
On 1 December 2022 08:13:22 CET, Eli Zaretskii <eliz <at> gnu.org> wrote:
>> From: Yuan Fu <casouri <at> gmail.com>
>> Date: Wed, 30 Nov 2022 15:23:17 -0800
>> Cc: Eli Zaretskii <eliz <at> gnu.org>,
>> 59662 <at> debbugs.gnu.org,
>> casouri <at> gmail.org
>>
>> Also, this is really not tree-sitter specific, it doesn’t require any
>> tree-sitter feature to accomplish: indent-defun only needs mark-defun
>> and indent-region, both are supported by practically any major mode.
>>
>> Normally this kind of thing goes into lisp.el, alongside commands like
>> fill-paragraph, indent-region, beginning-of-defun, etc, and claim a
>> global keybinding. But maybe we only want it to live under C-c prefix,
>> in that case I guess we can bind it in prog-mode-map, under C-c C-q?
>>
>> I’m thinking something like
>>
>> (defun prog-mode-indent-defun ()
>> (interactive)
>> (mark-defun)
>> (indent-region (region-beginning) (region-end)))
>>
>> (defvar prog-mode-map
>> (...
>> (define-key map (kbd "C-c C-q") #'prog-mode-indent-defun)))
>
>SGTM, but I'd begin by doing this only in c-ts-mode.el (and maybe also in
>csharp-ts-mode). After all, until now this functionality and the "C-c C-q"
>binding was only in CC Mode. (My original question about this was due to
>lack of enthusiasm to add the "C Indent" node of the Emacs manual text that
>explains this feature is only available in CC Mode, not in the tree-sitter
>based C/C++ modes.)
So you'd like the change in lisp.el, but bound only in the modes you mention, or have something similar to the original patch in treesit, but still bound only to the same modes?
Theo
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Thu, 01 Dec 2022 07:49:01 GMT)
Full text and
rfc822 format available.
Message #41 received at 59662 <at> debbugs.gnu.org (full text, mbox):
> Date: Thu, 01 Dec 2022 08:22:45 +0100
> From: Theodor Thornhill <theo <at> thornhill.no>
> CC: 59662 <at> debbugs.gnu.org, casouri <at> gmail.org
>
> >SGTM, but I'd begin by doing this only in c-ts-mode.el (and maybe also in
> >csharp-ts-mode). After all, until now this functionality and the "C-c C-q"
> >binding was only in CC Mode. (My original question about this was due to
> >lack of enthusiasm to add the "C Indent" node of the Emacs manual text that
> >explains this feature is only available in CC Mode, not in the tree-sitter
> >based C/C++ modes.)
>
> So you'd like the change in lisp.el, but bound only in the modes you mention, or have something similar to the original patch in treesit, but still bound only to the same modes?
Why in lisp.el? we are talking about a feature of C-like languages. The
current binding of "C-c C-q" is in cc-cmds.el, so what I had in mind is to
have its counterpart in c-ts-mode.el.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Thu, 01 Dec 2022 08:14:03 GMT)
Full text and
rfc822 format available.
Message #44 received at 59662 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
>> Date: Thu, 01 Dec 2022 08:22:45 +0100
>> From: Theodor Thornhill <theo <at> thornhill.no>
>> CC: 59662 <at> debbugs.gnu.org, casouri <at> gmail.org
>>
>> >SGTM, but I'd begin by doing this only in c-ts-mode.el (and maybe also in
>> >csharp-ts-mode). After all, until now this functionality and the "C-c C-q"
>> >binding was only in CC Mode. (My original question about this was due to
>> >lack of enthusiasm to add the "C Indent" node of the Emacs manual text that
>> >explains this feature is only available in CC Mode, not in the tree-sitter
>> >based C/C++ modes.)
>>
>> So you'd like the change in lisp.el, but bound only in the modes you
>> mention, or have something similar to the original patch in treesit,
>> but still bound only to the same modes?
>
> Why in lisp.el? we are talking about a feature of C-like languages. The
> current binding of "C-c C-q" is in cc-cmds.el, so what I had in mind is to
> have its counterpart in c-ts-mode.el.
Only because Yuan mentioned it. I will make that change.
Theo
Severity set to 'wishlist' from 'normal'
Request was from
Stefan Kangas <stefankangas <at> gmail.com>
to
control <at> debbugs.gnu.org.
(Thu, 01 Dec 2022 13:26:03 GMT)
Full text and
rfc822 format available.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Fri, 02 Dec 2022 15:07:02 GMT)
Full text and
rfc822 format available.
Message #49 received at 59662 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> gnu.org> writes:
>> Date: Thu, 01 Dec 2022 08:22:45 +0100
>> From: Theodor Thornhill <theo <at> thornhill.no>
>> CC: 59662 <at> debbugs.gnu.org, casouri <at> gmail.org
>>
>> >SGTM, but I'd begin by doing this only in c-ts-mode.el (and maybe also in
>> >csharp-ts-mode). After all, until now this functionality and the "C-c C-q"
>> >binding was only in CC Mode. (My original question about this was due to
>> >lack of enthusiasm to add the "C Indent" node of the Emacs manual text that
>> >explains this feature is only available in CC Mode, not in the tree-sitter
>> >based C/C++ modes.)
>>
>> So you'd like the change in lisp.el, but bound only in the modes you mention, or have something similar to the original patch in treesit, but still bound only to the same modes?
>
> Why in lisp.el? we are talking about a feature of C-like languages. The
> current binding of "C-c C-q" is in cc-cmds.el, so what I had in mind is to
> have its counterpart in c-ts-mode.el.
Something like this?
Theo
[0001-Add-c-ts-mode-indent-defun.patch (text/x-diff, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Fri, 02 Dec 2022 15:46:02 GMT)
Full text and
rfc822 format available.
Message #52 received at 59662 <at> debbugs.gnu.org (full text, mbox):
> From: Theodor Thornhill <theo <at> thornhill.no>
> Cc: casouri <at> gmail.com, 59662 <at> debbugs.gnu.org
> Date: Fri, 02 Dec 2022 16:06:46 +0100
>
> > Why in lisp.el? we are talking about a feature of C-like languages. The
> > current binding of "C-c C-q" is in cc-cmds.el, so what I had in mind is to
> > have its counterpart in c-ts-mode.el.
>
> Something like this?
Yes, thanks.
But a command should not have "--" in its name, since every command is by
definition a public function, not an internal one.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Fri, 02 Dec 2022 18:33:02 GMT)
Full text and
rfc822 format available.
Message #55 received at 59662 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> gnu.org> writes:
>> From: Theodor Thornhill <theo <at> thornhill.no>
>> Cc: casouri <at> gmail.com, 59662 <at> debbugs.gnu.org
>> Date: Fri, 02 Dec 2022 16:06:46 +0100
>>
>> > Why in lisp.el? we are talking about a feature of C-like languages. The
>> > current binding of "C-c C-q" is in cc-cmds.el, so what I had in mind is to
>> > have its counterpart in c-ts-mode.el.
>>
>> Something like this?
>
> Yes, thanks.
>
> But a command should not have "--" in its name, since every command is by
> definition a public function, not an internal one.
That makes sense. I was in doubt about that, but decided to go the
(wrong) direction. Updated in latest patch.
However - mark-defun signals an error on some constructs because of
number-or-marker error:
Debugger entered--Lisp error: (wrong-type-argument integer-or-marker-p nil)
treesit-end-of-defun()
c-ts-mode--end-of-defun()
end-of-defun()
mark-defun(1 nil)
mark-defun(1 23209)
funcall-interactively(mark-defun 1 23209)
call-interactively(mark-defun record nil)
command-execute(mark-defun record)
execute-extended-command(nil "mark-defun" nil)
funcall-interactively(execute-extended-command nil "mark-defun" nil)
call-interactively(execute-extended-command nil nil)
command-execute(execute-extended-command)
I'll file a separate bug report for this one, if that's ok? Yuan, are
you ok with this patch?
Theo
[0001-Add-c-ts-mode-indent-defun.patch (text/x-diff, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Fri, 02 Dec 2022 18:53:02 GMT)
Full text and
rfc822 format available.
Message #58 received at 59662 <at> debbugs.gnu.org (full text, mbox):
> From: Theodor Thornhill <theo <at> thornhill.no>
> Cc: casouri <at> gmail.com, 59662 <at> debbugs.gnu.org
> Date: Fri, 02 Dec 2022 19:32:33 +0100
>
> However - mark-defun signals an error on some constructs because of
> number-or-marker error:
>
> Debugger entered--Lisp error: (wrong-type-argument integer-or-marker-p nil)
> treesit-end-of-defun()
> c-ts-mode--end-of-defun()
> end-of-defun()
> mark-defun(1 nil)
> mark-defun(1 23209)
> funcall-interactively(mark-defun 1 23209)
> call-interactively(mark-defun record nil)
> command-execute(mark-defun record)
> execute-extended-command(nil "mark-defun" nil)
> funcall-interactively(execute-extended-command nil "mark-defun" nil)
> call-interactively(execute-extended-command nil nil)
> command-execute(execute-extended-command)
>
> I'll file a separate bug report for this one, if that's ok?
Yes, please.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Fri, 02 Dec 2022 19:03:02 GMT)
Full text and
rfc822 format available.
Message #61 received at submit <at> debbugs.gnu.org (full text, mbox):
Theodor Thornhill via "Bug reports for GNU Emacs, the Swiss army knife
of text editors" <bug-gnu-emacs <at> gnu.org> writes:
>>
>> Why in lisp.el? we are talking about a feature of C-like languages. The
>> current binding of "C-c C-q" is in cc-cmds.el, so what I had in mind is to
>> have its counterpart in c-ts-mode.el.
>
> Something like this?
Thanks, some comments below:
>
> From 26a8780950017a911bac7290366da05e0e35f13f Mon Sep 17 00:00:00 2001
> From: Theodor Thornhill <theo <at> thornhill.no>
> Date: Fri, 2 Dec 2022 16:05:35 +0100
> Subject: [PATCH] Add c-ts-mode--indent-defun
>
> Add in this function to mimic 'c-indent-defun'
>
> * lisp/progmodes/c-ts-mode.el (c-ts-mode--indent-defun): New function.
> (c-ts-mode-map): New mode map that uses said function.
> ---
> lisp/progmodes/c-ts-mode.el | 18 +++++++++++++++++-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/lisp/progmodes/c-ts-mode.el b/lisp/progmodes/c-ts-mode.el
> index fcabb5beac..3171600901 100644
> --- a/lisp/progmodes/c-ts-mode.el
> +++ b/lisp/progmodes/c-ts-mode.el
> @@ -518,9 +518,25 @@ c-ts-mode--end-of-defun
> (if (looking-at "\\s<\\|\n")
> (forward-line 1)))))
>
> +(defun c-ts-mode--indent-defun ()
This should be public: c-ts-mode-indent-defun
> + "Indent the current top-level declaration syntactically.
> +
> +`treesit-defun-type-regexp' defines what constructs to indent."
> + (interactive)
(interactive "*") will handle read-only buffers a bit better.
> + (save-excursion
> + (mark-defun)
> + (indent-region (region-beginning) (region-end))))
Nit: Once you call (mark-defun), you can call
(indent-region (point) (mark))
which should be more efficient than calling (region-beginning) or
(region-end).
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Fri, 02 Dec 2022 19:03:02 GMT)
Full text and
rfc822 format available.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Fri, 02 Dec 2022 19:54:02 GMT)
Full text and
rfc822 format available.
Message #67 received at 59662 <at> debbugs.gnu.org (full text, mbox):
On 01/12/2022 09:13, Eli Zaretskii wrote:
> After all, until now this functionality and the "C-c C-q"
> binding was only in CC Mode.
There is also the 'M-q' binding in the popular paredit-mode.
It runs paredit-reindent-defun.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Fri, 02 Dec 2022 20:03:01 GMT)
Full text and
rfc822 format available.
Message #70 received at 59662 <at> debbugs.gnu.org (full text, mbox):
On 02/12/2022 21:53, Dmitry Gutov wrote:
> On 01/12/2022 09:13, Eli Zaretskii wrote:
>> After all, until now this functionality and the "C-c C-q"
>> binding was only in CC Mode.
>
> There is also the 'M-q' binding in the popular paredit-mode.
>
> It runs paredit-reindent-defun.
That function runs fill-paragraph when in string or comment, and
beginning-of-defun + indent-sexp otherwise.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Fri, 02 Dec 2022 20:10:03 GMT)
Full text and
rfc822 format available.
Message #73 received at 59662 <at> debbugs.gnu.org (full text, mbox):
On 2 December 2022 20:53:13 CET, Dmitry Gutov <dgutov <at> yandex.ru> wrote:
>On 01/12/2022 09:13, Eli Zaretskii wrote:
>> After all, until now this functionality and the "C-c C-q"
>> binding was only in CC Mode.
>
>There is also the 'M-q' binding in the popular paredit-mode.
>
>It runs paredit-reindent-defun.
Yeah. My first iteration was piggybacking on fill-paragraph-function to get that binding, but I believe that was undesirable.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Fri, 02 Dec 2022 20:25:02 GMT)
Full text and
rfc822 format available.
Message #76 received at submit <at> debbugs.gnu.org (full text, mbox):
[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:
>
>>>
>>> Why in lisp.el? we are talking about a feature of C-like languages. The
>>> current binding of "C-c C-q" is in cc-cmds.el, so what I had in mind is to
>>> have its counterpart in c-ts-mode.el.
>>
>> Something like this?
>
> Thanks, some comments below:
>
Thanks for looking!
>>
>> From 26a8780950017a911bac7290366da05e0e35f13f Mon Sep 17 00:00:00 2001
>> From: Theodor Thornhill <theo <at> thornhill.no>
>> Date: Fri, 2 Dec 2022 16:05:35 +0100
>> Subject: [PATCH] Add c-ts-mode--indent-defun
>>
>> Add in this function to mimic 'c-indent-defun'
>>
>> * lisp/progmodes/c-ts-mode.el (c-ts-mode--indent-defun): New function.
>> (c-ts-mode-map): New mode map that uses said function.
>> ---
>> lisp/progmodes/c-ts-mode.el | 18 +++++++++++++++++-
>> 1 file changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/lisp/progmodes/c-ts-mode.el b/lisp/progmodes/c-ts-mode.el
>> index fcabb5beac..3171600901 100644
>> --- a/lisp/progmodes/c-ts-mode.el
>> +++ b/lisp/progmodes/c-ts-mode.el
>> @@ -518,9 +518,25 @@ c-ts-mode--end-of-defun
>> (if (looking-at "\\s<\\|\n")
>> (forward-line 1)))))
>>
>> +(defun c-ts-mode--indent-defun ()
>
> This should be public: c-ts-mode-indent-defun
>
Yeah.
>> + "Indent the current top-level declaration syntactically.
>> +
>> +`treesit-defun-type-regexp' defines what constructs to indent."
>> + (interactive)
>
> (interactive "*") will handle read-only buffers a bit better.
>
I was wondering about that, but I couldn't really find any situation
that gave any difference. But I've added it now.
>> + (save-excursion
>> + (mark-defun)
>> + (indent-region (region-beginning) (region-end))))
>
> Nit: Once you call (mark-defun), you can call
>
> (indent-region (point) (mark))
>
> which should be more efficient than calling (region-beginning) or
> (region-end).
Right - thanks. However, this makes me wonder - should we really be
setting mark here? I see that c-indent-defun does not, and it feels
weird that indenting adds to the mark ring. What do you think? My
first patch used
(indent-region
(treesit-node-start node)
(treesit-node-end node))
Which behaves similarly to c-indent-defun.
See attached patch.
Thanks,
Theo
[0001-Add-c-ts-mode-indent-defun.patch (text/x-diff, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Fri, 02 Dec 2022 20:25:02 GMT)
Full text and
rfc822 format available.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Fri, 02 Dec 2022 21:11:02 GMT)
Full text and
rfc822 format available.
Message #82 received at submit <at> debbugs.gnu.org (full text, mbox):
Theodor Thornhill <theo <at> thornhill.no> writes:
> Right - thanks. However, this makes me wonder - should we really be
> setting mark here? I see that c-indent-defun does not, and it feels
> weird that indenting adds to the mark ring. What do you think? My
> first patch used
>
> (indent-region
> (treesit-node-start node)
> (treesit-node-end node))
>
> Which behaves similarly to c-indent-defun.
>
> See attached patch.
I don't have a strong opinion, but if we can indent without setting the
mark, I think it'd be a cleaner command. Specially if c-indent-defun
doesn't set the mark either.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Fri, 02 Dec 2022 21:11:02 GMT)
Full text and
rfc822 format available.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Fri, 02 Dec 2022 21:21:02 GMT)
Full text and
rfc822 format available.
Message #88 received at submit <at> debbugs.gnu.org (full text, mbox):
On 2 December 2022 22:09:55 CET, "Daniel Martín" <mardani29 <at> yahoo.es> wrote:
>Theodor Thornhill <theo <at> thornhill.no> writes:
>
>> Right - thanks. However, this makes me wonder - should we really be
>> setting mark here? I see that c-indent-defun does not, and it feels
>> weird that indenting adds to the mark ring. What do you think? My
>> first patch used
>>
>> (indent-region
>> (treesit-node-start node)
>> (treesit-node-end node))
>>
>> Which behaves similarly to c-indent-defun.
>>
>> See attached patch.
>
>I don't have a strong opinion, but if we can indent without setting the
>mark, I think it'd be a cleaner command. Specially if c-indent-defun
>doesn't set the mark either.
Yeah, we don't need to set the mark. Thanks for the feedback :)
Theo
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Fri, 02 Dec 2022 21:21:02 GMT)
Full text and
rfc822 format available.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Fri, 02 Dec 2022 22:29:02 GMT)
Full text and
rfc822 format available.
Message #94 received at 59662 <at> debbugs.gnu.org (full text, mbox):
On 02/12/2022 22:07, Theodor Thornhill via Bug reports for GNU Emacs,
the Swiss army knife of text editors wrote:
>
> On 2 December 2022 20:53:13 CET, Dmitry Gutov<dgutov <at> yandex.ru> wrote:
>> On 01/12/2022 09:13, Eli Zaretskii wrote:
>>> After all, until now this functionality and the "C-c C-q"
>>> binding was only in CC Mode.
>> There is also the 'M-q' binding in the popular paredit-mode.
>>
>> It runs paredit-reindent-defun.
> Yeah. My first iteration was piggybacking on fill-paragraph-function to get that binding, but I believe that was undesirable.
IME it's quite handy to have in modes where there is separation between
(rigid) code and strings/comments.
Not sure if that's all of prog-mode descendants, or if there will be
exceptions.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Sat, 03 Dec 2022 06:49:02 GMT)
Full text and
rfc822 format available.
Message #97 received at 59662 <at> debbugs.gnu.org (full text, mbox):
> Date: Sat, 3 Dec 2022 00:27:51 +0200
> Cc: 59662 <at> debbugs.gnu.org, casouri <at> gmail.org
> From: Dmitry Gutov <dgutov <at> yandex.ru>
>
> On 02/12/2022 22:07, Theodor Thornhill via Bug reports for GNU Emacs,
> the Swiss army knife of text editors wrote:
> >
> > On 2 December 2022 20:53:13 CET, Dmitry Gutov<dgutov <at> yandex.ru> wrote:
> >> On 01/12/2022 09:13, Eli Zaretskii wrote:
> >>> After all, until now this functionality and the "C-c C-q"
> >>> binding was only in CC Mode.
> >> There is also the 'M-q' binding in the popular paredit-mode.
> >>
> >> It runs paredit-reindent-defun.
> > Yeah. My first iteration was piggybacking on fill-paragraph-function to get that binding, but I believe that was undesirable.
>
> IME it's quite handy to have in modes where there is separation between
> (rigid) code and strings/comments.
>
> Not sure if that's all of prog-mode descendants, or if there will be
> exceptions.
I'm okay with making "C-c C-q" work in more modes than just C-related ones,
but please do that on master, not on the release branch.
As for M-q, it should be bound to a fill-SOMETHING command, not to a
indent-SOMETHING command. So from my POV paredit-mode should be fixed to
use some other sequence for paredit-reindent-defun, perhaps even "C-c C-q",
but that is IMO an unrelated issue.
And if you wanted to suggest that an indent-defun command in c-ts-mode be
bound to M-q, then I'm very much against that: M-q already has a useful
command bound to it in CC Mode, and should do something similar in
c-ts-mode; I believe we are discussing that in bug#59763.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Sat, 03 Dec 2022 16:00:02 GMT)
Full text and
rfc822 format available.
Message #100 received at 59662 <at> debbugs.gnu.org (full text, mbox):
On 03/12/2022 08:47, Eli Zaretskii wrote:
>> Date: Sat, 3 Dec 2022 00:27:51 +0200
>> Cc: 59662 <at> debbugs.gnu.org, casouri <at> gmail.org
>> From: Dmitry Gutov <dgutov <at> yandex.ru>
>>
>> On 02/12/2022 22:07, Theodor Thornhill via Bug reports for GNU Emacs,
>> the Swiss army knife of text editors wrote:
>>>
>>> On 2 December 2022 20:53:13 CET, Dmitry Gutov<dgutov <at> yandex.ru> wrote:
>>>> On 01/12/2022 09:13, Eli Zaretskii wrote:
>>>>> After all, until now this functionality and the "C-c C-q"
>>>>> binding was only in CC Mode.
>>>> There is also the 'M-q' binding in the popular paredit-mode.
>>>>
>>>> It runs paredit-reindent-defun.
>>> Yeah. My first iteration was piggybacking on fill-paragraph-function to get that binding, but I believe that was undesirable.
>>
>> IME it's quite handy to have in modes where there is separation between
>> (rigid) code and strings/comments.
>>
>> Not sure if that's all of prog-mode descendants, or if there will be
>> exceptions.
>
> I'm okay with making "C-c C-q" work in more modes than just C-related ones,
> but please do that on master, not on the release branch.
>
> As for M-q, it should be bound to a fill-SOMETHING command, not to a
> indent-SOMETHING command. So from my POV paredit-mode should be fixed to
> use some other sequence for paredit-reindent-defun, perhaps even "C-c C-q",
> but that is IMO an unrelated issue.
>
> And if you wanted to suggest that an indent-defun command in c-ts-mode be
> bound to M-q, then I'm very much against that: M-q already has a useful
> command bound to it in CC Mode, and should do something similar in
> c-ts-mode; I believe we are discussing that in bug#59763.
Both c-fill-paragraph and the proposed ts-c-fill-paragraph perform
fill-region when inside a string or comment. Otherwise, do nothing.
paredit-reindent-defun, like I described, does all that too. But when
outside strings and comments, it reindents the current defun.
AFAICT, these behaviors are quite compatible. But paredit's binding
seems to be useful in more contexts.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Sat, 03 Dec 2022 17:56:02 GMT)
Full text and
rfc822 format available.
Message #103 received at 59662 <at> debbugs.gnu.org (full text, mbox):
> Date: Sat, 3 Dec 2022 17:59:04 +0200
> Cc: theo <at> thornhill.no, casouri <at> gmail.com, 59662 <at> debbugs.gnu.org,
> casouri <at> gmail.org
> From: Dmitry Gutov <dgutov <at> yandex.ru>
>
> > As for M-q, it should be bound to a fill-SOMETHING command, not to a
> > indent-SOMETHING command. So from my POV paredit-mode should be fixed to
> > use some other sequence for paredit-reindent-defun, perhaps even "C-c C-q",
> > but that is IMO an unrelated issue.
> >
> > And if you wanted to suggest that an indent-defun command in c-ts-mode be
> > bound to M-q, then I'm very much against that: M-q already has a useful
> > command bound to it in CC Mode, and should do something similar in
> > c-ts-mode; I believe we are discussing that in bug#59763.
>
> Both c-fill-paragraph and the proposed ts-c-fill-paragraph perform
> fill-region when inside a string or comment. Otherwise, do nothing.
Yes.
> paredit-reindent-defun, like I described, does all that too. But when
> outside strings and comments, it reindents the current defun.
So maybe paredit-reindent-defun doesn't need any fixes, but using it as a
role model for the C-like languages is not TRT, since in C these are two
separate commands with two different key bindings.
> AFAICT, these behaviors are quite compatible. But paredit's binding
> seems to be useful in more contexts.
I'm fine with trying something like that on master, to see whether users
like this conflation, but not on the release branch.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Sun, 04 Dec 2022 12:34:01 GMT)
Full text and
rfc822 format available.
Message #106 received at submit <at> debbugs.gnu.org (full text, mbox):
Theodor Thornhill <theo <at> thornhill.no> writes:
> On 2 December 2022 22:09:55 CET, "Daniel Martín" <mardani29 <at> yahoo.es> wrote:
>>Theodor Thornhill <theo <at> thornhill.no> writes:
>>
>>> Right - thanks. However, this makes me wonder - should we really be
>>> setting mark here? I see that c-indent-defun does not, and it feels
>>> weird that indenting adds to the mark ring. What do you think? My
>>> first patch used
>>>
>>> (indent-region
>>> (treesit-node-start node)
>>> (treesit-node-end node))
>>>
>>> Which behaves similarly to c-indent-defun.
>>>
>>> See attached patch.
>>
>>I don't have a strong opinion, but if we can indent without setting the
>>mark, I think it'd be a cleaner command. Specially if c-indent-defun
>>doesn't set the mark either.
>
> Yeah, we don't need to set the mark. Thanks for the feedback :)
> Theo
Eli, do you have any opinion on this matter? Should we implement this
function without using marks? That will make the function a smidge
bigger, but would not mess up the mark-ring. I think that's desirable,
at least.
Theo
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Sun, 04 Dec 2022 12:34:02 GMT)
Full text and
rfc822 format available.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Sun, 04 Dec 2022 12:41:01 GMT)
Full text and
rfc822 format available.
Message #112 received at submit <at> debbugs.gnu.org (full text, mbox):
> From: Theodor Thornhill <theo <at> thornhill.no>
> Cc: "Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife
> of text editors" <bug-gnu-emacs <at> gnu.org>, Eli Zaretskii <eliz <at> gnu.org>,
> casouri <at> gmail.com, 59662 <at> debbugs.gnu.org
> Date: Sun, 04 Dec 2022 13:33:35 +0100
>
> Theodor Thornhill <theo <at> thornhill.no> writes:
>
> > On 2 December 2022 22:09:55 CET, "Daniel Martín" <mardani29 <at> yahoo.es> wrote:
> >>Theodor Thornhill <theo <at> thornhill.no> writes:
> >>
> >>> Right - thanks. However, this makes me wonder - should we really be
> >>> setting mark here? I see that c-indent-defun does not, and it feels
> >>> weird that indenting adds to the mark ring. What do you think? My
> >>> first patch used
> >>>
> >>> (indent-region
> >>> (treesit-node-start node)
> >>> (treesit-node-end node))
> >>>
> >>> Which behaves similarly to c-indent-defun.
> >>>
> >>> See attached patch.
> >>
> >>I don't have a strong opinion, but if we can indent without setting the
> >>mark, I think it'd be a cleaner command. Specially if c-indent-defun
> >>doesn't set the mark either.
> >
> > Yeah, we don't need to set the mark. Thanks for the feedback :)
> > Theo
>
> Eli, do you have any opinion on this matter? Should we implement this
> function without using marks? That will make the function a smidge
> bigger, but would not mess up the mark-ring. I think that's desirable,
> at least.
IMO, it is better not to set the mark, indeed.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Sun, 04 Dec 2022 12:41:02 GMT)
Full text and
rfc822 format available.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Wed, 07 Dec 2022 09:32:01 GMT)
Full text and
rfc822 format available.
Message #118 received at submit <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> gnu.org> writes:
>> From: Theodor Thornhill <theo <at> thornhill.no>
>> Cc: "Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife
>> of text editors" <bug-gnu-emacs <at> gnu.org>, Eli Zaretskii <eliz <at> gnu.org>,
>> casouri <at> gmail.com, 59662 <at> debbugs.gnu.org
>> Date: Sun, 04 Dec 2022 13:33:35 +0100
>>
>> Theodor Thornhill <theo <at> thornhill.no> writes:
>>
>> > On 2 December 2022 22:09:55 CET, "Daniel Martín" <mardani29 <at> yahoo.es> wrote:
>> >>Theodor Thornhill <theo <at> thornhill.no> writes:
>> >>
>> >>> Right - thanks. However, this makes me wonder - should we really be
>> >>> setting mark here? I see that c-indent-defun does not, and it feels
>> >>> weird that indenting adds to the mark ring. What do you think? My
>> >>> first patch used
>> >>>
>> >>> (indent-region
>> >>> (treesit-node-start node)
>> >>> (treesit-node-end node))
>> >>>
>> >>> Which behaves similarly to c-indent-defun.
>> >>>
>> >>> See attached patch.
>> >>
>> >>I don't have a strong opinion, but if we can indent without setting the
>> >>mark, I think it'd be a cleaner command. Specially if c-indent-defun
>> >>doesn't set the mark either.
>> >
>> > Yeah, we don't need to set the mark. Thanks for the feedback :)
>> > Theo
>>
>> Eli, do you have any opinion on this matter? Should we implement this
>> function without using marks? That will make the function a smidge
>> bigger, but would not mess up the mark-ring. I think that's desirable,
>> at least.
>
> IMO, it is better not to set the mark, indeed.
Agreed, see attached patch. What do you think, Yuan and Eli?
Should we expose the internal helper I used, or maybe this function
could just as well live in treesit.c?
Theo
[0001-Add-c-ts-mode-indent-defun.patch (text/x-diff, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Wed, 07 Dec 2022 09:32:02 GMT)
Full text and
rfc822 format available.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Wed, 07 Dec 2022 13:45:01 GMT)
Full text and
rfc822 format available.
Message #124 received at submit <at> debbugs.gnu.org (full text, mbox):
> From: Theodor Thornhill <theo <at> thornhill.no>
> Cc: mardani29 <at> yahoo.es, bug-gnu-emacs <at> gnu.org, casouri <at> gmail.com,
> 59662 <at> debbugs.gnu.org
> Date: Wed, 07 Dec 2022 10:31:13 +0100
>
> > IMO, it is better not to set the mark, indeed.
>
> Agreed, see attached patch. What do you think, Yuan and Eli?
>
> Should we expose the internal helper I used, or maybe this function
> could just as well live in treesit.c?
If it is expected to be useful outside treesit.el, I think it should
become public.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Wed, 07 Dec 2022 13:45:02 GMT)
Full text and
rfc822 format available.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Wed, 07 Dec 2022 19:55:01 GMT)
Full text and
rfc822 format available.
Message #130 received at 59662 <at> debbugs.gnu.org (full text, mbox):
Theodor Thornhill <theo <at> thornhill.no> writes:
> Eli Zaretskii <eliz <at> gnu.org> writes:
>
>>> From: Theodor Thornhill <theo <at> thornhill.no>
>>> Cc: "Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife
>>> of text editors" <bug-gnu-emacs <at> gnu.org>, Eli Zaretskii <eliz <at> gnu.org>,
>>> casouri <at> gmail.com, 59662 <at> debbugs.gnu.org
>>> Date: Sun, 04 Dec 2022 13:33:35 +0100
>>>
>>> Theodor Thornhill <theo <at> thornhill.no> writes:
>>>
>>> > On 2 December 2022 22:09:55 CET, "Daniel Martín" <mardani29 <at> yahoo.es> wrote:
>>> >>Theodor Thornhill <theo <at> thornhill.no> writes:
>>> >>
>>> >>> Right - thanks. However, this makes me wonder - should we really be
>>> >>> setting mark here? I see that c-indent-defun does not, and it feels
>>> >>> weird that indenting adds to the mark ring. What do you think? My
>>> >>> first patch used
>>> >>>
>>> >>> (indent-region
>>> >>> (treesit-node-start node)
>>> >>> (treesit-node-end node))
>>> >>>
>>> >>> Which behaves similarly to c-indent-defun.
>>> >>>
>>> >>> See attached patch.
>>> >>
>>> >>I don't have a strong opinion, but if we can indent without setting the
>>> >>mark, I think it'd be a cleaner command. Specially if c-indent-defun
>>> >>doesn't set the mark either.
>>> >
>>> > Yeah, we don't need to set the mark. Thanks for the feedback :)
>>> > Theo
>>>
>>> Eli, do you have any opinion on this matter? Should we implement this
>>> function without using marks? That will make the function a smidge
>>> bigger, but would not mess up the mark-ring. I think that's desirable,
>>> at least.
>>
>> IMO, it is better not to set the mark, indeed.
>
> Agreed, see attached patch. What do you think, Yuan and Eli?
>
> Should we expose the internal helper I used, or maybe this function
> could just as well live in treesit.c?
>
> Theo
>
Very nice, I applied this patch with some changes to the function. I
think using treesit-beginning/end-of-defun is more robust.
Yuan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Wed, 07 Dec 2022 20:07:02 GMT)
Full text and
rfc822 format available.
Message #133 received at 59662 <at> debbugs.gnu.org (full text, mbox):
Yuan Fu <casouri <at> gmail.com> writes:
> Theodor Thornhill <theo <at> thornhill.no> writes:
>
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>>
>>>> From: Theodor Thornhill <theo <at> thornhill.no>
>>>> Cc: "Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife
>>>> of text editors" <bug-gnu-emacs <at> gnu.org>, Eli Zaretskii <eliz <at> gnu.org>,
>>>> casouri <at> gmail.com, 59662 <at> debbugs.gnu.org
>>>> Date: Sun, 04 Dec 2022 13:33:35 +0100
>>>>
>>>> Theodor Thornhill <theo <at> thornhill.no> writes:
>>>>
>>>> > On 2 December 2022 22:09:55 CET, "Daniel Martín" <mardani29 <at> yahoo.es> wrote:
>>>> >>Theodor Thornhill <theo <at> thornhill.no> writes:
>>>> >>
>>>> >>> Right - thanks. However, this makes me wonder - should we really be
>>>> >>> setting mark here? I see that c-indent-defun does not, and it feels
>>>> >>> weird that indenting adds to the mark ring. What do you think? My
>>>> >>> first patch used
>>>> >>>
>>>> >>> (indent-region
>>>> >>> (treesit-node-start node)
>>>> >>> (treesit-node-end node))
>>>> >>>
>>>> >>> Which behaves similarly to c-indent-defun.
>>>> >>>
>>>> >>> See attached patch.
>>>> >>
>>>> >>I don't have a strong opinion, but if we can indent without setting the
>>>> >>mark, I think it'd be a cleaner command. Specially if c-indent-defun
>>>> >>doesn't set the mark either.
>>>> >
>>>> > Yeah, we don't need to set the mark. Thanks for the feedback :)
>>>> > Theo
>>>>
>>>> Eli, do you have any opinion on this matter? Should we implement this
>>>> function without using marks? That will make the function a smidge
>>>> bigger, but would not mess up the mark-ring. I think that's desirable,
>>>> at least.
>>>
>>> IMO, it is better not to set the mark, indeed.
>>
>> Agreed, see attached patch. What do you think, Yuan and Eli?
>>
>> Should we expose the internal helper I used, or maybe this function
>> could just as well live in treesit.c?
>>
>> Theo
>>
>
> Very nice, I applied this patch with some changes to the function. I
> think using treesit-beginning/end-of-defun is more robust.
>
> Yuan
Thanks! Would you mind describing why?
Theo
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Wed, 07 Dec 2022 21:41:02 GMT)
Full text and
rfc822 format available.
Message #136 received at 59662 <at> debbugs.gnu.org (full text, mbox):
> On Dec 7, 2022, at 12:06 PM, Theodor Thornhill <theo <at> thornhill.no> wrote:
>
> Yuan Fu <casouri <at> gmail.com> writes:
>
>> Theodor Thornhill <theo <at> thornhill.no> writes:
>>
>>> Eli Zaretskii <eliz <at> gnu.org> writes:
>>>
>>>>> From: Theodor Thornhill <theo <at> thornhill.no>
>>>>> Cc: "Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife
>>>>> of text editors" <bug-gnu-emacs <at> gnu.org>, Eli Zaretskii <eliz <at> gnu.org>,
>>>>> casouri <at> gmail.com, 59662 <at> debbugs.gnu.org
>>>>> Date: Sun, 04 Dec 2022 13:33:35 +0100
>>>>>
>>>>> Theodor Thornhill <theo <at> thornhill.no> writes:
>>>>>
>>>>>> On 2 December 2022 22:09:55 CET, "Daniel Martín" <mardani29 <at> yahoo.es> wrote:
>>>>>>> Theodor Thornhill <theo <at> thornhill.no> writes:
>>>>>>>
>>>>>>>> Right - thanks. However, this makes me wonder - should we really be
>>>>>>>> setting mark here? I see that c-indent-defun does not, and it feels
>>>>>>>> weird that indenting adds to the mark ring. What do you think? My
>>>>>>>> first patch used
>>>>>>>>
>>>>>>>> (indent-region
>>>>>>>> (treesit-node-start node)
>>>>>>>> (treesit-node-end node))
>>>>>>>>
>>>>>>>> Which behaves similarly to c-indent-defun.
>>>>>>>>
>>>>>>>> See attached patch.
>>>>>>>
>>>>>>> I don't have a strong opinion, but if we can indent without setting the
>>>>>>> mark, I think it'd be a cleaner command. Specially if c-indent-defun
>>>>>>> doesn't set the mark either.
>>>>>>
>>>>>> Yeah, we don't need to set the mark. Thanks for the feedback :)
>>>>>> Theo
>>>>>
>>>>> Eli, do you have any opinion on this matter? Should we implement this
>>>>> function without using marks? That will make the function a smidge
>>>>> bigger, but would not mess up the mark-ring. I think that's desirable,
>>>>> at least.
>>>>
>>>> IMO, it is better not to set the mark, indeed.
>>>
>>> Agreed, see attached patch. What do you think, Yuan and Eli?
>>>
>>> Should we expose the internal helper I used, or maybe this function
>>> could just as well live in treesit.c?
>>>
>>> Theo
>>>
>>
>> Very nice, I applied this patch with some changes to the function. I
>> think using treesit-beginning/end-of-defun is more robust.
>>
>> Yuan
>
> Thanks! Would you mind describing why?
Yes, sorry. I’m planning to change treesit-beginning-of-defun and treesit-end-of-defun so they can behave differently according to user configuration, basically they can decide whether to skip nested defuns or not. If we use these functions rather than re-implementing their logic (to some extent) in c-ts-mode-indent-defun, any improvement to defun navigation would automatically benefit c-ts-mode-indent-defun.
Yuan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Wed, 07 Dec 2022 21:54:02 GMT)
Full text and
rfc822 format available.
Message #139 received at 59662 <at> debbugs.gnu.org (full text, mbox):
On 7 December 2022 22:40:31 CET, Yuan Fu <casouri <at> gmail.com> wrote:
>
>
>> On Dec 7, 2022, at 12:06 PM, Theodor Thornhill <theo <at> thornhill.no> wrote:
>>
>> Yuan Fu <casouri <at> gmail.com> writes:
>>
>>> Theodor Thornhill <theo <at> thornhill.no> writes:
>>>
>>>> Eli Zaretskii <eliz <at> gnu.org> writes:
>>>>
>>>>>> From: Theodor Thornhill <theo <at> thornhill.no>
>>>>>> Cc: "Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife
>>>>>> of text editors" <bug-gnu-emacs <at> gnu.org>, Eli Zaretskii <eliz <at> gnu.org>,
>>>>>> casouri <at> gmail.com, 59662 <at> debbugs.gnu.org
>>>>>> Date: Sun, 04 Dec 2022 13:33:35 +0100
>>>>>>
>>>>>> Theodor Thornhill <theo <at> thornhill.no> writes:
>>>>>>
>>>>>>> On 2 December 2022 22:09:55 CET, "Daniel Martín" <mardani29 <at> yahoo.es> wrote:
>>>>>>>> Theodor Thornhill <theo <at> thornhill.no> writes:
>>>>>>>>
>>>>>>>>> Right - thanks. However, this makes me wonder - should we really be
>>>>>>>>> setting mark here? I see that c-indent-defun does not, and it feels
>>>>>>>>> weird that indenting adds to the mark ring. What do you think? My
>>>>>>>>> first patch used
>>>>>>>>>
>>>>>>>>> (indent-region
>>>>>>>>> (treesit-node-start node)
>>>>>>>>> (treesit-node-end node))
>>>>>>>>>
>>>>>>>>> Which behaves similarly to c-indent-defun.
>>>>>>>>>
>>>>>>>>> See attached patch.
>>>>>>>>
>>>>>>>> I don't have a strong opinion, but if we can indent without setting the
>>>>>>>> mark, I think it'd be a cleaner command. Specially if c-indent-defun
>>>>>>>> doesn't set the mark either.
>>>>>>>
>>>>>>> Yeah, we don't need to set the mark. Thanks for the feedback :)
>>>>>>> Theo
>>>>>>
>>>>>> Eli, do you have any opinion on this matter? Should we implement this
>>>>>> function without using marks? That will make the function a smidge
>>>>>> bigger, but would not mess up the mark-ring. I think that's desirable,
>>>>>> at least.
>>>>>
>>>>> IMO, it is better not to set the mark, indeed.
>>>>
>>>> Agreed, see attached patch. What do you think, Yuan and Eli?
>>>>
>>>> Should we expose the internal helper I used, or maybe this function
>>>> could just as well live in treesit.c?
>>>>
>>>> Theo
>>>>
>>>
>>> Very nice, I applied this patch with some changes to the function. I
>>> think using treesit-beginning/end-of-defun is more robust.
>>>
>>> Yuan
>>
>> Thanks! Would you mind describing why?
>
>Yes, sorry. I’m planning to change treesit-beginning-of-defun and treesit-end-of-defun so they can behave differently according to user configuration, basically they can decide whether to skip nested defuns or not. If we use these functions rather than re-implementing their logic (to some extent) in c-ts-mode-indent-defun, any improvement to defun navigation would automatically benefit c-ts-mode-indent-defun.
>
>Yuan
Thanks for explaining your rationale.
By the way, there's an issue with the patch you applied:
void
foo()
{
...
}
If point is at the v in void, the current implementation will not reinvent. I didn't get time to debug yet, though. Possibly that will be fixed after your rework as well.
Theo
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Wed, 07 Dec 2022 23:10:02 GMT)
Full text and
rfc822 format available.
Message #142 received at 59662 <at> debbugs.gnu.org (full text, mbox):
> On Dec 7, 2022, at 1:53 PM, Theodor Thornhill <theo <at> thornhill.no> wrote:
>
>
>
> On 7 December 2022 22:40:31 CET, Yuan Fu <casouri <at> gmail.com> wrote:
>>
>>
>>> On Dec 7, 2022, at 12:06 PM, Theodor Thornhill <theo <at> thornhill.no> wrote:
>>>
>>> Yuan Fu <casouri <at> gmail.com> writes:
>>>
>>>> Theodor Thornhill <theo <at> thornhill.no> writes:
>>>>
>>>>> Eli Zaretskii <eliz <at> gnu.org> writes:
>>>>>
>>>>>>> From: Theodor Thornhill <theo <at> thornhill.no>
>>>>>>> Cc: "Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife
>>>>>>> of text editors" <bug-gnu-emacs <at> gnu.org>, Eli Zaretskii <eliz <at> gnu.org>,
>>>>>>> casouri <at> gmail.com, 59662 <at> debbugs.gnu.org
>>>>>>> Date: Sun, 04 Dec 2022 13:33:35 +0100
>>>>>>>
>>>>>>> Theodor Thornhill <theo <at> thornhill.no> writes:
>>>>>>>
>>>>>>>> On 2 December 2022 22:09:55 CET, "Daniel Martín" <mardani29 <at> yahoo.es> wrote:
>>>>>>>>> Theodor Thornhill <theo <at> thornhill.no> writes:
>>>>>>>>>
>>>>>>>>>> Right - thanks. However, this makes me wonder - should we really be
>>>>>>>>>> setting mark here? I see that c-indent-defun does not, and it feels
>>>>>>>>>> weird that indenting adds to the mark ring. What do you think? My
>>>>>>>>>> first patch used
>>>>>>>>>>
>>>>>>>>>> (indent-region
>>>>>>>>>> (treesit-node-start node)
>>>>>>>>>> (treesit-node-end node))
>>>>>>>>>>
>>>>>>>>>> Which behaves similarly to c-indent-defun.
>>>>>>>>>>
>>>>>>>>>> See attached patch.
>>>>>>>>>
>>>>>>>>> I don't have a strong opinion, but if we can indent without setting the
>>>>>>>>> mark, I think it'd be a cleaner command. Specially if c-indent-defun
>>>>>>>>> doesn't set the mark either.
>>>>>>>>
>>>>>>>> Yeah, we don't need to set the mark. Thanks for the feedback :)
>>>>>>>> Theo
>>>>>>>
>>>>>>> Eli, do you have any opinion on this matter? Should we implement this
>>>>>>> function without using marks? That will make the function a smidge
>>>>>>> bigger, but would not mess up the mark-ring. I think that's desirable,
>>>>>>> at least.
>>>>>>
>>>>>> IMO, it is better not to set the mark, indeed.
>>>>>
>>>>> Agreed, see attached patch. What do you think, Yuan and Eli?
>>>>>
>>>>> Should we expose the internal helper I used, or maybe this function
>>>>> could just as well live in treesit.c?
>>>>>
>>>>> Theo
>>>>>
>>>>
>>>> Very nice, I applied this patch with some changes to the function. I
>>>> think using treesit-beginning/end-of-defun is more robust.
>>>>
>>>> Yuan
>>>
>>> Thanks! Would you mind describing why?
>>
>> Yes, sorry. I’m planning to change treesit-beginning-of-defun and treesit-end-of-defun so they can behave differently according to user configuration, basically they can decide whether to skip nested defuns or not. If we use these functions rather than re-implementing their logic (to some extent) in c-ts-mode-indent-defun, any improvement to defun navigation would automatically benefit c-ts-mode-indent-defun.
>>
>> Yuan
>
> Thanks for explaining your rationale.
>
> By the way, there's an issue with the patch you applied:
>
> void
> foo()
> {
> ...
> }
>
> If point is at the v in void, the current implementation will not reinvent. I didn't get time to debug yet, though. Possibly that will be fixed after your rework as well.
Ah, thanks, I’ll fix that. And I’ll ask for your opinion before making changes next time, however innocent I thought the change is :-)
Yuan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Thu, 08 Dec 2022 01:39:02 GMT)
Full text and
rfc822 format available.
Message #145 received at 59662 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
On 03/12/2022 19:54, Eli Zaretskii wrote:
>> paredit-reindent-defun, like I described, does all that too. But when
>> outside strings and comments, it reindents the current defun.
> So maybe paredit-reindent-defun doesn't need any fixes, but using it as a
> role model for the C-like languages is not TRT, since in C these are two
> separate commands with two different key bindings.
It's not a given that the current state of things must automatically be
optimal. But of course I can't vouch whether this behavior is optimal
for every language.
And major modes will continue to be able to rebind these key sequences.
>> AFAICT, these behaviors are quite compatible. But paredit's binding
>> seems to be useful in more contexts.
> I'm fine with trying something like that on master, to see whether users
> like this conflation, but not on the release branch.
Here's a patch that people can try. From my brief testing, seems useful
enough. I suppose some might dislike it, though, in some codebases where
the methods/functions are consistently longer than a screenful. I do
like this binding in Emacs Lisp too, though.
[prog-reindent-defun.diff (text/x-patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Thu, 08 Dec 2022 06:04:03 GMT)
Full text and
rfc822 format available.
Message #148 received at 59662 <at> debbugs.gnu.org (full text, mbox):
Would you mind describing why?
>>>
>>> Yes, sorry. I’m planning to change treesit-beginning-of-defun and treesit-end-of-defun so they can behave differently according to user configuration, basically they can decide whether to skip nested defuns or not. If we use these functions rather than re-implementing their logic (to some extent) in c-ts-mode-indent-defun, any improvement to defun navigation would automatically benefit c-ts-mode-indent-defun.
>>>
>>> Yuan
>>
>> Thanks for explaining your rationale.
>>
>> By the way, there's an issue with the patch you applied:
>>
>> void
>> foo()
>> {
>> ...
>> }
>>
>> If point is at the v in void, the current implementation will not reinvent. I didn't get time to debug yet, though. Possibly that will be fixed after your rework as well.
>
>Ah, thanks, I’ll fix that. And I’ll ask for your opinion before making changes next time, however innocent I thought the change is :-)
>
>Yuan
No stress, just wanted to point it out, but thanks!
Theo
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Thu, 08 Dec 2022 07:17:02 GMT)
Full text and
rfc822 format available.
Message #151 received at 59662 <at> debbugs.gnu.org (full text, mbox):
> Date: Thu, 8 Dec 2022 03:37:54 +0200
> Cc: theo <at> thornhill.no, casouri <at> gmail.com, 59662 <at> debbugs.gnu.org,
> casouri <at> gmail.org
> From: Dmitry Gutov <dgutov <at> yandex.ru>
>
> >> AFAICT, these behaviors are quite compatible. But paredit's binding
> >> seems to be useful in more contexts.
> > I'm fine with trying something like that on master, to see whether users
> > like this conflation, but not on the release branch.
>
> Here's a patch that people can try. From my brief testing, seems useful
> enough. I suppose some might dislike it, though, in some codebases where
> the methods/functions are consistently longer than a screenful. I do
> like this binding in Emacs Lisp too, though.
Stefan, Lars: any comments?
I'm okay with installing this on master, but please rename the new
function to something like prog-fill-reindent; the "fill" part should
appear in the name, to explain why we have both prog-indent and this
function.
Also, a NEWS entry and some addition to the "Multi-line Indent" node
of the Emacs user manual would be nice.
Bonus points for adding something that could use treesit.el
capabilities, when those are available, instead of syntax-ppss, to
determine when we are in a comment or in a string. Or maybe Theo or
Yuan could suggest such an addition.
Thanks.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Thu, 08 Dec 2022 08:12:02 GMT)
Full text and
rfc822 format available.
Message #154 received at 59662 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
>> Date: Thu, 8 Dec 2022 03:37:54 +0200
>> Cc: theo <at> thornhill.no, casouri <at> gmail.com, 59662 <at> debbugs.gnu.org,
>> casouri <at> gmail.org
>> From: Dmitry Gutov <dgutov <at> yandex.ru>
>>
>> >> AFAICT, these behaviors are quite compatible. But paredit's binding
>> >> seems to be useful in more contexts.
>> > I'm fine with trying something like that on master, to see whether users
>> > like this conflation, but not on the release branch.
>>
>> Here's a patch that people can try. From my brief testing, seems useful
>> enough. I suppose some might dislike it, though, in some codebases where
>> the methods/functions are consistently longer than a screenful. I do
>> like this binding in Emacs Lisp too, though.
>
> Stefan, Lars: any comments?
>
> I'm okay with installing this on master, but please rename the new
> function to something like prog-fill-reindent; the "fill" part should
> appear in the name, to explain why we have both prog-indent and this
> function.
>
> Also, a NEWS entry and some addition to the "Multi-line Indent" node
> of the Emacs user manual would be nice.
>
> Bonus points for adding something that could use treesit.el
> capabilities, when those are available, instead of syntax-ppss, to
> determine when we are in a comment or in a string. Or maybe Theo or
> Yuan could suggest such an addition.
>
This works perfectly as is in *-ts-modes. At least for modes where
syntax-table is set correctly. I don't think there's a reason to enable
treesit.el capabilities, unless there's a case where this doesn't work.
If this ends up in master, should I backport some similar feature for
the *ts-modes?
Theo
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Thu, 08 Dec 2022 08:29:01 GMT)
Full text and
rfc822 format available.
Message #157 received at 59662 <at> debbugs.gnu.org (full text, mbox):
> From: Theodor Thornhill <theo <at> thornhill.no>
> Cc: casouri <at> gmail.com, 59662 <at> debbugs.gnu.org, casouri <at> gmail.org
> Date: Thu, 08 Dec 2022 09:11:28 +0100
>
> > Bonus points for adding something that could use treesit.el
> > capabilities, when those are available, instead of syntax-ppss, to
> > determine when we are in a comment or in a string. Or maybe Theo or
> > Yuan could suggest such an addition.
>
> This works perfectly as is in *-ts-modes. At least for modes where
> syntax-table is set correctly. I don't think there's a reason to enable
> treesit.el capabilities, unless there's a case where this doesn't work.
My point is that tree-sitter in many cases parses the program better
than syntax-ppss. So if we can use its information about comments and
strings, why not do that? It's exactly the same logic that Dmitry
used:
> + (if (or (nth 8 (syntax-ppss))
> + (re-search-forward comment-start-skip (line-end-position) t))
which falls back on regexp search if syntax-ppss cannot help. I'm
asking why not try tree-sitter before we fall back on syntax-ppss or
regexp search?
Does that make sense? If not, why not?
> If this ends up in master, should I backport some similar feature for
> the *ts-modes?
Why do we need this on the release branch? A general
indent-or-fill-comment feature for all the progmodes didn't exist
until now, which is why we intend to install this on master.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Thu, 08 Dec 2022 08:57:02 GMT)
Full text and
rfc822 format available.
Message #160 received at 59662 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
>> From: Theodor Thornhill <theo <at> thornhill.no>
>> Cc: casouri <at> gmail.com, 59662 <at> debbugs.gnu.org, casouri <at> gmail.org
>> Date: Thu, 08 Dec 2022 09:11:28 +0100
>>
>> > Bonus points for adding something that could use treesit.el
>> > capabilities, when those are available, instead of syntax-ppss, to
>> > determine when we are in a comment or in a string. Or maybe Theo or
>> > Yuan could suggest such an addition.
>>
>> This works perfectly as is in *-ts-modes. At least for modes where
>> syntax-table is set correctly. I don't think there's a reason to enable
>> treesit.el capabilities, unless there's a case where this doesn't work.
>
> My point is that tree-sitter in many cases parses the program better
> than syntax-ppss. So if we can use its information about comments and
> strings, why not do that? It's exactly the same logic that Dmitry
> used:
>
Something like this? If we add a defvar such as the below, similar to
treesit-defun-type-regexp we can use that in the function below:
```
(defvar-local treesit-comment-type-regexp nil
"A regexp that matches the node type of comment nodes.
For example, \"(line|block)_comment\". ")
(defun prog-reindent-defun (&optional argument)
"Refill paragraph or reindent the definition that the point is on.
If the point is in a string, or in a comment, or there is a
comment on the current line, fill the paragraph that the point is
in or is on the same line.
Otherwise, reindent the definition around or below point."
(interactive "P")
(save-excursion
(if (or (and (treesit-available-p)
(treesit-ready-p (treesit-language-at (point)))
(string-match-p
treesit-comment-type-regexp
(treesit-node-type (treesit-node-at (point)))))
(nth 8 (syntax-ppss))
(re-search-forward comment-start-skip (line-end-position) t))
(if (memq fill-paragraph-function '(t nil))
(lisp-fill-paragraph argument)
(funcall fill-paragraph-function argument))
(end-of-defun)
(let ((end (point)))
(beginning-of-defun)
(indent-region (point) end)))))
```
The error handling here is naive, but is this something in the line of
what you're suggesting? If so, I can tweak the error handling and
create a patch for it? Unless you want to do that, Dmitry
>> + (if (or (nth 8 (syntax-ppss))
>> + (re-search-forward comment-start-skip (line-end-position) t))
>
> which falls back on regexp search if syntax-ppss cannot help. I'm
> asking why not try tree-sitter before we fall back on syntax-ppss or
> regexp search?
>
> Does that make sense? If not, why not?
>
It does.
>> If this ends up in master, should I backport some similar feature for
>> the *ts-modes?
>
> Why do we need this on the release branch? A general
> indent-or-fill-comment feature for all the progmodes didn't exist
> until now, which is why we intend to install this on master.
Yeah, sure :-)
Theo
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Thu, 08 Dec 2022 09:09:01 GMT)
Full text and
rfc822 format available.
Message #163 received at 59662 <at> debbugs.gnu.org (full text, mbox):
Resending because I removed the wrong email to Yuan...
Theodor Thornhill <theo <at> thornhill.no> writes:
> Eli Zaretskii <eliz <at> gnu.org> writes:
>
>>> From: Theodor Thornhill <theo <at> thornhill.no>
>>> Cc: casouri <at> gmail.com, 59662 <at> debbugs.gnu.org, casouri <at> gmail.org
>>> Date: Thu, 08 Dec 2022 09:11:28 +0100
>>>
>>> > Bonus points for adding something that could use treesit.el
>>> > capabilities, when those are available, instead of syntax-ppss, to
>>> > determine when we are in a comment or in a string. Or maybe Theo or
>>> > Yuan could suggest such an addition.
>>>
>>> This works perfectly as is in *-ts-modes. At least for modes where
>>> syntax-table is set correctly. I don't think there's a reason to enable
>>> treesit.el capabilities, unless there's a case where this doesn't work.
>>
>> My point is that tree-sitter in many cases parses the program better
>> than syntax-ppss. So if we can use its information about comments and
>> strings, why not do that? It's exactly the same logic that Dmitry
>> used:
>>
>
> Something like this? If we add a defvar such as the below, similar to
> treesit-defun-type-regexp we can use that in the function below:
>
> ```
> (defvar-local treesit-comment-type-regexp nil
> "A regexp that matches the node type of comment nodes.
>
> For example, \"(line|block)_comment\". ")
>
>
> (defun prog-reindent-defun (&optional argument)
> "Refill paragraph or reindent the definition that the point is on.
>
> If the point is in a string, or in a comment, or there is a
> comment on the current line, fill the paragraph that the point is
> in or is on the same line.
>
> Otherwise, reindent the definition around or below point."
> (interactive "P")
> (save-excursion
> (if (or (and (treesit-available-p)
> (treesit-ready-p (treesit-language-at (point)))
> (string-match-p
> treesit-comment-type-regexp
> (treesit-node-type (treesit-node-at (point)))))
> (nth 8 (syntax-ppss))
> (re-search-forward comment-start-skip (line-end-position) t))
> (if (memq fill-paragraph-function '(t nil))
> (lisp-fill-paragraph argument)
> (funcall fill-paragraph-function argument))
> (end-of-defun)
> (let ((end (point)))
> (beginning-of-defun)
> (indent-region (point) end)))))
> ```
>
> The error handling here is naive, but is this something in the line of
> what you're suggesting? If so, I can tweak the error handling and
> create a patch for it? Unless you want to do that, Dmitry
>
>>> + (if (or (nth 8 (syntax-ppss))
>>> + (re-search-forward comment-start-skip (line-end-position) t))
>>
>> which falls back on regexp search if syntax-ppss cannot help. I'm
>> asking why not try tree-sitter before we fall back on syntax-ppss or
>> regexp search?
>>
>> Does that make sense? If not, why not?
>>
>
> It does.
>
>>> If this ends up in master, should I backport some similar feature for
>>> the *ts-modes?
>>
>> Why do we need this on the release branch? A general
>> indent-or-fill-comment feature for all the progmodes didn't exist
>> until now, which is why we intend to install this on master.
>
> Yeah, sure :-)
>
> Theo
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Thu, 08 Dec 2022 10:40:02 GMT)
Full text and
rfc822 format available.
Message #166 received at 59662 <at> debbugs.gnu.org (full text, mbox):
> From: Theodor Thornhill <theo <at> thornhill.no>
> Cc: dgutov <at> yandex.ru, monnier <at> iro.umontreal.ca, larsi <at> gnus.org,
> 59662 <at> debbugs.gnu.org, casouri <at> gmail.org
> Date: Thu, 08 Dec 2022 09:56:44 +0100
>
> Eli Zaretskii <eliz <at> gnu.org> writes:
>
> > My point is that tree-sitter in many cases parses the program better
> > than syntax-ppss. So if we can use its information about comments and
> > strings, why not do that? It's exactly the same logic that Dmitry
> > used:
> >
>
> Something like this? If we add a defvar such as the below, similar to
> treesit-defun-type-regexp we can use that in the function below:
>
> ```
> (defvar-local treesit-comment-type-regexp nil
> "A regexp that matches the node type of comment nodes.
>
> For example, \"(line|block)_comment\". ")
The intent is that major modes set this variable? SGTM.
> (if (or (and (treesit-available-p)
> (treesit-ready-p (treesit-language-at (point)))
I'd imagine that this kind of test should have an API, so the
treesit-ready-p call should not be made explicitly? Yuan, WDYT?
Also, isn't it enough to check whether the buffer has a tree-sitter
parser or something?
> The error handling here is naive, but is this something in the line of
> what you're suggesting?
Yes, it is what I had in mind, but see above.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Thu, 08 Dec 2022 11:10:02 GMT)
Full text and
rfc822 format available.
Message #169 received at 59662 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
>> From: Theodor Thornhill <theo <at> thornhill.no>
>> Cc: dgutov <at> yandex.ru, monnier <at> iro.umontreal.ca, larsi <at> gnus.org,
>> 59662 <at> debbugs.gnu.org, casouri <at> gmail.org
>> Date: Thu, 08 Dec 2022 09:56:44 +0100
>>
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>>
>> > My point is that tree-sitter in many cases parses the program better
>> > than syntax-ppss. So if we can use its information about comments and
>> > strings, why not do that? It's exactly the same logic that Dmitry
>> > used:
>> >
>>
>> Something like this? If we add a defvar such as the below, similar to
>> treesit-defun-type-regexp we can use that in the function below:
>>
>> ```
>> (defvar-local treesit-comment-type-regexp nil
>> "A regexp that matches the node type of comment nodes.
>>
>> For example, \"(line|block)_comment\". ")
>
> The intent is that major modes set this variable? SGTM.
Yeah, that was the idea.
>
>> (if (or (and (treesit-available-p)
>> (treesit-ready-p (treesit-language-at (point)))
>
> I'd imagine that this kind of test should have an API, so the
> treesit-ready-p call should not be made explicitly? Yuan, WDYT?
>
> Also, isn't it enough to check whether the buffer has a tree-sitter
> parser or something?
>
>> The error handling here is naive, but is this something in the line of
>> what you're suggesting?
>
> Yes, it is what I had in mind, but see above.
What about this? I changed it to make a point that we should fill if
inside of a string or comment, and indent if inside a code block. We
one such variable for each type, or in both as suggested below.
```
(defvar-local treesit-text-type-regexp nil
"A regexp that matches the node type of textual nodes.
For example, \"(line|block)_comment\" in the case of a comment,
or \"string_literal\" in the case of a string.")
(defun prog-reindent-defun (&optional argument)
"Refill paragraph or reindent the definition that the point is on.
If the point is in a string, or in a comment, or there is a
comment on the current line, fill the paragraph that the point is
in or is on the same line.
Otherwise, reindent the definition around or below point."
(interactive "P")
(save-excursion
(if (or (and (treesit-parser-list)
(string-match-p
treesit-text-type-regexp
(treesit-node-type (treesit-node-at (point)))))
(nth 8 (syntax-ppss))
(re-search-forward comment-start-skip (line-end-position) t))
(if (memq fill-paragraph-function '(t nil))
(lisp-fill-paragraph argument)
(funcall fill-paragraph-function argument))
(end-of-defun)
(let ((end (point)))
(beginning-of-defun)
(indent-region (point) end)))))
```
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Thu, 08 Dec 2022 11:39:01 GMT)
Full text and
rfc822 format available.
Message #172 received at 59662 <at> debbugs.gnu.org (full text, mbox):
> From: Theodor Thornhill <theo <at> thornhill.no>
> Cc: dgutov <at> yandex.ru, monnier <at> iro.umontreal.ca, larsi <at> gnus.org,
> 59662 <at> debbugs.gnu.org, casouri <at> gmail.com
> Date: Thu, 08 Dec 2022 12:09:18 +0100
>
> What about this? I changed it to make a point that we should fill if
> inside of a string or comment, and indent if inside a code block. We
> one such variable for each type, or in both as suggested below.
I'll defer to Dmitry on that.
Thanks.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Thu, 08 Dec 2022 14:19:02 GMT)
Full text and
rfc822 format available.
Message #175 received at 59662 <at> debbugs.gnu.org (full text, mbox):
On 08/12/2022 10:56, Theodor Thornhill wrote:
> (if (or (and (treesit-available-p)
> (treesit-ready-p (treesit-language-at (point)))
> (string-match-p
> treesit-comment-type-regexp
> (treesit-node-type (treesit-node-at (point)))))
I was kind of hoping that sometime in the near future our tree-sitter
code would somehow plug into syntax-ppss (maybe via advice or etc) to
provide the same results faster.
Or that we'd add similar helpers which would dispatch to either
syntax-ppss or tree-sitter, depending on whether the latter is enabled
and working.
The first approach will be more compatible for third-party code to use
right now; the second will probably be more powerful. We might even do both.
As a result, a lot of functions won't have to reimplement the quoted
part. Inside, and (more importantly) outside Emacs core.
So my preference would be to check in the implementation as-is (if
everybody agrees with the behavior), and then migrate it to new helpers
(in case plugging into syntax-ppss won't be automatic).
But I don't really mind the code you posted, if somebody (e.g. Eli) is
going to insist on it. It just doesn't seem critical from the
performance POV, and the variable treesit-comment-type-regexp seems to
be a little out of place. But then again, if the helpers will be put in
prog-mode.el, the var will be there as well.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Thu, 08 Dec 2022 14:23:01 GMT)
Full text and
rfc822 format available.
Message #178 received at 59662 <at> debbugs.gnu.org (full text, mbox):
On 08/12/2022 13:09, Theodor Thornhill wrote:
> What about this? I changed it to make a point that we should fill if
> inside of a string or comment, and indent if inside a code block. We
> one such variable for each type, or in both as suggested below.
Could you rephrase maybe? I'm not seeing the difference.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Thu, 08 Dec 2022 14:25:02 GMT)
Full text and
rfc822 format available.
Message #181 received at 59662 <at> debbugs.gnu.org (full text, mbox):
On 08/12/2022 09:16, Eli Zaretskii wrote:
> I'm okay with installing this on master, but please rename the new
> function to something like prog-fill-reindent; the "fill" part should
> appear in the name, to explain why we have both prog-indent and this
> function.
Sure.
prog-indent-sexp is fairly different, though, so it wouldn't be out of
the question to have both commands, even if prog-reindent-defun didn't
refill.
> Also, a NEWS entry and some addition to the "Multi-line Indent" node
> of the Emacs user manual would be nice.
I suppose.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Thu, 08 Dec 2022 14:55:01 GMT)
Full text and
rfc822 format available.
Message #184 received at 59662 <at> debbugs.gnu.org (full text, mbox):
> Date: Thu, 8 Dec 2022 16:18:17 +0200
> Cc: monnier <at> iro.umontreal.ca, larsi <at> gnus.org, 59662 <at> debbugs.gnu.org,
> casouri <at> gmail.org
> From: Dmitry Gutov <dgutov <at> yandex.ru>
>
> On 08/12/2022 10:56, Theodor Thornhill wrote:
> > (if (or (and (treesit-available-p)
> > (treesit-ready-p (treesit-language-at (point)))
> > (string-match-p
> > treesit-comment-type-regexp
> > (treesit-node-type (treesit-node-at (point)))))
>
> I was kind of hoping that sometime in the near future our tree-sitter
> code would somehow plug into syntax-ppss (maybe via advice or etc) to
> provide the same results faster.
It is too early to consider practical code along these lines, we don't
have enough experience with tree-sitter. I'm not even sure it can be
easily factored into the same categories as syntax-ppss uses.
IMO, the way to move to a unified handling is to use tree-sitter
wherever we can, originally as special-cased code, and later refactor
that into generalized APIs, when we know more about how to define and
use them.
> So my preference would be to check in the implementation as-is (if
> everybody agrees with the behavior), and then migrate it to new helpers
> (in case plugging into syntax-ppss won't be automatic).
>
> But I don't really mind the code you posted, if somebody (e.g. Eli) is
> going to insist on it. It just doesn't seem critical from the
> performance POV, and the variable treesit-comment-type-regexp seems to
> be a little out of place. But then again, if the helpers will be put in
> prog-mode.el, the var will be there as well.
I don't insist, but it sounds like the logical thing to do. So unless
you feel strongly against that, I'd say let's add that part. This is,
after all, an experiment, so let's experiment.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Thu, 08 Dec 2022 14:59:01 GMT)
Full text and
rfc822 format available.
Message #187 received at 59662 <at> debbugs.gnu.org (full text, mbox):
Dmitry Gutov <dgutov <at> yandex.ru> writes:
> On 08/12/2022 13:09, Theodor Thornhill wrote:
>> What about this? I changed it to make a point that we should fill if
>> inside of a string or comment, and indent if inside a code block. We
>> one such variable for each type, or in both as suggested below.
>
> Could you rephrase maybe? I'm not seeing the difference.
In the first example I made the variable specific to comments. But
filling should work on everything that's considered text, IIUC. So if
the variable refers to both the regex needs to accommodate strings and
comments. That will in turn make it less useful if we want to find
comments specifically, or strings specifically. so we could have both
'treesit-comment-regexp' and 'treesit-string-regexp' and match on '(or
treesit-comment-regexp treesit-string-regexp)' in this particular case.
Not really sure if that made more sense.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Thu, 08 Dec 2022 15:39:02 GMT)
Full text and
rfc822 format available.
Message #190 received at 59662 <at> debbugs.gnu.org (full text, mbox):
On 08/12/2022 16:24, Dmitry Gutov wrote:
> On 08/12/2022 09:16, Eli Zaretskii wrote:
>> I'm okay with installing this on master, but please rename the new
>> function to something like prog-fill-reindent; the "fill" part should
>> appear in the name, to explain why we have both prog-indent and this
>> function.
>
> Sure.
I think we'll need to cram the word "defun" in the name too, won't we?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Thu, 08 Dec 2022 15:45:02 GMT)
Full text and
rfc822 format available.
Message #193 received at 59662 <at> debbugs.gnu.org (full text, mbox):
On 08/12/2022 16:53, Eli Zaretskii wrote:
>> Date: Thu, 8 Dec 2022 16:18:17 +0200
>> Cc: monnier <at> iro.umontreal.ca, larsi <at> gnus.org, 59662 <at> debbugs.gnu.org,
>> casouri <at> gmail.org
>> From: Dmitry Gutov <dgutov <at> yandex.ru>
>>
>> On 08/12/2022 10:56, Theodor Thornhill wrote:
>>> (if (or (and (treesit-available-p)
>>> (treesit-ready-p (treesit-language-at (point)))
>>> (string-match-p
>>> treesit-comment-type-regexp
>>> (treesit-node-type (treesit-node-at (point)))))
>>
>> I was kind of hoping that sometime in the near future our tree-sitter
>> code would somehow plug into syntax-ppss (maybe via advice or etc) to
>> provide the same results faster.
>
> It is too early to consider practical code along these lines, we don't
> have enough experience with tree-sitter. I'm not even sure it can be
> easily factored into the same categories as syntax-ppss uses.
I'm fairly certain that tree-sitter contexts could be "dumbed down" into
syntax-ppss categories.
But that's not the only way: another is to just use tree-sitter to find
a "safe place" to call parse-partial-sexp from. That would give us the
speed bump together with 100% backward compatibility.
> IMO, the way to move to a unified handling is to use tree-sitter
> wherever we can, originally as special-cased code, and later refactor
> that into generalized APIs, when we know more about how to define and
> use them.
syntax-ppss is used in a lot of places. A "free" improvement like the
above wouldn't hurt.
>> So my preference would be to check in the implementation as-is (if
>> everybody agrees with the behavior), and then migrate it to new helpers
>> (in case plugging into syntax-ppss won't be automatic).
>>
>> But I don't really mind the code you posted, if somebody (e.g. Eli) is
>> going to insist on it. It just doesn't seem critical from the
>> performance POV, and the variable treesit-comment-type-regexp seems to
>> be a little out of place. But then again, if the helpers will be put in
>> prog-mode.el, the var will be there as well.
>
> I don't insist, but it sounds like the logical thing to do. So unless
> you feel strongly against that, I'd say let's add that part. This is,
> after all, an experiment, so let's experiment.
I'm not going to object.
Doubly so if somebody else wants to finish up the patch and commit it
with NEWS and documentation updates. ;-)
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Thu, 08 Dec 2022 15:48:02 GMT)
Full text and
rfc822 format available.
Message #196 received at 59662 <at> debbugs.gnu.org (full text, mbox):
> Date: Thu, 8 Dec 2022 17:37:55 +0200
> From: Dmitry Gutov <dgutov <at> yandex.ru>
> Cc: casouri <at> gmail.com, 59662 <at> debbugs.gnu.org, theo <at> thornhill.no,
> casouri <at> gmail.org
>
> On 08/12/2022 16:24, Dmitry Gutov wrote:
> > On 08/12/2022 09:16, Eli Zaretskii wrote:
> >> I'm okay with installing this on master, but please rename the new
> >> function to something like prog-fill-reindent; the "fill" part should
> >> appear in the name, to explain why we have both prog-indent and this
> >> function.
> >
> > Sure.
>
> I think we'll need to cram the word "defun" in the name too, won't we?
I won't object to that.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Thu, 08 Dec 2022 16:07:01 GMT)
Full text and
rfc822 format available.
Message #199 received at 59662 <at> debbugs.gnu.org (full text, mbox):
On 08/12/2022 16:58, Theodor Thornhill via Bug reports for GNU Emacs,
the Swiss army knife of text editors wrote:
> Dmitry Gutov<dgutov <at> yandex.ru> writes:
>
>> On 08/12/2022 13:09, Theodor Thornhill wrote:
>>> What about this? I changed it to make a point that we should fill if
>>> inside of a string or comment, and indent if inside a code block. We
>>> one such variable for each type, or in both as suggested below.
>> Could you rephrase maybe? I'm not seeing the difference.
> In the first example I made the variable specific to comments. But
> filling should work on everything that's considered text, IIUC. So if
> the variable refers to both the regex needs to accommodate strings and
> comments. That will in turn make it less useful if we want to find
> comments specifically, or strings specifically. so we could have both
> 'treesit-comment-regexp' and 'treesit-string-regexp' and match on '(or
> treesit-comment-regexp treesit-string-regexp)' in this particular case.
>
> Not really sure if that made more sense.
Ah. So that was about the name of the variable. Looks good.
Note that if the improvement which I described ever comes to life
(hopefully), then the variable will probably need to be
renamed/moved/multiplied again.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Thu, 08 Dec 2022 16:09:02 GMT)
Full text and
rfc822 format available.
Message #202 received at 59662 <at> debbugs.gnu.org (full text, mbox):
>
>syntax-ppss is used in a lot of places. A "free" improvement like the above wouldn't hurt.
>
>>> So my preference would be to check in the , the var will be there as well.
>>
>> I don't insist, but it sounds like the logical thing to do. So unless
>> you feel strongly against that, I'd say let's add that part. This is,
>> after all, an experiment, so let's experiment.
>
>I'm not going to object.
>
>Doubly so if somebody else wants to finish up the patch and commit it with NEWS and documentation updates. ;-)
I can finish up the patch if you don't want to :)
Theo
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Thu, 08 Dec 2022 16:24:02 GMT)
Full text and
rfc822 format available.
Message #205 received at 59662 <at> debbugs.gnu.org (full text, mbox):
On 08/12/2022 18:04, Theodor Thornhill via Bug reports for GNU Emacs,
the Swiss army knife of text editors wrote:
>
>
>> syntax-ppss is used in a lot of places. A "free" improvement like the above wouldn't hurt.
>>
>>>> So my preference would be to check in the , the var will be there as well.
>>> I don't insist, but it sounds like the logical thing to do. So unless
>>> you feel strongly against that, I'd say let's add that part. This is,
>>> after all, an experiment, so let's experiment.
>> I'm not going to object.
>>
>> Doubly so if somebody else wants to finish up the patch and commit it with NEWS and documentation updates. 😉
> I can finish up the patch if you don't want to 😄
If you don't mind. Thank you. :-)
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Thu, 08 Dec 2022 16:42:02 GMT)
Full text and
rfc822 format available.
Message #208 received at 59662 <at> debbugs.gnu.org (full text, mbox):
On 8 December 2022 17:06:26 CET, Dmitry Gutov <dgutov <at> yandex.ru> wrote:
>On 08/12/2022 16:58, Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors wrote:
>> Dmitry Gutov<dgutov <at> yandex.ru> writes:
>>
>>> On 08/12/2022 13:09, Theodor Thornhill wrote:
>>>> What about this? I changed it to make a point that we should fill if
>>>> inside of a string or comment, and indent if inside a code block. We
>>>> one such variable for each type, or in both as suggested below.
>>> Could you rephrase maybe? I'm not seeing the difference.
>> In the first example I made the variable specific to comments. But
>> filling should work on everything that's considered text, IIUC. So if
>> the variable refers to both the regex needs to accommodate strings and
>> comments. That will in turn make it less useful if we want to find
>> comments specifically, or strings specifically. so we could have both
>> 'treesit-comment-regexp' and 'treesit-string-regexp' and match on '(or
>> treesit-comment-regexp treesit-string-regexp)' in this particular case.
>>
>> Not really sure if that made more sense.
>
>Ah. So that was about the name of the variable. Looks good.
>
Yeah, and thus what nodes it should capture
>Note that if the improvement which I described ever comes to life (hopefully), then the variable will probably need to be renamed/moved/multiplied again.
Why is that?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Thu, 08 Dec 2022 19:16:02 GMT)
Full text and
rfc822 format available.
Message #211 received at 59662 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Dmitry Gutov <dgutov <at> yandex.ru> writes:
> On 08/12/2022 18:04, Theodor Thornhill via Bug reports for GNU Emacs,
> the Swiss army knife of text editors wrote:
>>
>>
>>> syntax-ppss is used in a lot of places. A "free" improvement like the above wouldn't hurt.
>>>
>>>>> So my preference would be to check in the , the var will be there as well.
>>>> I don't insist, but it sounds like the logical thing to do. So unless
>>>> you feel strongly against that, I'd say let's add that part. This is,
>>>> after all, an experiment, so let's experiment.
>>> I'm not going to object.
>>>
>>> Doubly so if somebody else wants to finish up the patch and commit it with NEWS and documentation updates. 😉
>> I can finish up the patch if you don't want to 😄
>
> If you don't mind. Thank you. :-)
Ok, will do. In preparation for that function, we need that variable on
the emacs-29 branch, right? So here's a patch for that. What do you
think, Dmitry, Eli and Yuan? If this gets installed on emacs-29, I can
make the function we discussed on the master branch when that is merged.
Theo
[0001-Add-treesit-text-type-regexp.patch (text/x-diff, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Thu, 08 Dec 2022 19:52:01 GMT)
Full text and
rfc822 format available.
Message #214 received at 59662 <at> debbugs.gnu.org (full text, mbox):
> Sometimes it is good to easily differentiate between textual nodes and
> normal code. One such case is for a filling and reindenting command,
> where we fill the textual node if point is on one, and reindent code
> otherwise. This is a part of bug#59662.
I wonder if we shouldn't have something slightly more general, such as
a function which returns nil for "normal code" nodes and non-nil for
"textual" nodes, in which case it could additionally return info such as
whether it's rather like a comment or rather like a string or maybe some
information about how to insert/remove a linebreak from within it (in
case they need to be escaped somehow, or for line-comments).
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Thu, 08 Dec 2022 20:14:01 GMT)
Full text and
rfc822 format available.
Message #217 received at 59662 <at> debbugs.gnu.org (full text, mbox):
> From: Theodor Thornhill <theo <at> thornhill.no>
> Cc: larsi <at> gnus.org, 59662 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca, Yuan Fu
> <casouri <at> gmail.com>
> Date: Thu, 08 Dec 2022 20:15:37 +0100
>
> Ok, will do. In preparation for that function, we need that variable on
> the emacs-29 branch, right?
Why? Why cannot we introduce the variable only on master?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Thu, 08 Dec 2022 21:23:01 GMT)
Full text and
rfc822 format available.
Message #220 received at 59662 <at> debbugs.gnu.org (full text, mbox):
On 8 December 2022 21:12:50 CET, Eli Zaretskii <eliz <at> gnu.org> wrote:
>> From: Theodor Thornhill <theo <at> thornhill.no>
>> Cc: larsi <at> gnus.org, 59662 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca, Yuan Fu
>> <casouri <at> gmail.com>
>> Date: Thu, 08 Dec 2022 20:15:37 +0100
>>
>> Ok, will do. In preparation for that function, we need that variable on
>> the emacs-29 branch, right?
>
>Why? Why cannot we introduce the variable only on master?
Mostly because I want to use it for other stuff, and for merge conflict reasons, but you decide!
I can revise the patch to go on master and introduce the function there.
By the way, what is the deciding factor for what goes where wrt treesit going forward?
Theo
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Thu, 08 Dec 2022 22:05:01 GMT)
Full text and
rfc822 format available.
Message #223 received at 59662 <at> debbugs.gnu.org (full text, mbox):
On 08/12/2022 18:33, Theodor Thornhill wrote:
>
>
> On 8 December 2022 17:06:26 CET, Dmitry Gutov <dgutov <at> yandex.ru> wrote:
>> On 08/12/2022 16:58, Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors wrote:
>>> Dmitry Gutov<dgutov <at> yandex.ru> writes:
>>>
>>>> On 08/12/2022 13:09, Theodor Thornhill wrote:
>>>>> What about this? I changed it to make a point that we should fill if
>>>>> inside of a string or comment, and indent if inside a code block. We
>>>>> one such variable for each type, or in both as suggested below.
>>>> Could you rephrase maybe? I'm not seeing the difference.
>>> In the first example I made the variable specific to comments. But
>>> filling should work on everything that's considered text, IIUC. So if
>>> the variable refers to both the regex needs to accommodate strings and
>>> comments. That will in turn make it less useful if we want to find
>>> comments specifically, or strings specifically. so we could have both
>>> 'treesit-comment-regexp' and 'treesit-string-regexp' and match on '(or
>>> treesit-comment-regexp treesit-string-regexp)' in this particular case.
>>>
>>> Not really sure if that made more sense.
>>
>> Ah. So that was about the name of the variable. Looks good.
>>
>
> Yeah, and thus what nodes it should capture
>
>
>> Note that if the improvement which I described ever comes to life (hopefully), then the variable will probably need to be renamed/moved/multiplied again.
>
> Why is that?
Suppose we have a thingy called tree-sitter-parse-state, or tsps in short.
As a general purpose thigy, it would have the means to check whether the
state corresponds to a string, or to a comment, at least. Maybe it would
be able to signal different kinds of strings and comments as well, but
let's put that aside for now.
For clients to get that info, it can have a general function that would
return the current node (or maybe a derivative of it) -- call it
tree-sitter-parse-state, and helpers like tsps-string-p and
tsps-comment-p, as well as, possibly, tsps-comment-or-string-start.
Maybe other helpers similar to the fields in the ppss defstruct.
And it seems to me that to implement tsps-string-p and tsps-comment-p
you will need two different constants like that. Then, in turn,
prog-fill-or-reindent-defun could be implemented in terms of
tsps-string-p and tsps-comment-p. It could also call
tsps-comment-or-string[-start], if such helper will be defined, which
will internally be implemented either in terms of tsps-*-p functions, or
use the two corresponding new constants anyway.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Thu, 08 Dec 2022 22:06:02 GMT)
Full text and
rfc822 format available.
Message #226 received at 59662 <at> debbugs.gnu.org (full text, mbox):
On 08/12/2022 21:15, Theodor Thornhill via Bug reports for GNU Emacs,
the Swiss army knife of text editors wrote:
> Ok, will do. In preparation for that function, we need that variable on
> the emacs-29 branch, right?
If the variable will be soon substituted with something else, it seems
unwise to define it on the release branch. But then again, if we declare
the whole tree-sitter feature as experimental, maybe it'll be okay.
Depends on what else you want to do with it in emacs-29.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Thu, 08 Dec 2022 22:12:02 GMT)
Full text and
rfc822 format available.
Message #229 received at 59662 <at> debbugs.gnu.org (full text, mbox):
On 8 December 2022 23:05:37 CET, Dmitry Gutov <dgutov <at> yandex.ru> wrote:
>On 08/12/2022 21:15, Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors wrote:
>> Ok, will do. In preparation for that function, we need that variable on
>> the emacs-29 branch, right?
>
>If the variable will be soon substituted with something else, it seems unwise to define it on the release branch. But then again, if we declare the whole tree-sitter feature as experimental, maybe it'll be okay.
>
>Depends on what else you want to do with it in emacs-29.
I was thinking in line of what you wrote in your other mail, but that might be too big to go to emacs-29.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Thu, 08 Dec 2022 22:50:02 GMT)
Full text and
rfc822 format available.
Message #232 received at 59662 <at> debbugs.gnu.org (full text, mbox):
On 09/12/2022 00:10, Theodor Thornhill via Bug reports for GNU Emacs,
the Swiss army knife of text editors wrote:
>
> On 8 December 2022 23:05:37 CET, Dmitry Gutov<dgutov <at> yandex.ru> wrote:
>> On 08/12/2022 21:15, Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors wrote:
>>> Ok, will do. In preparation for that function, we need that variable on
>>> the emacs-29 branch, right?
>> If the variable will be soon substituted with something else, it seems unwise to define it on the release branch. But then again, if we declare the whole tree-sitter feature as experimental, maybe it'll be okay.
>>
>> Depends on what else you want to do with it in emacs-29.
> I was thinking in line of what you wrote in your other mail, but that might be too big to go to emacs-29.
My gut feeling is it will require some time to stabilize, grow the
required features and whatnot, with associated churn. Then it's better
to put into master.
But I can just as well be wrong on this subject, and being able to take
advantage of this feature will be pretty nice.
Speaking of improvements for release, though, have you thought about the
other idea, the "faster syntax-ppss"? It shouldn't require changes to
public-facing infrastructure (or very little; like un-obsoleting
syntax-begin-function). Though whether it will require new constants or
not, I cannot say.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Fri, 09 Dec 2022 05:07:02 GMT)
Full text and
rfc822 format available.
Message #235 received at 59662 <at> debbugs.gnu.org (full text, mbox):
> Date: Thu, 08 Dec 2022 22:21:35 +0100
> From: Theodor Thornhill <theo <at> thornhill.no>
> CC: dgutov <at> yandex.ru, larsi <at> gnus.org, 59662 <at> debbugs.gnu.org,
> monnier <at> iro.umontreal.ca, casouri <at> gmail.com
>
> >> Ok, will do. In preparation for that function, we need that variable on
> >> the emacs-29 branch, right?
> >
> >Why? Why cannot we introduce the variable only on master?
>
> Mostly because I want to use it for other stuff, and for merge conflict reasons, but you decide!
>
> I can revise the patch to go on master and introduce the function there.
Yes, please do that on master.
> By the way, what is the deciding factor for what goes where wrt treesit going forward?
In general, anything that doesn't need to support the minimal treesit
modes we have now on emacs-29 should go to master. That includes any
new features, even if they are supported by treesit. IOW, mot
everything related to tree-sitter should automatically go to the
release branch.
And when in doubt, ask, as always.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Fri, 09 Dec 2022 13:25:01 GMT)
Full text and
rfc822 format available.
Message #238 received at 59662 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> gnu.org> writes:
>> Date: Thu, 08 Dec 2022 22:21:35 +0100
>> From: Theodor Thornhill <theo <at> thornhill.no>
>> CC: dgutov <at> yandex.ru, larsi <at> gnus.org, 59662 <at> debbugs.gnu.org,
>> monnier <at> iro.umontreal.ca, casouri <at> gmail.com
>>
>> >> Ok, will do. In preparation for that function, we need that variable on
>> >> the emacs-29 branch, right?
>> >
>> >Why? Why cannot we introduce the variable only on master?
>>
>> Mostly because I want to use it for other stuff, and for merge conflict reasons, but you decide!
>>
>> I can revise the patch to go on master and introduce the function there.
>
> Yes, please do that on master.
>
>> By the way, what is the deciding factor for what goes where wrt treesit going forward?
>
> In general, anything that doesn't need to support the minimal treesit
> modes we have now on emacs-29 should go to master. That includes any
> new features, even if they are supported by treesit. IOW, mot
> everything related to tree-sitter should automatically go to the
> release branch.
>
> And when in doubt, ask, as always.
Ok! Here's the patch. What do you all think?
Theo
[0001-Add-prog-fill-reindent-defun-bug-59664.patch (text/x-diff, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Fri, 09 Dec 2022 13:39:02 GMT)
Full text and
rfc822 format available.
Message #241 received at 59662 <at> debbugs.gnu.org (full text, mbox):
On 09/12/2022 15:24, Theodor Thornhill via Bug reports for GNU Emacs,
the Swiss army knife of text editors wrote:
> Ok! Here's the patch. What do you all think?
- The NEWS entry probably doesn't need to mention syntax-ppss or
tree-sitter. Those are just implementation details.
- treesit-text-type-regexp could have a default value. (regexp-opt
'("comment")) seems popular enough.
Looks good overall.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Fri, 09 Dec 2022 14:47:02 GMT)
Full text and
rfc822 format available.
Message #244 received at 59662 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Dmitry Gutov <dgutov <at> yandex.ru> writes:
> On 09/12/2022 15:24, Theodor Thornhill via Bug reports for GNU Emacs,
> the Swiss army knife of text editors wrote:
>> Ok! Here's the patch. What do you all think?
>
> - The NEWS entry probably doesn't need to mention syntax-ppss or
> tree-sitter. Those are just implementation details.
>
> - treesit-text-type-regexp could have a default value. (regexp-opt
> '("comment")) seems popular enough.
>
Something like this?
> Looks good overall.
Thanks!
[0001-Add-prog-fill-reindent-defun-bug-59664.patch (text/x-diff, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Fri, 09 Dec 2022 14:57:02 GMT)
Full text and
rfc822 format available.
Message #247 received at 59662 <at> debbugs.gnu.org (full text, mbox):
On 09/12/2022 16:46, Theodor Thornhill wrote:
> Dmitry Gutov <dgutov <at> yandex.ru> writes:
>
>> On 09/12/2022 15:24, Theodor Thornhill via Bug reports for GNU Emacs,
>> the Swiss army knife of text editors wrote:
>>> Ok! Here's the patch. What do you all think?
>>
>> - The NEWS entry probably doesn't need to mention syntax-ppss or
>> tree-sitter. Those are just implementation details.
>>
>> - treesit-text-type-regexp could have a default value. (regexp-opt
>> '("comment")) seems popular enough.
>>
>
> Something like this?
Yes. But... :-)
I guess you can replace (regexp-opt '("comment")) with just "comment"?
Both regexps are equivalent, and neither uses anchoring (like \` and
\'). Should they have anchoring?
The programs.texi addition should probably mention refilling too.
And I see some additional changes in this patch, including ones that
improve defun navigation, font-lock and indentation, in particular in
java-ts-mode (but also json and typescript). These changes should
probably go to emacs-29.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Fri, 09 Dec 2022 15:14:01 GMT)
Full text and
rfc822 format available.
Message #250 received at 59662 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Dmitry Gutov <dgutov <at> yandex.ru> writes:
> On 09/12/2022 16:46, Theodor Thornhill wrote:
>> Dmitry Gutov <dgutov <at> yandex.ru> writes:
>>
>>> On 09/12/2022 15:24, Theodor Thornhill via Bug reports for GNU Emacs,
>>> the Swiss army knife of text editors wrote:
>>>> Ok! Here's the patch. What do you all think?
>>>
>>> - The NEWS entry probably doesn't need to mention syntax-ppss or
>>> tree-sitter. Those are just implementation details.
>>>
>>> - treesit-text-type-regexp could have a default value. (regexp-opt
>>> '("comment")) seems popular enough.
>>>
>>
>> Something like this?
>
> Yes. But... :-)
>
> I guess you can replace (regexp-opt '("comment")) with just "comment"?
> Both regexps are equivalent, and neither uses anchoring (like \` and
> \'). Should they have anchoring?
>
Hehe, I did that first, but I trusted you knew some secret about that
expression. No, we get the names, so I think it's good.
> The programs.texi addition should probably mention refilling too.
>
Good point. Will fix that.
> And I see some additional changes in this patch, including ones that
> improve defun navigation, font-lock and indentation, in particular in
> java-ts-mode (but also json and typescript). These changes should
> probably go to emacs-29.
Yeah, I agree. Some of them are needed for this function to work
properly, but the patches I've submitted for them have been lingering
for a while.
See below patch - I guess we can decide on that last remark later :-)
[0001-Add-prog-fill-reindent-defun-bug-59664.patch (text/x-diff, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Fri, 09 Dec 2022 15:28:02 GMT)
Full text and
rfc822 format available.
Message #253 received at 59662 <at> debbugs.gnu.org (full text, mbox):
On 09/12/2022 17:13, Theodor Thornhill via Bug reports for GNU Emacs,
the Swiss army knife of text editors wrote:
>>> Something like this?
>>
>> Yes. But... :-)
>>
>> I guess you can replace (regexp-opt '("comment")) with just "comment"?
>> Both regexps are equivalent, and neither uses anchoring (like \` and
>> \'). Should they have anchoring?
>>
>
> Hehe, I did that first, but I trusted you knew some secret about that
> expression. No, we get the names, so I think it's good.
Shouldn't trust me that much ;-)
It could be a problem if some grammar somewhere will have a node type
called "not_comment", and the regexp will still match it. Or
"pseudo_string", etc.
Not sure how realistic that scenario is, up to you. The authors of such
major modes could opt for a stricted regexp individually, too.
>> The programs.texi addition should probably mention refilling too.
>>
>
> Good point. Will fix that.
Still not ideal: it says "refill all the lines within a defun", but the
refilling step only touches the paragraph around point.
Also about NEWS: "textual element" is a little value; it really refills
a single paragraph. That distinction is meaningful when a string or a
comment are long, containing several paragraphs inside.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Fri, 09 Dec 2022 15:30:03 GMT)
Full text and
rfc822 format available.
Message #256 received at 59662 <at> debbugs.gnu.org (full text, mbox):
Sorry:
On 09/12/2022 17:27, Dmitry Gutov wrote:
> "textual element" is a little value;
^ vague
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Fri, 09 Dec 2022 15:51:02 GMT)
Full text and
rfc822 format available.
Message #259 received at 59662 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Dmitry Gutov <dgutov <at> yandex.ru> writes:
> On 09/12/2022 17:13, Theodor Thornhill via Bug reports for GNU Emacs,
> the Swiss army knife of text editors wrote:
>>>> Something like this?
>>>
>>> Yes. But... :-)
>>>
>>> I guess you can replace (regexp-opt '("comment")) with just "comment"?
>>> Both regexps are equivalent, and neither uses anchoring (like \` and
>>> \'). Should they have anchoring?
>>>
>>
>> Hehe, I did that first, but I trusted you knew some secret about that
>> expression. No, we get the names, so I think it's good.
>
> Shouldn't trust me that much ;-)
>
:-)
> It could be a problem if some grammar somewhere will have a node type
> called "not_comment", and the regexp will still match it. Or
> "pseudo_string", etc.
>
> Not sure how realistic that scenario is, up to you. The authors of such
> major modes could opt for a stricted regexp individually, too.
>
Right. Yeah, I don't think it'll be a problem, but better to be on the
safe side. Tweaked the regexp a little
>>> The programs.texi addition should probably mention refilling too.
>>>
>>
>> Good point. Will fix that.
>
> Still not ideal: it says "refill all the lines within a defun", but the
> refilling step only touches the paragraph around point.
>
> Also about NEWS: "textual element" is a little value; it really refills
> a single paragraph. That distinction is meaningful when a string or a
> comment are long, containing several paragraphs inside.
Something like this?
Theo
[0001-Add-prog-fill-reindent-defun-bug-59664.patch (text/x-diff, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Fri, 09 Dec 2022 16:06:02 GMT)
Full text and
rfc822 format available.
Message #262 received at 59662 <at> debbugs.gnu.org (full text, mbox):
On 09/12/2022 17:50, Theodor Thornhill via Bug reports for GNU Emacs,
the Swiss army knife of text editors wrote:
> Something like this?
Looking good, thanks!
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Fri, 09 Dec 2022 19:16:01 GMT)
Full text and
rfc822 format available.
Message #265 received at 59662 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Dmitry Gutov <dgutov <at> yandex.ru> writes:
> On 09/12/2022 17:50, Theodor Thornhill via Bug reports for GNU Emacs,
> the Swiss army knife of text editors wrote:
>> Something like this?
>
> Looking good, thanks!
My pleasure. I removed the code that should go to emacs-29. Can you
install if you agree with this patch, Dmitry?
Theo
[0001-Add-prog-fill-reindent-defun-bug-59664.patch (text/x-diff, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Fri, 09 Dec 2022 20:21:02 GMT)
Full text and
rfc822 format available.
Message #268 received at 59662 <at> debbugs.gnu.org (full text, mbox):
> From: Theodor Thornhill <theo <at> thornhill.no>
> Cc: larsi <at> gnus.org, 59662 <at> debbugs.gnu.org, casouri <at> gmail.com,
> monnier <at> iro.umontreal.ca
> Date: Fri, 09 Dec 2022 16:50:25 +0100
>
> diff --git a/doc/emacs/programs.texi b/doc/emacs/programs.texi
> index ba8475e86a..cc06a003e2 100644
> --- a/doc/emacs/programs.texi
> +++ b/doc/emacs/programs.texi
> @@ -409,6 +409,14 @@ Multi-line Indent
> @table @kbd
> @item C-M-q
> Reindent all the lines within one parenthetical grouping.
> +@item M-q
> +Major modes that derive from @code{prog-mode} can reindent all the
> +lines within a defun around point using
> +@code{prog-mode-fill-reindent-defun}. The bounds of of a defun is
> +decided by @code{beginning-of-defun-function} and
> +@code{end-of-defun-function}. If point is inside a comment or a
> +doc-string, refill that paragraph instead, using
> +@code{fill-paragraph-function}.
The 2 variables you mention should have @vindex entries. Also, please
add a cross-reference to the mode in elisp manual where these two
variables are described.
> +*** New command 'prog-fill-reindent-defun'
> +This command either fills a single paragraph in a defun, such as a
> +doc-string, or a comment. If point is inside normal code, it
> +(re)indents the surrounding defun.
These two sentences should be one sentence, otherwise that 'either" is
out of place. Something like
This command either fills a single paragraph in a defun, such as a
doc-string, or a comment, or (re)indents the surrounding defun if
point is not in a comment or a string.
> Bound by default in
> +'prog-mode-map' to 'M-q'.
"It is by default bound to 'M-q' in 'prog-mode' and all its
descendants."
> +(defun prog-fill-reindent-defun (&optional argument)
> + "Refill paragraph or reindent the definition that the point is on.
> +
> +If the point is in a string, or in a comment, or there is a
> +comment on the current line, fill the paragraph that the point is
> +in or is on the same line.
I don't understand the difference between "point is in a string, or in
a comment" and "there is a comment on the current line".
> +Otherwise, reindent the definition around or below point."
Likewise this one is confusing.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Fri, 09 Dec 2022 20:29:02 GMT)
Full text and
rfc822 format available.
Message #271 received at 59662 <at> debbugs.gnu.org (full text, mbox):
On 09/12/2022 21:15, Theodor Thornhill via Bug reports for GNU Emacs,
the Swiss army knife of text editors wrote:
> Dmitry Gutov <dgutov <at> yandex.ru> writes:
>
>> On 09/12/2022 17:50, Theodor Thornhill via Bug reports for GNU Emacs,
>> the Swiss army knife of text editors wrote:
>>> Something like this?
>>
>> Looking good, thanks!
>
> My pleasure. I removed the code that should go to emacs-29. Can you
> install if you agree with this patch, Dmitry?
LGTM.
I expected Eli would also take a look, but since it's for master, I
suppose we don't have to wait for his sign-off.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Fri, 09 Dec 2022 20:32:02 GMT)
Full text and
rfc822 format available.
Message #274 received at 59662 <at> debbugs.gnu.org (full text, mbox):
On 09/12/2022 22:20, Eli Zaretskii wrote:
>> +(defun prog-fill-reindent-defun (&optional argument)
>> + "Refill paragraph or reindent the definition that the point is on.
>> +
>> +If the point is in a string, or in a comment, or there is a
>> +comment on the current line, fill the paragraph that the point is
>> +in or is on the same line.
>
> I don't understand the difference between "point is in a string, or in
> a comment" and "there is a comment on the current line".
The comment may start after point.
>> +Otherwise, reindent the definition around or below point."
>
> Likewise this one is confusing.
The intent was to describe the behavior that follows 'mark-defun'. Which
has this in its docstring:
Put mark at end of this defun, point at beginning.
The defun marked is the one that contains point or follows point.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Fri, 09 Dec 2022 20:32:02 GMT)
Full text and
rfc822 format available.
Message #277 received at 59662 <at> debbugs.gnu.org (full text, mbox):
On 09/12/2022 22:28, Dmitry Gutov wrote:
> I expected Eli would also take a look, but since it's for master, I
> suppose we don't have to wait for his sign-off.
And scratch that.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Fri, 09 Dec 2022 21:10:01 GMT)
Full text and
rfc822 format available.
Message #280 received at 59662 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> gnu.org> writes:
>> From: Theodor Thornhill <theo <at> thornhill.no>
>> Cc: larsi <at> gnus.org, 59662 <at> debbugs.gnu.org, casouri <at> gmail.com,
>> monnier <at> iro.umontreal.ca
>> Date: Fri, 09 Dec 2022 16:50:25 +0100
>>
>> diff --git a/doc/emacs/programs.texi b/doc/emacs/programs.texi
>> index ba8475e86a..cc06a003e2 100644
>> --- a/doc/emacs/programs.texi
>> +++ b/doc/emacs/programs.texi
>> @@ -409,6 +409,14 @@ Multi-line Indent
>> @table @kbd
>> @item C-M-q
>> Reindent all the lines within one parenthetical grouping.
>> +@item M-q
>> +Major modes that derive from @code{prog-mode} can reindent all the
>> +lines within a defun around point using
>> +@code{prog-mode-fill-reindent-defun}. The bounds of of a defun is
>> +decided by @code{beginning-of-defun-function} and
>> +@code{end-of-defun-function}. If point is inside a comment or a
>> +doc-string, refill that paragraph instead, using
>> +@code{fill-paragraph-function}.
>
> The 2 variables you mention should have @vindex entries. Also, please
> add a cross-reference to the mode in elisp manual where these two
> variables are described.
>
>> +*** New command 'prog-fill-reindent-defun'
>> +This command either fills a single paragraph in a defun, such as a
>> +doc-string, or a comment. If point is inside normal code, it
>> +(re)indents the surrounding defun.
>
> These two sentences should be one sentence, otherwise that 'either" is
> out of place. Something like
>
> This command either fills a single paragraph in a defun, such as a
> doc-string, or a comment, or (re)indents the surrounding defun if
> point is not in a comment or a string.
>
>> Bound by default in
>> +'prog-mode-map' to 'M-q'.
Something like the patch below? Forgive me, but my texinfo-fu is
limited. I'm a little unsure if I xrefed to the correct nodes. Maybe
it is better to just mention 'beginning-of-defun' rather than the
variable 'beginning-of-defun-function'?
>
> "It is by default bound to 'M-q' in 'prog-mode' and all its
> descendants."
>
>> +(defun prog-fill-reindent-defun (&optional argument)
>> + "Refill paragraph or reindent the definition that the point is on.
>> +
>> +If the point is in a string, or in a comment, or there is a
>> +comment on the current line, fill the paragraph that the point is
>> +in or is on the same line.
>
> I don't understand the difference between "point is in a string, or in
> a comment" and "there is a comment on the current line".
>
>> +Otherwise, reindent the definition around or below point."
>
> Likewise this one is confusing.
I'm not sure how to word this differently, but I agree it's a mouthful.
[0001-Add-prog-fill-reindent-defun-bug-59664.patch (text/x-diff, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Sat, 10 Dec 2022 12:03:01 GMT)
Full text and
rfc822 format available.
Message #283 received at 59662 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Theodor Thornhill <theo <at> thornhill.no> writes:
> Eli Zaretskii <eliz <at> gnu.org> writes:
>
>>> From: Theodor Thornhill <theo <at> thornhill.no>
>>> Cc: larsi <at> gnus.org, 59662 <at> debbugs.gnu.org, casouri <at> gmail.com,
>>> monnier <at> iro.umontreal.ca
>>> Date: Fri, 09 Dec 2022 16:50:25 +0100
>>>
>>> diff --git a/doc/emacs/programs.texi b/doc/emacs/programs.texi
>>> index ba8475e86a..cc06a003e2 100644
>>> --- a/doc/emacs/programs.texi
>>> +++ b/doc/emacs/programs.texi
>>> @@ -409,6 +409,14 @@ Multi-line Indent
>>> @table @kbd
>>> @item C-M-q
>>> Reindent all the lines within one parenthetical grouping.
>>> +@item M-q
>>> +Major modes that derive from @code{prog-mode} can reindent all the
>>> +lines within a defun around point using
>>> +@code{prog-mode-fill-reindent-defun}. The bounds of of a defun is
>>> +decided by @code{beginning-of-defun-function} and
>>> +@code{end-of-defun-function}. If point is inside a comment or a
>>> +doc-string, refill that paragraph instead, using
>>> +@code{fill-paragraph-function}.
>>
>> The 2 variables you mention should have @vindex entries. Also, please
>> add a cross-reference to the mode in elisp manual where these two
>> variables are described.
>>
Is this worded a little better?
Theo
[0001-Add-prog-fill-reindent-defun-bug-59664.patch (text/x-diff, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Sat, 10 Dec 2022 14:05:01 GMT)
Full text and
rfc822 format available.
Message #286 received at 59662 <at> debbugs.gnu.org (full text, mbox):
> Date: Fri, 9 Dec 2022 22:30:55 +0200
> Cc: larsi <at> gnus.org, 59662 <at> debbugs.gnu.org, casouri <at> gmail.com,
> monnier <at> iro.umontreal.ca
> From: Dmitry Gutov <dgutov <at> yandex.ru>
>
> On 09/12/2022 22:20, Eli Zaretskii wrote:
>
> >> +(defun prog-fill-reindent-defun (&optional argument)
> >> + "Refill paragraph or reindent the definition that the point is on.
> >> +
> >> +If the point is in a string, or in a comment, or there is a
> >> +comment on the current line, fill the paragraph that the point is
> >> +in or is on the same line.
> >
> > I don't understand the difference between "point is in a string, or in
> > a comment" and "there is a comment on the current line".
>
> The comment may start after point.
Only after, not before?
> >> +Otherwise, reindent the definition around or below point."
> >
> > Likewise this one is confusing.
>
> The intent was to describe the behavior that follows 'mark-defun'. Which
> has this in its docstring:
>
> Put mark at end of this defun, point at beginning.
> The defun marked is the one that contains point or follows point.
That one is clear. Does "below point" means before point (i.e. lower
buffer position) or after point (i.e. lower on the screen)?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Sat, 10 Dec 2022 14:10:02 GMT)
Full text and
rfc822 format available.
Message #289 received at 59662 <at> debbugs.gnu.org (full text, mbox):
> From: Theodor Thornhill <theo <at> thornhill.no>
> Cc: dgutov <at> yandex.ru, larsi <at> gnus.org, 59662 <at> debbugs.gnu.org,
> casouri <at> gmail.com, monnier <at> iro.umontreal.ca
> Date: Fri, 09 Dec 2022 22:09:10 +0100
>
> Something like the patch below? Forgive me, but my texinfo-fu is
> limited.
You have nothing to worry in that department.
> I'm a little unsure if I xrefed to the correct nodes. Maybe
> it is better to just mention 'beginning-of-defun' rather than the
> variable 'beginning-of-defun-function'?
I meant a reference to where beginning-of-defun-function is mentioned,
and that is in the ELisp manual. You can see examples of how to
reference another manual in many places in doc/emacs/*.texi; seartch
for "elisp".
> >> +(defun prog-fill-reindent-defun (&optional argument)
> >> + "Refill paragraph or reindent the definition that the point is on.
> >> +
> >> +If the point is in a string, or in a comment, or there is a
> >> +comment on the current line, fill the paragraph that the point is
> >> +in or is on the same line.
> >
> > I don't understand the difference between "point is in a string, or in
> > a comment" and "there is a comment on the current line".
> >
> >> +Otherwise, reindent the definition around or below point."
> >
> > Likewise this one is confusing.
>
> I'm not sure how to word this differently, but I agree it's a mouthful.
If you explain to me in plain English what you meant, I can suggest a
better wording.
Thanks.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Sat, 10 Dec 2022 14:28:02 GMT)
Full text and
rfc822 format available.
Message #292 received at 59662 <at> debbugs.gnu.org (full text, mbox):
> From: Theodor Thornhill <theo <at> thornhill.no>
> Cc: dgutov <at> yandex.ru, larsi <at> gnus.org, 59662 <at> debbugs.gnu.org,
> casouri <at> gmail.com, monnier <at> iro.umontreal.ca
> Date: Sat, 10 Dec 2022 13:02:09 +0100
>
> Is this worded a little better?
I think my last message answers this as well.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Sat, 10 Dec 2022 19:26:01 GMT)
Full text and
rfc822 format available.
Message #295 received at 59662 <at> debbugs.gnu.org (full text, mbox):
On 10/12/2022 16:04, Eli Zaretskii wrote:
>> Date: Fri, 9 Dec 2022 22:30:55 +0200
>> Cc:larsi <at> gnus.org,59662 <at> debbugs.gnu.org,casouri <at> gmail.com,
>> monnier <at> iro.umontreal.ca
>> From: Dmitry Gutov<dgutov <at> yandex.ru>
>>
>> On 09/12/2022 22:20, Eli Zaretskii wrote:
>>
>>>> +(defun prog-fill-reindent-defun (&optional argument)
>>>> + "Refill paragraph or reindent the definition that the point is on.
>>>> +
>>>> +If the point is in a string, or in a comment, or there is a
>>>> +comment on the current line, fill the paragraph that the point is
>>>> +in or is on the same line.
>>> I don't understand the difference between "point is in a string, or in
>>> a comment" and "there is a comment on the current line".
>> The comment may start after point.
> Only after, not before?
Either point is in a comment (possibly multiline), or the comment starts
after point on the same line.
>>>> +Otherwise, reindent the definition around or below point."
>>> Likewise this one is confusing.
>> The intent was to describe the behavior that follows 'mark-defun'. Which
>> has this in its docstring:
>>
>> Put mark at end of this defun, point at beginning.
>> The defun marked is the one that contains point or follows point.
> That one is clear. Does "below point" means before point (i.e. lower
> buffer position) or after point (i.e. lower on the screen)?
Same as "after point", yes.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Sat, 10 Dec 2022 20:23:02 GMT)
Full text and
rfc822 format available.
Message #298 received at 59662 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> gnu.org> writes:
>> From: Theodor Thornhill <theo <at> thornhill.no>
>> Cc: dgutov <at> yandex.ru, larsi <at> gnus.org, 59662 <at> debbugs.gnu.org,
>> casouri <at> gmail.com, monnier <at> iro.umontreal.ca
>> Date: Fri, 09 Dec 2022 22:09:10 +0100
>>
>> Something like the patch below? Forgive me, but my texinfo-fu is
>> limited.
>
> You have nothing to worry in that department.
>
>> I'm a little unsure if I xrefed to the correct nodes. Maybe
>> it is better to just mention 'beginning-of-defun' rather than the
>> variable 'beginning-of-defun-function'?
>
> I meant a reference to where beginning-of-defun-function is mentioned,
> and that is in the ELisp manual. You can see examples of how to
> reference another manual in many places in doc/emacs/*.texi; seartch
> for "elisp".
>
Ok, thanks!
>> >> +(defun prog-fill-reindent-defun (&optional argument)
>> >> + "Refill paragraph or reindent the definition that the point is on.
>> >> +
>> >> +If the point is in a string, or in a comment, or there is a
>> >> +comment on the current line, fill the paragraph that the point is
>> >> +in or is on the same line.
>> >
>> > I don't understand the difference between "point is in a string, or in
>> > a comment" and "there is a comment on the current line".
>> >
>> >> +Otherwise, reindent the definition around or below point."
>> >
>> > Likewise this one is confusing.
>>
>> I'm not sure how to word this differently, but I agree it's a mouthful.
>
> If you explain to me in plain English what you meant, I can suggest a
> better wording.
>
I made another attempt - I think it's a little clearer. What I want to
say is something like this (| denotes point):
1. If point is inside a comment, refill the paragraph:
// foo bar | baz
2. If point is before a comment, refill the paragraph:
| // foo bar baz
3. If point is inside code, reindent the defun:
int
main(void)
{
int i; // <-- This will be reindented
/* Some very long comment that will _not_ be refilled in this case. */
for(;;)
{
| int x;
}
}
Anyway, see attached patch:
Theo
[0001-Add-prog-fill-reindent-defun-bug-59664.patch (text/x-diff, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Sun, 11 Dec 2022 08:13:02 GMT)
Full text and
rfc822 format available.
Message #301 received at 59662 <at> debbugs.gnu.org (full text, mbox):
> From: Theodor Thornhill <theo <at> thornhill.no>
> Cc: dgutov <at> yandex.ru, larsi <at> gnus.org, 59662 <at> debbugs.gnu.org,
> casouri <at> gmail.com, monnier <at> iro.umontreal.ca
> Date: Sat, 10 Dec 2022 21:21:53 +0100
>
> I made another attempt - I think it's a little clearer. What I want to
> say is something like this (| denotes point):
>
> 1. If point is inside a comment, refill the paragraph:
> // foo bar | baz
>
> 2. If point is before a comment, refill the paragraph:
> | // foo bar baz
>
> 3. If point is inside code, reindent the defun:
> int
> main(void)
> {
> int i; // <-- This will be reindented
> /* Some very long comment that will _not_ be refilled in this case. */
> for(;;)
> {
> | int x;
> }
> }
What if point is after a comment, like this:
/* foo bar baz */ |
> +@code{fill-paragraph-function} (@pxref{List Motion,,, elisp, The Emacs
> +Lisp Reference Manual} or @ref{Filling,,, elisp, The Emacs Lisp
> +Reference Manual} for more information).
The @pxref should be "see @ref", and please add a comma after the
closing brace (some old versions of makeinfo insist on that). @pxref
is only appropriate if its closing brace immediately precedes the
closing parenthesis.
Otherwise LGTM, thanks.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Sun, 11 Dec 2022 08:40:01 GMT)
Full text and
rfc822 format available.
Message #304 received at 59662 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> gnu.org> writes:
>> From: Theodor Thornhill <theo <at> thornhill.no>
>> Cc: dgutov <at> yandex.ru, larsi <at> gnus.org, 59662 <at> debbugs.gnu.org,
>> casouri <at> gmail.com, monnier <at> iro.umontreal.ca
>> Date: Sat, 10 Dec 2022 21:21:53 +0100
>>
>> I made another attempt - I think it's a little clearer. What I want to
>> say is something like this (| denotes point):
>>
>> 1. If point is inside a comment, refill the paragraph:
>> // foo bar | baz
>>
>> 2. If point is before a comment, refill the paragraph:
>> | // foo bar baz
>>
>> 3. If point is inside code, reindent the defun:
>> int
>> main(void)
>> {
>> int i; // <-- This will be reindented
>> /* Some very long comment that will _not_ be refilled in this case. */
>> for(;;)
>> {
>> | int x;
>> }
>> }
>
> What if point is after a comment, like this:
>
> /* foo bar baz */ |
>
Then the defun will be reindented. In one line comments the comment
will be filled.
>> +@code{fill-paragraph-function} (@pxref{List Motion,,, elisp, The Emacs
>> +Lisp Reference Manual} or @ref{Filling,,, elisp, The Emacs Lisp
>> +Reference Manual} for more information).
>
> The @pxref should be "see @ref", and please add a comma after the
> closing brace (some old versions of makeinfo insist on that). @pxref
> is only appropriate if its closing brace immediately precedes the
> closing parenthesis.
>
> Otherwise LGTM, thanks.
Thanks for your patience!
What about this?
Theo
[0001-Add-prog-fill-reindent-defun-bug-59664.patch (text/x-diff, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Sun, 11 Dec 2022 10:04:02 GMT)
Full text and
rfc822 format available.
Message #307 received at 59662 <at> debbugs.gnu.org (full text, mbox):
> From: Theodor Thornhill <theo <at> thornhill.no>
> Cc: dgutov <at> yandex.ru, larsi <at> gnus.org, 59662 <at> debbugs.gnu.org,
> casouri <at> gmail.com, monnier <at> iro.umontreal.ca
> Date: Sun, 11 Dec 2022 09:39:46 +0100
>
> What about this?
LGTM, thanks.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Sun, 11 Dec 2022 14:14:02 GMT)
Full text and
rfc822 format available.
Message #310 received at 59662 <at> debbugs.gnu.org (full text, mbox):
On 11 December 2022 11:03:35 CET, Eli Zaretskii <eliz <at> gnu.org> wrote:
>> From: Theodor Thornhill <theo <at> thornhill.no>
>> Cc: dgutov <at> yandex.ru, larsi <at> gnus.org, 59662 <at> debbugs.gnu.org,
>> casouri <at> gmail.com, monnier <at> iro.umontreal.ca
>> Date: Sun, 11 Dec 2022 09:39:46 +0100
>>
>> What about this?
>
>LGTM, thanks.
Nice. Will you or Dmitry install?
Thanks,
Theo
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Sun, 11 Dec 2022 20:37:01 GMT)
Full text and
rfc822 format available.
Message #313 received at 59662 <at> debbugs.gnu.org (full text, mbox):
On 11/12/2022 16:13, Theodor Thornhill via Bug reports for GNU Emacs,
the Swiss army knife of text editors wrote:
> Nice. Will you or Dmitry install?
Pushed to master, thank you.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#59662; Package
emacs.
(Sun, 11 Dec 2022 20:41:01 GMT)
Full text and
rfc822 format available.
Message #316 received at 59662 <at> debbugs.gnu.org (full text, mbox):
Dmitry Gutov <dgutov <at> yandex.ru> writes:
> On 11/12/2022 16:13, Theodor Thornhill via Bug reports for GNU Emacs,
> the Swiss army knife of text editors wrote:
>> Nice. Will you or Dmitry install?
>
> Pushed to master, thank you.
Thanks!
Theo
Reply sent
to
Yuan Fu <casouri <at> gmail.com>:
You have taken responsibility.
(Sat, 07 Jan 2023 23:02:02 GMT)
Full text and
rfc822 format available.
Notification sent
to
Theodor Thornhill <theo <at> thornhill.no>:
bug acknowledged by developer.
(Sat, 07 Jan 2023 23:02:02 GMT)
Full text and
rfc822 format available.
Message #321 received at 59662-done <at> debbugs.gnu.org (full text, mbox):
Theodor Thornhill <theo <at> thornhill.no> writes:
> Dmitry Gutov <dgutov <at> yandex.ru> writes:
>
>> On 11/12/2022 16:13, Theodor Thornhill via Bug reports for GNU Emacs,
>> the Swiss army knife of text editors wrote:
>>> Nice. Will you or Dmitry install?
>>
>> Pushed to master, thank you.
>
> Thanks!
>
> Theo
Closing this :-)
Yuan
bug archived.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org.
(Sun, 05 Feb 2023 12:24:05 GMT)
Full text and
rfc822 format available.
This bug report was last modified 2 years and 158 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.