Refactor for asynchronous saving #4
Conversation
When saving a document, Nova would save the file, then Black would apply the formatting after the save. This left the document as unsaved _after an explicit save_. To ensure that nova waits for the Black process to finish, Nova's onWillSave method needs to return a Promise. It will wait for 5 seconds while that promise resolves, ensuring that the document will be reformatted first and saved second. This refactor attempts just that. The main refactor is around the execute method of BlackProcess and the format method of Formatter. execute now returns a promise. If the promise resolves, it passes the formatted text to the formatter, which calls the edit function on the document to make the change. If the promise rejects it represents a failure in some way. The formatter catches this rejection and creates a notification for the user. It's often (but not always) the case that the error is something the user would want to know about, like a malformed python document. This also * Formatted the edited files through Prettier * removed `didExit` from BlackProcess * Removed some unused async declarations from BlackProcess * Formatter returns the results of BlackProcess.execute * main returns the result of Formatter.format
|
(Sorry for the late reply, we're deep into the long summer holidays here.) This is wonderful, thanks for looking into this. I'm happy to merge this, but first I wonder if I could be a pain and ask you to split the changes out into separate commits. At the moment it's hard to see the changes required to support asynchronous saving due to the lines altered purely for formatting. I'd prefer it if there were two commits, one for async support and one for formatting. I think this might mean a new pull request — sorry! — but do you think you have the time to do that before a merge? |
There's nothing wrong with the formatting changes, but they're not related to the PR itself and I like to keep things atomic.
|
I've finally got around to looking at this again and I removed the Prettier formatting changes in 8677044. There's no problem with the formatting itself of course, I just have a preference for atomic commits. The problem I have now, unfortunately, is that I neither write much Python code nor do I use Nova. I don't even have a Python environment set up on my laptop 😭. But I'll try and test this soon and get a new version of the extension up on panic.com. |
There's a bunch here and I'm happy to explain any of it. I also totally understand if this is not a solution you're looking for. Feel free to use this as inspiration for a different solution. I won't be offended at all.
When saving a document, Nova would save the file, then Black would
apply the formatting after the save. This left the document as unsaved
after an explicit save. To ensure that nova waits for the Black
process to finish, Nova's onWillSave method needs to return a Promise.
It will wait for 5 seconds while that promise resolves, ensuring that
the document will be reformatted first and saved second. This refactor
attempts just that.
The main refactor is around the execute method of BlackProcess and the
format method of Formatter. execute now returns a promise. If the
promise resolves, it passes the formatted text to the formatter, which
calls the edit function on the document to make the change. If the
promise rejects it represents a failure in some way. The formatter
catches this rejection and creates a notification for the user. It's
often (but not always) the case that the error is something the user
would want to know about, like a malformed python document.
This also
didExitfrom BlackProcessFixes #1