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

Initial implementation of file save#16

Merged
gruehle merged 22 commits into
masterfrom
save-file
Dec 17, 2011
Merged

Initial implementation of file save#16
gruehle merged 22 commits into
masterfrom
save-file

Conversation

@njx
Copy link
Copy Markdown

@njx njx commented Dec 16, 2011

Not ready for pulling into master--this is just for initial review.

File save and dirty bit management are implemented, but the save function is using the raw file API instead of the HTML API.

Prompting to save a dirty file on close is not yet implemented.

@njx
Copy link
Copy Markdown
Author

njx commented Dec 16, 2011

Also, Cmd-S is not yet implemented. Use File > Save from the in-app menu bar (not the native menubar).

njadbe added 2 commits December 15, 2011 22:23
…it more generic, driven by HTML. Added basic keymapping functions.
…ted logic of the open/save/close commands to take advantage of this.
@njx
Copy link
Copy Markdown
Author

njx commented Dec 16, 2011

Okay, now Cmd-S is implemented (as are Cmd-O and Cmd-W), using a new keybinding infrastructure. I also refactored the dialog code to make it more generic (so I could use it for the save file case).

The most recent commit will likely be controversial. It makes all commands deal with asynchronicity using $.Deferred(). I think this is the most elegant way to deal with this (and I already have to deal with it because dialog handling is asynchronous), but it definitely introduces some complexity. We should discuss this tomorrow.

Note that the selection in the file tree gets out of sync in various cases (e.g. if you make changes in one file, click on another file, then cancel out of the save changes dialog). We should discuss whether we need to handle this in this sprint.

Comment thread src/brackets.js Outdated
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 this be called showModalDialog instead?

njadbe added 3 commits December 16, 2011 12:55
…out the file handling code into a separate file. Simplified KeyBindingManager. Changed $.Deferred() to new $.Deferred().
Comment thread src/NativeFileSystem.js Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should add Google Closure comments to getFile() function

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.

Done

Comment thread src/NativeFileSystem.js Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

NJ originally had these constants defined as well and then decided to use the native ones and just wrap them. See the class FileError

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.

I didn't see FileException when running in the console, so I added them here.

Comment thread src/FileCommandHandlers.js Outdated
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.

This code path doesn't return a Deferred

Comment thread src/FileCommandHandlers.js Outdated
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.

_editor.clearHistory() is already done on line 117

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.

4 participants