Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #336: add autosave extension #337

Merged
merged 1 commit into from
Jun 3, 2015
Merged

Fix #336: add autosave extension #337

merged 1 commit into from
Jun 3, 2015

Conversation

humphd
Copy link

@humphd humphd commented Jun 2, 2015

To test this, open your filetree, and start changing files. Watch the dot beside your working files (which means they are dirty), and notice it vanish 15s after you make changes!

@humphd
Copy link
Author

humphd commented Jun 2, 2015

@@ -60,7 +61,8 @@ define(function (require, exports, module) {
var extraExtensions = [
"brackets-cdn-suggestions", // https://github.com/szdc/brackets-cdn-suggestions
"ImageUrlCodeHints",
"HTMLHinter"
"HTMLHinter",
"Autosave"

Choose a reason for hiding this comment

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

Why do we need it as an extra extension as well as a default extension? IMO it should just be a default extenstion

Copy link
Author

Choose a reason for hiding this comment

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

I think the bramble extension is getting too big, and I wanted an easy way to turn this feature on/off for user testing.

Choose a reason for hiding this comment

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

So if you do ?disableExtensions=Autosave, even though it's a default extension, we can disable it?

Choose a reason for hiding this comment

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

Actually, just tested it. It doesn't need to be in the extraExtensions list to allow disabling.

Copy link
Author

Choose a reason for hiding this comment

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

Good catch, I forgot to remove it from here. My thinking was to put it in the default list, and yes, you'd be able to disable with ?disableExtensions=Autosave. I'll remove from extra.

@gideonthomas
Copy link

one comment...looks good otherwise and seems to be working fine r+

@gideonthomas
Copy link

Actually, can we move the time before save config value to the env? It seems like it would make more sense for it to be there

@humphd
Copy link
Author

humphd commented Jun 3, 2015

@gideonthomas what does "to the env" mean? We don't have an env for Bramble. I could make these prefs that we can change via PrefManger, I suppose. Do we care that much? Changing this value in the extension is pretty easy: it's a 1 line change; otherwise we have to deal with prefs over in bramble for this extension.

@gideonthomas
Copy link

Oh right, we don't have one in the context of the browser. Ok then nvm

Move Autosave to extensions/default, add save-on-editor-change logic

Fix typo on SAVE_ON_EDTIOR_CHANGE

Remove Autosave from extraExtensions list (added twice by accident)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants