-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Pre-warm idea #81
Comments
Great idea, it seems so obvious that I suspect there might be a reason why I didn't do it that way, as I remember spending quite a bit of time on tweaking the experience around first use back when I started out. But it's also possible it simply didn't occur to me. Let me take a closer look at this sometime soon. |
What I've found out so far revisiting my code:
Aside from fixing that bug, perhaps an improvement would be a setting that starts the process at load time rather than at first use. Perhaps a new setting |
Sounds like a good compromise.
P.S. Which code contains the bug that renders the strategy useless? Have
you reported it?
…On Thu, Mar 4, 2021 at 3:27 AM Julian Scheid ***@***.***> wrote:
What I've found out so far revisiting my code:
1. It turns out that this is actually how I had intended for it to
work, except that there's a bug which renders it useless: the warm-up
message is sent in a generator function, but because iter-next isn't
called on the resulting iterator, the function -- although it will be
invoked -- is never actually executed and therefore the warm-up message
never sent.
2. Even if it were to work correctly, the subprocess is only started
when a relevant buffer is visited (or is already open at the time
global-prettier-mode is activated) -- but in this case, the subprocess
is (in the default configuration) queried immediately for Prettier
settings, which means that any background warm-up activity won't have time
to complete before the next request comes in, and so will effectively be
blocking anyway.
Aside from fixing that bug, perhaps an improvement would be a setting that
starts the process at load time rather than at first use. Perhaps a new
setting eager for prettier-pre-warm, and it would probably be OK to
enable that by default, as presumably people will only load (as opposed to
install) the package if/when they intend to use it. What do you think?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#81 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAKYIJG35A6LXIW5QEHOWLTB5VATANCNFSM4YRQGXYA>
.
--
-Dave
|
It's a bug in this package. The iterator shouldn't be discarded here: Line 891 in ae5553b
I'll fix that as part of implementing eager pre-warming. |
Posting here as this is seems somewhat related: https://prettier.io/blog/2022/06/14/2.7.0.html
Can this be leveraged? I'll admit that I only briefly looked at the source out of curiosity but don't have a MR to offer. (not that I have any performance issues at the moment) |
Ah, thanks for the heads up, however this package isn't using the Prettier CLI, it's using the API. At any rate, the intended usage is to format on save, and Emacs doesn't save unchanged files (it will print |
Definitely, thanks for the clarification 👍 |
Did you ever implement the eager pre-warming? Should we close this issue? |
@dabrahams I've actually started working on it recently as part of a major overhaul which I think of as v2.0. I've been working on it on and off and it's coming along nicely, watch this space. It's going to be a variation of what we had discussed before -- specifically, in the (default) case where we sync over settings from Prettier, such as indentation width, I think we should block on first edit by default rather than on save. This should still give a smooth experience in the common case where you don't immediately start editing a buffer after it opens. You'll usually navigate inside the buffer first or do something else entirely, such as opening a second file. |
Just a thought: why not start pre-warming in the background on first activation and wait for it (if it hasn't completed already) on first save? That would seem to give the ideal user experience in most cases.
The text was updated successfully, but these errors were encountered: