Write path fixes

[dead10ck]

Edit: this PR was split up, now integration tests are in #2359 and this PR is based on that branch.

Well, I did not expect this PR to get this large, I'm so sorry about that. This PR started as an attempt to fix #1575, and along the way, I discovered that there was a larger problem: Document writes are submitted to a single job pool that all run unordered. I suspected that this could lead to file write inconsistencies if the user submitted multiple writes on the same document at the same time. I took this opportunity to continue work on the integration testing harness that @archseer started a while back. I added an integration test (see below) that confirms that indeed the file writes happen not only out of order, but simtultaneously, leading to the file being clobbered. In a test where we issue a thousand writes that each change the entire file to consist only of the iteration number (i.e., 1, 2, 3, ..., 1000), we can see that the file ends up with a seemingly random jumble:

failures:

---- integration::write::test_write_concurrent stdout ----
thread 'integration::write::test_write_concurrent' panicked at 'assertion failed: `(left == right)`
  left: `"1000"`,
 right: `"4010"`', helix-term/tests/integration/write.rs:66:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    integration::write::test_write_concurrent

This test is now passing with the changes to the write path.

Below is an explanation of some of my decisions in this PR. Opinions and suggestions are welcome!

Document writes

The approach taken was:

1. Rather than submitting a future to the general job pool, each Document manages its own queue of writes with an mpsc channel. When a user submits a write, this merely queues up a write request on the internal channel.
2. Document writes emit a new special event type that communicates the revision number that was written and the path. The main event loop is responsible for polling the futures, which prompt the runtime to carry out the actual writes. When writes finish, the main event loop will update the document's last saved revision and the path if the write was successful.
3. All events that originate from the Editor were consolidated into one enum type. This was discussed in the chat room, and it was originally expressed that we could do this incrementally over time, but the borrow checker intervened with this plan: since the event polls require a mutable borrow, and we can only do this once within the event loop, we have to do this all at once.

Incidentally, this allowed fixes for two unrelated bugs that presently exist:

• When a document write fails, the modified flag is still reset. By sending the revision number in the write event, the current revision number and the last saved revision number need not be the same at any given point in time. This would, e.g., allow large writes to carry out in the background while the user makes other edits at the same time, and correctly keep the modified flag even after the background write succeeds.
• In a write command that sets a new path, if the write fails, the path is still changed to the argument path. Now, it will not update the path unless the write succeeds.

Also, the first attempt at fixing #1575 was done by adding fallibility to the job pool. Presently, if you join on all the jobs, any errors are discarded.

Since I was already here, I attempted to fix #1633 as well, and it kind of works, except that the status message quickly gets overwritten by LSP progress messages. I'm not quite sure how to fix this, but I think that can happen in another PR, as this one is quite big enough already.

Edit: integration tests were split off into their own PR: #2359

Fixes #3975
Fixes #2518
Fixes #3727

[dead10ck]

Not really sure what's going on with the EAGAIN errors. Trying to dig into it. I'm guessing it has something to do with the terminal in the build environment.

[the-mikedavis]

Would it be possible to split the PR up into two - one for the integration tests and one to fix document writes (probably based on the integration testing one)? It is quite large as you say 😅

[dead10ck]

Sure, I have some of the changes mixed up, but I'll see if I can rebase and split them up

[pickfire]

I think you accidentally included out log file.

[dead10ck]

I think you accidentally included out log file.

Thanks! I fixed this. Though as a side note, if you'd like to review, I'd recommend taking a look at #2359 first, as this PR is based on that one. A review would be appreciated. 🙂

[pickfire]

We usually call it text instead of doc.

[pickfire]

Hmm, I wonder why editors don't show user the revision, if it did then users are able to use time travel easily. Like :ear 1f, but yeah the issue could be that even if we show it, user don't know the range they could use for time travel, like 1 or 0 is the min but don't know the max if they travel back.

[archseer]

Can you rebase this now? :)

[dead10ck]

Yep, will do next time I get a chance. Although I meant to make a comment here: I've been dogfooding this branch for a little while, and I've noticed one issue with the changes to the revision number tracking: when an auto format happens, helix doesn't mark it as unmodified because the revision number it saves for sending across the channel happens before the formatting change, which itself increases the revision number. I will have to think about how to fix this. I may have to split up the format and save operations into discrete steps so it can do the formatting first and then save the revision number before issuing the write operation.

[archseer]

It's time! I want to refactor things further to get rid of the Callback enum, but we should ship these write fixes first

[dawkrish]

I am not able to write to a file by helix but I am able to do with vim, what is the reason ?

[dawkrish]

nevermind, it got automaticaly fixed