Skip to content
This repository was archived by the owner on Sep 6, 2021. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
fa0942a
Implementation of opening file from the tree, using a simple CommandM…
Dec 14, 2011
ffbe5fc
Implementation of opening file from the tree, using a simple CommandM…
Dec 14, 2011
66dc4db
Merge branch 'open-file-from-tree' of github.com:adobe/brackets into …
Dec 14, 2011
bee53cb
Merge branch 'initial-file-tree' into open-file-from-tree
Dec 14, 2011
e222f45
Updated to work with latest ProjectManager changes
Dec 14, 2011
9820db4
Merge remote-tracking branch 'origin/master' into open-file-from-tree
Dec 14, 2011
0ebaefe
Update editor toolbar to show current file path
Dec 14, 2011
6ab2c44
Created Commands file for command ID constants. Fixed async handling …
Dec 14, 2011
0ae3aa9
Improve handling of title bar path
Dec 14, 2011
0fa72df
Merge remote-tracking branch 'origin/initial-fileio-work' into open-f…
Dec 14, 2011
e448ecc
Merge remote-tracking branch 'origin/initial-fileio-work' into open-f…
Dec 15, 2011
1531bc2
Changed file-open code to use HTML API
Dec 15, 2011
31a4b78
Clear history after opening a new file
Dec 15, 2011
0862720
Merge remote-tracking branch 'origin/initial-fileio-work' into open-f…
Dec 15, 2011
ead9cd5
Merge remote-tracking branch 'origin/initial-fileio-work'
Dec 16, 2011
338b2eb
Merge branch 'master' of github.com:adobe/brackets
Dec 16, 2011
35da1ef
Merge branch 'master' into open-file-from-tree
Dec 16, 2011
0ad40b8
Code review cleanup
Dec 16, 2011
2c707fa
Merge from master
Dec 16, 2011
9fa9d1c
Move definition of showErrorDialog out of document.ready into top level
Dec 16, 2011
2241ef9
Fixed bug in previous merge
Dec 16, 2011
c38918f
Final code review cleanup
Dec 16, 2011
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions src/CommandManager.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
* Copyright 2011 Adobe Systems Incorporated. All Rights Reserved.
*/

var CommandManager = {};

CommandManager._commands = {};

/**
* Registers a global command.
*
* @param {string} id The ID of the command.
* @param {function} command The function to call when the command is executed. Any arguments passed to
* execute() (after the id) are passed as arguments to the function.
*/
CommandManager.register = function(id, command) {
if (CommandManager._commands[id]) {
throw new Error("Attempting to register an already-registered command: " + id);
}
CommandManager._commands[id] = command;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should probably assert/throw here if the same id has already been registered.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I'll do that for now...though we should think about whether we want to allow people to redefine existing commands. (I can think of reasons not to allow that :), but it's worth considering.)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yeah I agree -- eventually extensions at least will need to be able to overwrite. But for maintainability, it's also really nice to get warnings if you accidentally register the same command id twice. Maybe eventually we could ask callers to explicitly specify whether they intend to override an existing handler (a la "@OverRide" in Java / "override" in AS)?

}

/**
* Runs a global command. Additional arguments are passed to the command.
*
* @param {string} id The ID of the command to run.
*/
CommandManager.execute = function(id) {
var command = CommandManager._commands[id];
if (command) {
command.apply(null, Array.prototype.slice.call(arguments, 1));
}
else {
console.log("Attempted to call unregistered command: " + id);
}
}
10 changes: 10 additions & 0 deletions src/Commands.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/*
* Copyright 2011 Adobe Systems Incorporated. All Rights Reserved.
*/

/**
* List of constants for global command IDs.
*/
var Commands = {
FILE_OPEN: "file.open"
};
3 changes: 3 additions & 0 deletions src/ProjectManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -191,5 +191,8 @@ ProjectManager._renderTree = function(treeDataProvider) {
// file because jsTree insists on loading one itself)

strings : { loading : "Loading ...", new_node : "New node" } // TODO: localization
})
.bind("select_node.jstree", function(event, data) {
CommandManager.execute(Commands.FILE_OPEN, data.rslt.obj.data("entry").fullPath);
});
};
119 changes: 90 additions & 29 deletions src/brackets.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,43 +2,42 @@
* Copyright 2011 Adobe Systems Incorporated. All Rights Reserved.
*/

// TODO: break out the definition of brackets into a separate module from the application controller logic

// Define core brackets namespace
brackets = window.brackets || {};

brackets.inBrowser = !brackets.hasOwnProperty("fs");

/**
* General purpose modal error dialog.
*
* @param {string} title The title of the error dialog. Can contain HTML markup.
* @param {string} message The message to display in the error dialog. Can contain HTML markup.
*/
brackets.showErrorDialog = function(title, message) {
var dlg = $("#error-dialog");

// Set title and message
$("#error-dialog-title").html(title);
$("#error-dialog-message").html(message);

// Click handler for OK button
dlg.delegate("#error-dialog-ok", "click", function(e) {
dlg.modal(true).hide();
});

// Run the dialog
dlg.modal(
{ backdrop: "static"
, show: true
}
);
}

$(document).ready(function() {

/**
* General purpose modal error dialog.
*
* @param {string} title The title of the error dialog. Can contain HTML markup.
* @param {string} message The message to display in the error dialog. Can contain HTML markup.
*/
brackets.showErrorDialog = function(title, message) {
var dlg = $("#error-dialog");

// Set title and message
$("#error-dialog-title").html(title);
$("#error-dialog-message").html(message);

// Click handler for OK button
dlg.delegate("#error-dialog-ok", "click", function(e) {
dlg.modal(true).hide();
});

// Run the dialog
dlg.modal(
{ backdrop: "static"
, show: true
}
);
}

var myCodeMirror = CodeMirror($('#editor').get(0), {
value: 'var myResponse="Yes, it will be!"\n'
});
var editor = CodeMirror($('#editor').get(0));

// Load a default project into the tree
if (brackets.inBrowser) {
Expand All @@ -56,6 +55,10 @@ $(document).ready(function() {
ProjectManager.openProject();
});

// Implements the "Open File" menu
$("#menu-file-open").click(function() {
CommandManager.execute(Commands.FILE_OPEN);
});

// Implements the 'Run Tests' menu to bring up the Jasmine unit test window
var testWindow = null;
Expand Down Expand Up @@ -117,4 +120,62 @@ $(document).ready(function() {
});
});*/

// Utility functions
function doOpen(fullPath) {
if (fullPath) {
var reader = new NativeFileSystem.FileReader();

// TODO: we should implement something like NativeFileSystem.resolveNativeFileSystemURL() (similar
// to what's in the standard file API) to get a FileEntry, rather than manually constructing it
var fileEntry = new NativeFileSystem.FileEntry(fullPath);

// TODO: it's weird to have to construct a FileEntry just to get a File.
fileEntry.file(function(file) {
reader.onload = function(event) {
// TODO: have a real controller object for the editor
editor.setValue(event.target.result);
editor.clearHistory();

// In the main toolbar, show the project-relative path (if the file is inside the current project)
// or the full absolute path (if it's not in the project).
var projectRootPath = ProjectManager.getProjectRoot().fullPath;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

  1. It'd be good to have a comment over this block saying what the string manipulation is doing...
  2. So it looks like it's making the path project-root-relative where possible and leaving it absolute otherwise. Is there a reason we need to have it be relative sometimes? Seems simpler to just always use absolute if it will work. (And if absolute paths / paths outside the project eventually won't work, we should have some sort of error UI instead of crashing or silently failing to open it).

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

This is just for the titlebar. The idea is that if the file is in the project, the title shows the path relative to the project to avoid redundancy, whereas if it's outside the project, it shows the full path. I'll add a comment.

if (projectRootPath.length > 0 && projectRootPath.charAt(projectRootPath.length - 1) != "/") {
projectRootPath += "/";
}
if (fullPath.indexOf(projectRootPath) == 0) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe a little overly edge-casey, but will this fail for cases like: projectRootPath="/a/b", fullPath="/a/bfoo" ? (this issue couldn't happen if projectRootPath is guaranteed to end in a "/", but I'm not sure if that's true)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed.

fullPath = fullPath.slice(projectRootPath.length);
if (fullPath.charAt(0) == '/') {
fullPath = fullPath.slice(1);
}
}
$("#main-toolbar .title").text(fullPath);
};
reader.onerror = function(event) {
// TODO: display meaningful error
}

reader.readAsText(file, "utf8");
},
function (error) {
// TODO: display meaningful error
});
Copy link
Copy Markdown
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.

}
}

// Register global commands
CommandManager.register(Commands.FILE_OPEN, function(fullPath) {
if (!fullPath) {
// Prompt the user with a dialog
NativeFileSystem.showOpenDialog(false, false, "Open File", ProjectManager.getProjectRoot().fullPath,
["htm", "html", "js", "css"], function(files) {
if (files.length > 0) {
doOpen(files[0]);
}
});
}
else {
doOpen(fullPath);
}
});

});
6 changes: 4 additions & 2 deletions src/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,10 @@
<script src="thirdparty/jstree_pre1.0_fix_1/jquery.jstree.js"></script>

<script src="NativeFileSystem.js"></script>
<script src="brackets.js"></script>
<script src="CommandManager.js"></script>
<script src="Commands.js"></script>
<script src="ProjectManager.js"></script>
<script src="brackets.js"></script>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmm.. currently, brackets.js has a few bootstrappy things that feel like they should come before other files (declaring the brackets namespace and setting up the isBrowser flag). But it also has business logic stuff (the commands) that feel like they should come after other files. Should we maybe split this into two separate files?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Leaving this the way it is for now--no real top level code should be running before $(document).ready() anyway, and stuff like the brackets namespace will be taken care of properly when we move to modules.

<script src="strings.js"></script>
</head>
<body>
Expand Down Expand Up @@ -73,7 +75,7 @@
<input class="settings" type="button" value="Settings"/>
-->
</div>
<div class="title">will-brackets-be-awesome.js</div>
<div class="title">Untitled</div>
</div>
<div id="editor"></div>
</div>
Expand Down