Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Open file from tree #12

Merged
merged 22 commits into from
Dec 16, 2011
Merged

Open file from tree #12

merged 22 commits into from
Dec 16, 2011

Conversation

njx
Copy link

@njx njx commented Dec 14, 2011

Implemented opening a file by clicking on it in the file tree, and also implemented File > Open to show a file dialog that the user can select from.

As part of this, I created a simple CommandManager to start to decouple parts of the UI from each other. When a file in the tree is clicked, the ProjectManager tells the CommandManager to execute the "file.open" command. This command is registered in brackets.js as part of the boot sequence.

Eventually we can add things like key binding, and perhaps undo/redo handling, to the CommandManager. Also, we should consider factoring out some of the code in brackets.js out into a more formal application controller.

/**
* Registers a global command.
*
* @param id {string} The ID of the command.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fwiw, the official Closure docs put the type before the parameter name. I don't know if it also accepts this reverse order (which I actually think is nicer... so maybe it's worth testing whether this will work?)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's follow the official docs.

@njx
Copy link
Author

njx commented Dec 15, 2011

FYI, this is not yet ready for master--waiting on finalizing Ty's pull request for the open file API.

* Copyright 2011 Adobe Systems Incorporated. All Rights Reserved.
*/

// List of constants for global command IDs.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should use JSDoc comment style for top-level declarations

editor.setValue(event.target.result);
editor.clearHistory();

// In the titlebar, show the project-relative path (if the file is inside the current project)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first time I read this I thought it meant the browser title bar... should we call it something different to disambiguate?

}

reader.readAsText(file, "utf8");
});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does our FileEntry.file() take an error callback as a 2nd arg, like the PhoneGap one? If so, I wonder if we should add a "TODO" error callback like you did above with the reader. That way we'll have TODOs for all known cases of missing error handling.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants