-
Notifications
You must be signed in to change notification settings - Fork 39
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
Add a watch mode for automatic formatting on file save #509
Comments
I'm (lightly) opposed to this feature. IMO, it's something your text editor/IDE should be doing instead.
Which editor do you use? Neovim now has support for this (via none-ls): https://github.com/nvimtools/none-ls.nvim/blob/main/doc/BUILTINS.md#nix_flake_fmt, and it doesn't create double work for you. Perhaps you could implement something similar for your editor? |
I don't think I'm representative of most of the community. I regularly switch between vscode, neovim and sometimes helix. And I use nix fmt a lot, but have found the nix flake command to be particularly slow to start compared to running treefmt itself. Having a watch method gives more room for optimisation, as not re-scanning the repository and other startup routines. |
I encourage you to try the nix_flake_fmt builtin for none-ls for neovim that I mentioned earlier. It immediately discovers the nix fmt entry point on startup, and caches the result. In practice, I never find that I have to wait at all for it, because I usually take a few moments after opening a file before I make a change. As for switching between editors, if they don't already have support for |
I was speaking with another user who was finding running via I think this feature would be good to have, happy to review when you feel #510 is ready. |
I have just tried out my idea for an implementation, but it seems that I need to refactor the whole processing workflow. Currently, the implementation waits for the walk to finish before starting the formatting and we need to run in stream way. |
It should batch things up and kick them off in parallel. I'll try and find some time to have a look at your branch. |
@shikanime I looked at your branch and played around a bit locally. I can't push my work-in-progress changes. I think the PR has an option to allow maintainers to push. By default, The processing pipeline doesn't need to be changed. Instead, we must reduce the batch size to 1 when operating in watch mode. This will ensure any batches being produced by the walker are formatted immediately. With that in place, the new problem is an infinite formatting loop where the watch keeps detecting the file being formatted, thus forcing another format. We currently track changes with a simple heuristic based on To avoid this infinite loop, we will need to make the change detection configurable and, when in watch mode, switch to a content-based detection method so we only format when there has been a material change to the underlying path. Introducing the configurable change detection strategy in a separate PR might be best. There is also a stale PR for making batch sizes configurable in general. Perhaps both should be implemented first 🤔 |
Setting the batch size to 1 will work, but won't the process stop on the first iteration? As you can see, I tried something in the PR of #511, it works, but it's dirty and seems to break the expectations of the unit tests. Oh yeah, you're right, I completely forgot that formatting is also a write change, so it gets picked up by fsnotify at least once, I saw that during my test in the last PR. Assuming most of the formatters doesn't reformat files that don't need to be formatted, it's fine? Or is this assumption completely wrong in most or some cases? Good question about batch size, maybe it could be? Should the user be able to configure it from a ux perspective? |
Reading from walkers is supposed to keep happening until they return an In #512 I cleaned up the logic around that section. In my local tweaks of #510, I had to make a minor adjustment, but I can't remember exactly what (I'm on a different device). All tests are passing. I can share it tomorrow on a separate branch or a gist. The formatters aren't responsible for determining whether a file needs to be reformated. That is something We have a caching layer that wraps the walkers to filter out unnecessary formatting. As mentioned above, this must be adjusted to operate content-based when in watch mode to prevent constant re-formatting. Currently, it leans on the |
Is your feature request related to a problem? Please describe.
Yes, currently treefmt requires manual execution to format the code.
Describe the solution you'd like
I propose to add a new format watch mode to treefmt. This mode would allow treefmt to automatically format the code whenever the file is saved to disk. This could be achieved by integrating with IDEs or using a file watcher to detect changes, perhaps using watchman.
Describe alternatives you've considered
One alternative is to manually format the code after each save. However, this is inefficient and easy to forget. Another alternative is to use an IDE extension that provides automatic formatting for treefmt. However, treefmt-nix doesn't export the config file, so implementing it in every IDE would be double work.
Additional context
Afaik, it would be implementing a new walk provider that returns an unbounded number of files to process until we kill the process. If it's relevant, I have a good enough idea of how to implement it if you'd like.
The text was updated successfully, but these errors were encountered: