* Re: master updated (cd8d3f3379e -> 1b0348d9593)
[not found] <168703958196.28351.5331986860123726819@vcs2.savannah.gnu.org>
@ 2023-06-18 16:02 ` Michael Albinus
2023-06-18 18:28 ` Stefan Monnier
[not found] ` <20230617220622.EC118C1925A@vcs2.savannah.gnu.org>
1 sibling, 1 reply; 11+ messages in thread
From: Michael Albinus @ 2023-06-18 16:02 UTC (permalink / raw)
To: emacs-devel; +Cc: Stefan Monnier
Stefan Monnier via Mailing list for Emacs changes <emacs-diffs@gnu.org>
writes:
Hi,
> monnier pushed a change to branch master.
>
> from cd8d3f3379e Fix some tree-sitter :match regexps
> new f411cc3a957 * lisp/emacs-lisp/lisp-mode.el (lisp-ppss): Fix performance bug
> new 184106be267 pp.el (pp-default-function): New custom var
> new 2f181d60323 pp.el (pp-fill): New default pp function
> new 017475a70ed * doc/lispref/streams.texi (Output Variables): Document `pp-default-function`
> new a9c962be961 pp-fill: Fix tests breakage
> new 1b0348d9593 Merge branch 'master' of git+ssh://git.sv.gnu.org/srv/git/emacs
>
>
> Summary of changes:
> doc/lispref/streams.texi | 9 +
> etc/NEWS | 7 +
> lisp/emacs-lisp/lisp-mode.el | 2 +-
> lisp/emacs-lisp/pp.el | 288 ++++++++++++++++++++++++++------
> test/lisp/emacs-lisp/backtrace-tests.el | 6 +-
> test/lisp/emacs-lisp/pp-tests.el | 4 +-
> 6 files changed, 260 insertions(+), 56 deletions(-)
It looks like this patch series has broken esh-util-tests.el. Try
--8<---------------cut here---------------start------------->8---
# make -C test esh-util-tests
...
Test esh-util-test/eshell-stringify/list condition:
(ert-test-failed
((should (equal (eshell-stringify ...) "((1 2)\n (3 . 4))")) :form
(equal "((1 2) (3 . 4))" "((1 2)\n (3 . 4))") :value nil
:explanation
(arrays-of-different-length 15 16 "((1 2) (3 . 4))"
"((1 2)\n (3 . 4))" first-mismatch-at 6)))
FAILED 9/13 esh-util-test/eshell-stringify/list (0.000212 sec) at lisp/eshell/esh-util-tests.el:47
--8<---------------cut here---------------end--------------->8---
Best regards, Michael.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: master updated (cd8d3f3379e -> 1b0348d9593)
2023-06-18 16:02 ` master updated (cd8d3f3379e -> 1b0348d9593) Michael Albinus
@ 2023-06-18 18:28 ` Stefan Monnier
2023-06-18 18:30 ` Eli Zaretskii
0 siblings, 1 reply; 11+ messages in thread
From: Stefan Monnier @ 2023-06-18 18:28 UTC (permalink / raw)
To: Michael Albinus; +Cc: emacs-devel
> It looks like this patch series has broken esh-util-tests.el. Try
>
> --8<---------------cut here---------------start------------->8---
> # make -C test esh-util-tests
> ...
> Test esh-util-test/eshell-stringify/list condition:
> (ert-test-failed
> ((should (equal (eshell-stringify ...) "((1 2)\n (3 . 4))")) :form
> (equal "((1 2) (3 . 4))" "((1 2)\n (3 . 4))") :value nil
> :explanation
> (arrays-of-different-length 15 16 "((1 2) (3 . 4))"
> "((1 2)\n (3 . 4))" first-mismatch-at 6)))
> FAILED 9/13 esh-util-test/eshell-stringify/list (0.000212 sec) at lisp/eshell/esh-util-tests.el:47
> --8<---------------cut here---------------end--------------->8---
I don't think it's a bug if the output of the pretty printer is
((1 2) (3 . 4))
rather than
((1 2)
(3 . 4))
so I think the esh-util-tests is just too strict: both output are valid,
as are various others such as
((1
2)
(3
. 4))
Not knowing what the test is indeed the check I'm not sure how it should
be fixed. Maybe replace all `[ \n]+` with SPC ?
Stefan
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: master updated (cd8d3f3379e -> 1b0348d9593)
2023-06-18 18:28 ` Stefan Monnier
@ 2023-06-18 18:30 ` Eli Zaretskii
2023-06-18 18:39 ` Eli Zaretskii
0 siblings, 1 reply; 11+ messages in thread
From: Eli Zaretskii @ 2023-06-18 18:30 UTC (permalink / raw)
To: Stefan Monnier; +Cc: michael.albinus, emacs-devel
> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: emacs-devel@gnu.org
> Date: Sun, 18 Jun 2023 14:28:00 -0400
>
> Not knowing what the test is indeed the check I'm not sure how it should
> be fixed. Maybe replace all `[ \n]+` with SPC ?
Indeed, that's what I would do. Or maybe read the result back and
compare Lisp objects?
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: master updated (cd8d3f3379e -> 1b0348d9593)
2023-06-18 18:30 ` Eli Zaretskii
@ 2023-06-18 18:39 ` Eli Zaretskii
2023-06-18 21:39 ` Jim Porter
0 siblings, 1 reply; 11+ messages in thread
From: Eli Zaretskii @ 2023-06-18 18:39 UTC (permalink / raw)
To: Jim Porter; +Cc: monnier, michael.albinus, emacs-devel
> Date: Sun, 18 Jun 2023 21:30:20 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: michael.albinus@gmx.de, emacs-devel@gnu.org
>
> > From: Stefan Monnier <monnier@iro.umontreal.ca>
> > Cc: emacs-devel@gnu.org
> > Date: Sun, 18 Jun 2023 14:28:00 -0400
> >
> > Not knowing what the test is indeed the check I'm not sure how it should
> > be fixed. Maybe replace all `[ \n]+` with SPC ?
>
> Indeed, that's what I would do. Or maybe read the result back and
> compare Lisp objects?
Jim, could you please look into fixing the test?
Thanks.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: master updated (cd8d3f3379e -> 1b0348d9593)
2023-06-18 18:39 ` Eli Zaretskii
@ 2023-06-18 21:39 ` Jim Porter
0 siblings, 0 replies; 11+ messages in thread
From: Jim Porter @ 2023-06-18 21:39 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: monnier, michael.albinus, emacs-devel
On 6/18/2023 11:39 AM, Eli Zaretskii wrote:
>> Date: Sun, 18 Jun 2023 21:30:20 +0300
>> From: Eli Zaretskii <eliz@gnu.org>
>> Cc: michael.albinus@gmx.de, emacs-devel@gnu.org
>>
>>> From: Stefan Monnier <monnier@iro.umontreal.ca>
>>> Cc: emacs-devel@gnu.org
>>> Date: Sun, 18 Jun 2023 14:28:00 -0400
>>>
>>> Not knowing what the test is indeed the check I'm not sure how it should
>>> be fixed. Maybe replace all `[ \n]+` with SPC ?
>>
[snip]
>
> Jim, could you please look into fixing the test?
Thanks for letting me know. I've fixed this as suggested above, and
added a comment explaining what the test should, well... test.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: master 2f181d60323 3/6: pp.el (pp-fill): New default pp function
[not found] ` <20230617220622.EC118C1925A@vcs2.savannah.gnu.org>
@ 2023-07-14 3:31 ` Michael Heerdegen
2023-07-14 4:57 ` Stefan Monnier
0 siblings, 1 reply; 11+ messages in thread
From: Michael Heerdegen @ 2023-07-14 3:31 UTC (permalink / raw)
To: emacs-devel; +Cc: Stefan Monnier
Stefan Monnier via Mailing list for Emacs changes <emacs-diffs@gnu.org>
writes:
> branch: master
> commit 2f181d60323bd9e0196775828de633100479f4c2
> Author: Stefan Monnier <monnier@iro.umontreal.ca>
> Commit: Stefan Monnier <monnier@iro.umontreal.ca>
>
> pp.el (pp-fill): New default pp function
> [...]
> + (when (and paired (not (eq ?\" (char-after paired))))
> + ;; The sexp has sub-parts, so let's try and spread
> + ;; them over several lines.
> + (save-excursion
> + (goto-char beg)
> + (when (looking-at "(\\([^][()\" \t\n;']+\\)")
> + ;; Inside an expression of the form (SYM ARG1
> + ;; ARG2 ... ARGn) where SYM has a `lisp-indent-function'
> + ;; property that's a number, insert a newline after
> + ;; the corresponding ARGi, because it tends to lead to
> + ;; more natural and less indented code.
> + (let* ((sym (intern-soft (match-string 1)))
> + (lif (and sym (get sym 'lisp-indent-function))))
> + (if (eq lif 'defun) (setq lif 2))
> + (when (natnump lif)
> + (goto-char (match-end 0))
> + (forward-sexp lif)
I only want to give the hint that this `forward-sexp' in the last line
will error when the programmer has, e.g., commented out the body of a
lambda expression. Or the user wants to fill automatically created code
that looks like that, for whatever reason.
Michael.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: master 2f181d60323 3/6: pp.el (pp-fill): New default pp function
2023-07-14 3:31 ` master 2f181d60323 3/6: pp.el (pp-fill): New default pp function Michael Heerdegen
@ 2023-07-14 4:57 ` Stefan Monnier
2023-07-15 2:48 ` Michael Heerdegen
0 siblings, 1 reply; 11+ messages in thread
From: Stefan Monnier @ 2023-07-14 4:57 UTC (permalink / raw)
To: Michael Heerdegen; +Cc: emacs-devel
> I only want to give the hint that this `forward-sexp' in the last line
> will error when the programmer has, e.g., commented out the body of a
> lambda expression. Or the user wants to fill automatically created code
> that looks like that, for whatever reason.
Hmm... I don't quite understand what you mean...
Oh, are you thinking of when `lif` is a number larger than the number of
elements in the list?
Indeed, I think we'll burp then. I'll take a look at that.
Stefan
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: master 2f181d60323 3/6: pp.el (pp-fill): New default pp function
2023-07-14 4:57 ` Stefan Monnier
@ 2023-07-15 2:48 ` Michael Heerdegen
2023-07-15 15:19 ` Stefan Monnier
0 siblings, 1 reply; 11+ messages in thread
From: Michael Heerdegen @ 2023-07-15 2:48 UTC (permalink / raw)
To: emacs-devel
Stefan Monnier <monnier@iro.umontreal.ca> writes:
> > I only want to give the hint that this `forward-sexp' in the last line
> > will error when the programmer has, e.g., commented out the body of a
> > lambda expression. Or the user wants to fill automatically created code
> > that looks like that, for whatever reason.
>
> Hmm... I don't quite understand what you mean...
> Oh, are you thinking of when `lif` is a number larger than the number of
> elements in the list?
Yes.
> Indeed, I think we'll burp then. I'll take a look at that.
Thanks. I would vote for silently ignoring and falling back to default
indentation.
BTW, we had a thread in the Gnus mailing list
(info-gnus-english@gnu.org) about `pp-fill'. Now that it's the default
pp behavior, Gnus score files will be filled using this function before
saving. I don't use score files, could be using this function is not
appropriate for this kind of data.
Anyway, a user reported a massive slowdown after you had made that
change (thread "slow saving of scores when leaving a virtual (nnml+)
group"), and posted this profiler output:
| Cpu report (partly expanded):
|
| 10133 79% - command-execute
| 8519 66% - funcall-interactively
| 4767 37% - gnus-summary-exit
| 4659 36% - gnus-score-save
| 4655 36% - gnus-pp
| 4655 36% - pp
| 4655 36% - pp-to-string
| 4655 36% - pp-fill
| 4647 36% - pp--object
| 4627 36% - pp-fill
| 4615 36% - pp-fill
| 4555 35% - pp-fill
| 4263 33% - pp-fill
| 4243 33% - indent-according-to-mode
| 4243 33% - lisp-indent-line
| 4163 32% - calculate-lisp-indent
| 4163 32% - lisp-indent-function
| 4163 32% lisp--local-defform-body-p
| 48 0% + lisp-ppss
| 240 1% + indent-according-to-mode
| 72 0% + gnus-run-hooks
| 32 0% + gnus-close-group
| 4 0% + gnus-summary-update-info
| 3688 28% + gnus-topic-select-group
| 60 0% + file-notify-handle-event
| 3 0% + execute-extended-command
| 1614 12% + byte-code
| 2260 17% Automatic GC
| 407 3% + timer-event-handler
| 0 0% ...
Dunno if it is interesting for you. So far I don't know how the data
looked like, but we can request it from the user. After changing
`pp-default-function' to 'pp-28' the user is happy.
Michael.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: master 2f181d60323 3/6: pp.el (pp-fill): New default pp function
2023-07-15 2:48 ` Michael Heerdegen
@ 2023-07-15 15:19 ` Stefan Monnier
2023-07-17 9:58 ` Eric S Fraga
2023-07-17 12:24 ` Eric S Fraga
0 siblings, 2 replies; 11+ messages in thread
From: Stefan Monnier @ 2023-07-15 15:19 UTC (permalink / raw)
To: Michael Heerdegen; +Cc: emacs-devel
>> Indeed, I think we'll burp then. I'll take a look at that.
> Thanks. I would vote for silently ignoring and falling back to default
> indentation.
Indeed.
> BTW, we had a thread in the Gnus mailing list
> (info-gnus-english@gnu.org) about `pp-fill'. Now that it's the default
> pp behavior, Gnus score files will be filled using this function before
> saving. I don't use score files, could be using this function is not
> appropriate for this kind of data.
I'm always ambivalent about the use of `pp` for "internal data files"
like these. I suspect they'd often be better served with a plain
`prin1`, but I'd be interesting to hear the motivation behind the use
of `pp`.
> Anyway, a user reported a massive slowdown after you had made that
> change (thread "slow saving of scores when leaving a virtual (nnml+)
> group"), and posted this profiler output:
>
> | Cpu report (partly expanded):
> |
> | 10133 79% - command-execute
> | 8519 66% - funcall-interactively
> | 4767 37% - gnus-summary-exit
> | 4659 36% - gnus-score-save
> | 4655 36% - gnus-pp
> | 4655 36% - pp
> | 4655 36% - pp-to-string
> | 4655 36% - pp-fill
> | 4647 36% - pp--object
> | 4627 36% - pp-fill
> | 4615 36% - pp-fill
> | 4555 35% - pp-fill
> | 4263 33% - pp-fill
> | 4243 33% - indent-according-to-mode
> | 4243 33% - lisp-indent-line
> | 4163 32% - calculate-lisp-indent
> | 4163 32% - lisp-indent-function
> | 4163 32% lisp--local-defform-body-p
> | 48 0% + lisp-ppss
> | 240 1% + indent-according-to-mode
> | 72 0% + gnus-run-hooks
> | 32 0% + gnus-close-group
> | 4 0% + gnus-summary-update-info
> | 3688 28% + gnus-topic-select-group
> | 60 0% + file-notify-handle-event
> | 3 0% + execute-extended-command
> | 1614 12% + byte-code
> | 2260 17% Automatic GC
> | 407 3% + timer-event-handler
> | 0 0% ...
Hmm... that looks bad, indeed, and looks (again) like a performance
problem in `lisp-indent-line`.
More specifically the `lisp--local-defform-body-p` up there suggests the
problem was introduced by the special indentation rules for `cl-flet`.
> Dunno if it is interesting for you. So far I don't know how the data
> looked like, but we can request it from the user.
That would be nice, yes. Opening an Emacs bug report for it would make
a lot of sense as well.
> After changing `pp-default-function' to 'pp-28' the user is happy.
If Gnus doesn't want to use `prin1` than maybe it should use `pp-28` or
even some other/new alternative designed for "fast pp of data": we
should be able to go significantly faster than `pp-28`. E.g. we could
forgo indentation altogether and only insert \n at appropriate places,
which would not only be fast but result is smaller files.
Stefan
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: master 2f181d60323 3/6: pp.el (pp-fill): New default pp function
2023-07-15 15:19 ` Stefan Monnier
@ 2023-07-17 9:58 ` Eric S Fraga
2023-07-17 12:24 ` Eric S Fraga
1 sibling, 0 replies; 11+ messages in thread
From: Eric S Fraga @ 2023-07-17 9:58 UTC (permalink / raw)
To: emacs-devel
On Saturday, 15 Jul 2023 at 11:19, Stefan Monnier wrote:
>> Dunno if it is interesting for you. So far I don't know how the data
>> looked like, but we can request it from the user.
>
> That would be nice, yes. Opening an Emacs bug report for it would make
> a lot of sense as well.
The user was me; I will try to open a bug report later today. But I'm
here should you want to know anything further etc.
--
Eric S Fraga via gnus (Emacs 30.0.50 2023-07-10) on Debian 12.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: master 2f181d60323 3/6: pp.el (pp-fill): New default pp function
2023-07-15 15:19 ` Stefan Monnier
2023-07-17 9:58 ` Eric S Fraga
@ 2023-07-17 12:24 ` Eric S Fraga
1 sibling, 0 replies; 11+ messages in thread
From: Eric S Fraga @ 2023-07-17 12:24 UTC (permalink / raw)
To: emacs-devel
Emacs bug reported: bug#64681
Thank you,
eric
--
Eric S Fraga via gnus (Emacs 30.0.50 2023-07-10) on Debian 12.0
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-07-17 12:24 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <168703958196.28351.5331986860123726819@vcs2.savannah.gnu.org>
2023-06-18 16:02 ` master updated (cd8d3f3379e -> 1b0348d9593) Michael Albinus
2023-06-18 18:28 ` Stefan Monnier
2023-06-18 18:30 ` Eli Zaretskii
2023-06-18 18:39 ` Eli Zaretskii
2023-06-18 21:39 ` Jim Porter
[not found] ` <20230617220622.EC118C1925A@vcs2.savannah.gnu.org>
2023-07-14 3:31 ` master 2f181d60323 3/6: pp.el (pp-fill): New default pp function Michael Heerdegen
2023-07-14 4:57 ` Stefan Monnier
2023-07-15 2:48 ` Michael Heerdegen
2023-07-15 15:19 ` Stefan Monnier
2023-07-17 9:58 ` Eric S Fraga
2023-07-17 12:24 ` Eric S Fraga
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).