Formatting on save takes ~300ms, the CLI takes ~15ms

[innocenzi]

Context

I'm trying to move from my php-cs-fixer setup to Pint, so I need IDE support and I naturally found this extension.

Describe the bug

Saving using the formatter takes about 300 milliseconds. This is very noticeable when formatting on save compared to the ~20ms that the CLI command takes.

To Reproduce
Steps to reproduce the behavior:

1. Install the extension
2. Configure formatOnSave to true or codeActionsOnSave to source.fixAll.format
3. Configure php's formatter to open-southeners.laravel-pint
4. Save a PHP file
5. See how long it takes for the changes to happen

Expected behavior
The changes should not take more than a few more milliseconds compared to the CLI.

Environment:

• Operating system: macOS
• IDE / version: VSC 1.79.2
• Extension version: 1.1.4
• Extension's config:

"[php]": {
	"editor.codeActionsOnSave": ["source.fixAll.format"],
	"editor.defaultFormatter": "open-southeners.laravel-pint",
},

[innocenzi]

The issue seems to happen in PintEditService#formatFile, since manually formatting the a file using the VSC command takes just as long.

[Madscientiste]

I have the same issue, sometimes reaching up to 500ms~ ..

[Urbanproof]

I did some digging, and it looks like this could be resolved by caching the resolved command. The actual format-command takes practically no time at all, even with a heap of PHP extensions installed & active (including xdebug which usually slows things down quite a bit).

I added a very hacky caching logic to src/PintEditService.ts, and voila:

["INFO" - 23.01.55] Extension Name: open-southeners.laravel-pint.
["INFO" - 23.01.55] Extension Version: 1.1.4.
["INFO" - 23.02.21] [TESTING] Command resolution completed in 602ms.     <------ First run
["INFO" - 23.02.21] [TESTING] Command execution completed in 1ms.
["INFO" - 23.02.21] [TESTING] Formatting completed in 604ms.
["INFO" - 23.02.26] [TESTING] Command resolution completed in 1ms.         <------ Second run
["INFO" - 23.02.26] [TESTING] Command execution completed in 1ms.
["INFO" - 23.02.26] [TESTING] Formatting completed in 2ms.

[d8vjork]

@Urbanproof Will love to see that caching logic 👀

Otherwise feel free to open a draft PR so we can review it

[innocenzi]

@Urbanproof I think the resolution logic should happen when the extension loads, not when the first save happens, because the delay will still be very noticeable

[d8vjork]

Doing some tests on one of my projects (on a very small file) still don't see much difference (taking less maybe because of the process spawn of NodeJS):

[Urbanproof]

@Urbanproof Will love to see that caching logic 👀

Otherwise feel free to open a draft PR so we can review it

I don't actually have the code any more - I threw it away as it was more of a quick test than an actual patch. I intended to get back to this as soon as I had spare time, and surprise surprise, that time never came. 😁 If you still want help with this I could take another look.

@Urbanproof I think the resolution logic should happen when the extension loads, not when the first save happens, because the delay will still be very noticeable

I thought about this, too, but I wasn't sure if this would cause problems with project setup. If you were to clone a fresh project you wouldn't have the vendor-directory there just yet, so you would have to reload the extension after running composer install. IMHO not an issue, but as a extension authors you might want to consider if this constitutes as a breaking change.

[d8vjork]

I don't actually have the code any more - I threw it away as it was more of a quick test than an actual patch. I intended to get back to this as soon as I had spare time, and surprise surprise, that time never came. 😁 If you still want help with this I could take another look.

Ah, that's sad, anyway I could take a look as well at this, maybe just for the sake of fixing the saving issue when applying refactorings or code actions to lot of files at once

[innocenzi]

I thought about this, too, but I wasn't sure if this would cause problems with project setup. If you were to clone a fresh project you wouldn't have the vendor-directory there just yet, so you would have to reload the extension after running composer install. IMHO not an issue, but as a extension authors you might want to consider if this constitutes as a breaking change.

@Urbanproof good thinking - this good be solved relatively easily by watching file changes though, VSC has a nice API for that 👍

[d8vjork]

@innocenzi It's what it does right now for the pint.json config

About the rest of files I don't know how much will this help, depends on how the VSCode's API detect a file change (if is based on the file content or just last update timestamp)

IMHO not an issue, but as a extension authors you might want to consider if this constitutes as a breaking change.

This will depend as of today extension is on pre-release until I manage to have tests (to also determine what a break change will really be), so it can be v2 or still be v1.x but as a final release, tho I still need to manage the integration with Laravel Sail which was once working and not anymore