Improving the Document APIs

[DennisKehrig]

First a few snippets for reference:

$(DocumentManager).on("currentDocumentChange", onCurrentDocumentChange);
function onCurrentDocumentChange(event) {
    var doc = DocumentManager.getCurrentDocument();
    ...
}

$(DocumentManager).on("documentSaved", onDocumentSaved);
function onDocumentSaved(event, doc) {
    ...
}

$(doc).on("change", onChange);
function onChange(event, doc, change) {
    ...
}

$(doc).on("deleted", onDeleted);
function onDeleted(event) {
    var doc = event.target;
    ...
}

Inconsistent Handler Parameters

Currently there are three different ways to access the document: sometimes it is passed as a parameter to the event handler, or it is the event target, or we can access the current document via the DocumentManager.

Personally I'd like there to be an explicit doc parameter in all cases:

function onCurrentDocumentChange(event, doc) {
    ...
}

function onDeleted(event, doc) {
    ...
}

Inconsistent Event Manager

It makes sense to register currentDocumentChange with the DocumentManager as it is an event related to the set of documents the user is dealing with, not any particular document.

For all other events I would tend to register them with the document itself. This is the case for the change and deleted event, but not for the saved event, which I have to register for via the DocumentManager.

Registering with the DocumentManager has the advantage that I only have to do it once. It has the disadvantage that whenever something happens to any document (it was saved, changed, ...) I have to decide over and over again whether I actually care, i.e. whether I can handle that file type at all, etc.

Ambiguous Event Names

Currently two styles are used: present tense (currentDocumentChange, change), past tense (documentSaved, deleted)
The same verb is used for different events: "currentDocumentChange" is used when the user switches to a different document, "change" is used when a document is modified. Nothing keeps me from interpreting onCurrentDocumentChange as "when the current document was modified".

Missing Event Details

currentDocumentChange currently does not provide any information at all. This means we have to figure out not just which document is now open (if any), but also which document was open before (for cleanup). The latter requires storing the new document in a variable somewhere outside the handler so I still have access to it when currentDocumentChange is triggered. My suggestion would be another parameter:

function onCurrentDocumentChange(event, doc, previousDoc) {
    ...
}

After all, a change occurs from something to something else.

Synchronization

As an extension developer I usually want to deal with the current open document, so I listen to currentDocumentChange. But extensions are loaded after Brackets restores its state, so by the time I register for currentDocumentChange, a document has already been opened (usually) and I have to call my handler manually to notice that.

With jQuery's document ready event, the behavior is different: if the DOM is ready before you register for the event, the handler just gets called right away. Granted, this is more intuitive for document ready, since it describes a state ("ready"), not a transition ("change").

I see three alternatives:

1. when registering for currentDocumentChange, just call the handler if there already is a document.
2. Like 1, but with a name for the event that suggests this behavior
3. Load extensions before restoring the previous state of Brackets

Alternative 3 generally makes sense, although it is tricky to implement (extensions have to tell Brackets when they are fully loaded, which might require asynchronous actions) and might slow down the startup of Brackets - or at least the time until it's fully usable.

However, for extensions loaded later (with the extension manager), alternative 3 would still not be enough.

Files vs. Documents

I wanted to do something with all files the user opened. I assumed those were the files listed in the top left corner (not those below, in the directory tree). I found getAllOpenDocuments() which seemed to match my intention. To my surprise, it did not list all the files I considered to be "open". It listed the current file, sure enough, and also some other files (probably files used by the current file), but some were missing. Huh.

So I had a look at getWorkingSet(). Previously I assumed that the working set refers to the files in the folder that I opened, but I was wrong. The working set actually means what I considered to be "open files". However, it does not contain objects of type Document, it contains objects of type FileEntry.

Alright, luckily there is getDocumentForPath. So I'd just call that and get my documents. And then I would look at doc.extension to decide whether I can actually handle that file - but no. doc.extension isn't always set! It's set somewhere in LiveDevelopment.js by _setDocInfo, along with the file URL that I use so often. So I have to extract the extension myself. Cumbersome!

So for one I would enhance Document to always provide the extension (for filtering) and the URL (for LiveDevelopment stuff, but even before we're live). Maybe URLs should even be the primary designators for files since Brackets is supposed to run in the Browser, too - I might not have access to any local files there. Then again, in the cloud scenario, those files might still be read by a server process that can make sense of paths. That's for you to decide.

But also I wonder whether we can't get rid of the separation between documents and file entries, at least for the working set. A document does not have to be loaded - it's enough to have it load automatically when calling getText(), for instance. So I doubt efficiency is the issue here.

Maybe extension and url actually belong to files. So I would call doc.file.extension, doc.file.url, doc.file.fullPath, etc. - that makes sense, too.

[pthiess]

Reviewed - assigned Peter