search

jscheid / prettier.el

Prettier code formatting for Emacs.
GNU General Public License v3.0
164 stars 11 forks source link

Fix performance issue with cursor translation #74

Closed jscheid closed 3 years ago

jscheid commented 3 years ago

As per https://github.com/prettier/prettier/issues/4801, formatWithCursor can be extremely slow and consume large amounts of memory. We're actually doing something very similar to the algorithm described in that ticket already, namely character-wise diffs. This means that the cursor is actually fine if we just let Emacs manage the position.

jscheid commented 3 years ago

@asbish good question, I chose it without too much consideration as a starting point and was planning on tweaking it over time.

However, after looking at the underlying issue (https://github.com/prettier/prettier/issues/4801) more closely, I think this is actually a problem regardless of parser. Perhaps it should be an allowlist rather than a denylist (i.e. prettier-parsers-with-cursor-translation) or perhaps we should even go so far as to never use the upstream cursor handling, without a configuration option. What do you think? Did you have a chance to play with it and what was your experience?

By the way, I have been thinking about a small future improvement for this: if we were to merge #66 it would allow for slightly improved logic when the cursor is inside a chunk of text that is replaced (deleted, then inserted). I'm not actually sure it's necessary but probably worth exploring more.

asbish commented 3 years ago

Sorry for the delayed response.

I've took a closer look at the effect of this in several modes and couldn't see differences in behavior or speed between format and formatWithCursor. IMHO, since formatWithCursor has the issue, I think it might be a good idea to never use the upstream cursor handling in the future. As you already know, prettier-vscode uses only format and the issue has remained over 2 years....

Also many prettier users (including me) use some of these modes in prettier-without-cursor-translation, so I think this change makes format default behavior for them.

I began using #66 because I've got some unformatted css files with over 1000lines:joy:

jscheid commented 3 years ago

Thanks @asbish , I didn't know prettier-vscode (or any other plugin) wasn't using formatWithCursor.

I agree, let's just remove it altogether. I'm going to close this and open a new PR instead sometime soon.

jscheid commented 3 years ago

-> #77