Add nix fmt support#102
Conversation
303bfc7 to
e3f6c26
Compare
JojOatXGME
left a comment
There was a problem hiding this comment.
Looks good. I would merge the PR (assuming you don't object for whatever reason) and release the changes on the weekend.
I also want to thank you again for your contributions over last few months. 🚀
| ), | ||
| CommandSuggestionsPopup.Suggestion.builtin( | ||
| "<html>Use <b>nix fmt</b> from flake.nix</html>", | ||
| "nix --extra-experimental-features \"nix-command flakes\" fmt" |
There was a problem hiding this comment.
thought 💭: Just wondering how reliable that will work as intended for different configurations of nix fmt. Specifically, I have recently used nixfmt-tree, and I think it will just reformat the entire repository if no argument is provided, not stdin as expected by this plugin.
Anyway, that should not block the merge of this PR. It would probably be noticeably more effort (and may include some "design-work") to make it work reliable across the board.
There was a problem hiding this comment.
Right now, we always read the file content to pass it as stdin to the called process and get the formatted content from stdout. We could add an option to pass the file by path instead of content (and do the same for the formatted output).
I don't mind going the extra mile and implementing this as well, I think it would cover most cases.
But you can merge this as-is, I'll try to find some time to implement this after my vacations.
There was a problem hiding this comment.
Yes, makes sense.
Note that you will probably have to call FileDocumentManager.getInstance().saveDocument(document) or ManagingFS.getInstance().flushPendingUpdates(virtualFile) before starting the formatter. Not sure which one it is. This was recently announced at Async VFS Disk Writes: What Plugin Authors Should Check
You're welcome, thank you for the time and effort you put into maintaining this tool :-) |
fixes #97 using suggested implementation
nix fmtsuggestion for the format command