Async transact callbacks

[mauricioszabo]

With Electron 44 removing the synchronous clipboard API, we need to start a better support for async things.

This is kind of a deal breaker for some plug-ins - for example, the TextEditor API supports only synchronous callbacks for transact. Luckily, it only delegates to TextBuffer and we don't actually need too much work to make asynchronous callbacks work (nor do we need to change any API - we just need to treat errors in two different ways and treat the stack in two different ways, but it's localized).

This PR adds support for async callbacks on the transact method, changes no APIs, and it's tested for some edge-cases.

[savetheclocktower]

Aside from the if nitpick, my only thought is that we should enforce a timeout maximum if we're going to allow async transactions — even if it's something really high, like 1000ms — just to guard against something returning a promise that never actually resolves. Otherwise you run the risk of a transaction that opens but never closes.

[savetheclocktower]

Suggested change

	if(result?.then) {
	if (result?.then) {

[savetheclocktower]

Actually, this would be a big problem, now that I think about it.

Currently, synchronousness of transactions guarantees that transactions can contain or be contained by other transactions. Which is fine; it's just a matter of incrementing or decrementing call depth.

If transactions can go async, then a third possibility presents itself: overlapping transactions. This would be disastrous.

When a transaction starts, it marks a position in the undo stack. Then it performs some operations (each of which possibly adds to the undo stack); and when it's done, it calls groupChangesSinceCheckpoint in order to flatten all the things it did into one operation in the undo stack.

If a transaction starts and ends inside another transaction, that's fine; they compose easily! The outermost transaction will be the one that acts last and groups everything into one operation in the stack.

The fact that nothing else can run during the transaction is important! If transactions can go async, then whenever they yield, some other code gets a chance to possibly put an unrelated edit onto the buffer’s undo stack. We lose grouping and atomicity. Once the transaction ends, that unrelated buffer change gets flattened alongside the other operations into a single operation on the undo stack; and if the user chooses Undo, that unrelated change is incorrectly reverted.

---

If we did allow transactions to go async, we'd need some sort of mechanism to restrict access, such as a queue. But I'm not sure that an async clipboard API would necessitate async transactions in the first place.

Suppose this code exists right now:

editor.transact(() => {
  const selection = editor.getSelectedBufferRange();
  atom.clipboard.write(selection.getText());
  selection.setText('');
  // …
});

Assuming we have to bite the bullet and let clipboard methods go async, we might be tempted to allow the following:

editor.transact(async () => {
  const selection = editor.getSelectedBufferRange();
  await atom.clipboard.write(selection.getText());
  selection.setText('');
  // …
});

But even this fix would require touching the code — and if you're touching the code, there's a different fix that is much more sound:

const selection = editor.getSelectedBufferRange();
await atom.clipboard.write(selection.getText());
editor.transact(() => {
  selection.setText('');
  // …
});

We can't restrict what actually happens inside the transaction function, but only things that alter the undo stack actually need to be in the transaction function — everything else can be hoisted outside of it!

I know this example is contrived, and surely there are better ones that will prove trickier to code around. But I think the cure here is worse than the disease.

@mauricioszabo, if you have specific use cases that seem to absolutely require async transactions, let's talk about them.