Minor PSA, 'observeTextEditors' can cause callback stacking


#1

Brief: a buffer open in 2+ panes can receive the same callback from multiple editors, because each pane is an editor, but there’s only one buffer.

Perhaps obvious for some, but the problem is, Atom appears to recommend solutions that create the problem >.<

Here is the case, from the Atom docs:

A single TextBuffer can belong to multiple editors […] if the same file is open in two different panes

Also from Atom docs, a typical use case/recommendation of ‘observeTextEditors’. Many of these textEditor methods are actually quite concerning in light of this behavior, just the existence of these methods is odd !

atom.workspace.observeTextEditors (editor) ->
  editor.insertText('Hello World')

So let’s design a simple test for our init script and observe the stacking behavior

atom.workspace.observeTextEditors (editor) ->
   editor.onDidSave () -> editor.insertText('Hello World\n')
# Reload > split right > split right > split right > and save!

We will see a few ‘Hello Worlds’ after saving, instead of just one. Different editors issued their own callback, but each applied it to the same buffer. Oddly, the behavior will persist so long as one of the files remains open.

Luckily, this problem can be averted by enabling the tree view setting ‘Always Open Existing’. It is usually disabled!


#2

The behavior will persist because you’re not properly cleaning up your event handlers. A better written example would be:

atom.workspace.observeTextEditors (editor) ->
  disposable = editor.onDidSave -> editor.insertText('Hello World\n')

  editor.onDidDestroy -> disposable.dispose()

#3

:scissors: *trim

And why would this non-existent editor emit an onDidSave event? :eyeglasses: Also, let’s not forget that the behavior occurs in the first place, whether we are disposing or not.

This certainly relates to model/view seperation. It is part of the design of Atom and can be worked around, and there are advantages, but there are disadvantages too. I think a good suggestion in this case would be a little less of observeTextEditors and a little more of obverseTextBuffers, a method that I don’t believe exists yet, but definitely should.


#4

I think the docs could be a little more explicit about this, but here’s the problem: Text editors don’t have an onDidSave event. Some functions in TextEditor (like onDidSave/save) just call the same function in the underlying TextBuffer instance. In your example, you think that each editor has one onDidSave callback which gets destroyed when the editor gets destroyed, but really you have one TextBuffer with 4 onDidSave callbacks. Even if you destroy an editor, the callback you registered in observeTextEditors will remain as long as at least one editor with the same buffer remains. That’s why you should always clean up editor callbacks with editor.onDidDestroy.

Something like that actually exists, but it’s undocumented. In theory, there is a atom.project.eachBuffer(callback) which AFAIK acts like an observeTextBuffers. Unfortunately, it has a bug and doesn’t work (I guess that’s why it’s undocumented :wink:). But you can easily implement it yourself:

atom.project.observeTextBuffers = (callback) ->
  callback(buffer) for buffer in @getBuffers()
  @onDidAddBuffer callback

If you don’t want to patch atom.project:

for buffer in atom.project.getBuffers()
  # Do things with buffer
atom.project.onDidAddBuffer (buffer) ->
  # Do the same things with buffer

But remember to clean up with buffer.onDidDestroy.


#5

I don’t mind a patch : ) but I think we would have some trouble implementing it for our example.

Look at when we try to call insertText, insertText is a textEditor method, and is quite sophisticated, I had to make a call to getActiveTextEditor() which really dirtied up and complicated things.

module.exports = HelloWorld =

   activate: ->
      @composite = []
      @composite.push atom.project.observeTextBuffers (buffer) =>
         @composite.push buffer.onDidSave ->

            # get an editor
            editor = atom.project.getActiveTextEditor()
            # is it the right editor?
            if editor.buffer == buffer
               editor.insertText 'Hello World\n'
            # else
               # ???
               # cycle through editors! ?
               # what if it's remote? is that a thing?

   deactivate: ->
      sub.dispose() for sub in composite

# patch
atom.project.observeTextBuffers ?= (callback) ->
  callback(buffer) for buffer in @getBuffers()
  @onDidAddBuffer callback

Now we need to iterate through the active text editors, and I guess find an appropriate one, this is on top of patching in a core function. IMO it’s easier to dance around the drawbacks of ‘observeTextEditors’ using @leedohm’s advice and similar.

But if we’re going for observeTextBuffer should we try to hack a back-reference to an editor? Or maybe the best policy would be create a headless editor ? Kind of interesting idea to me.