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

Deleting Prefs File Causes Headache #7220

Closed
JeffryBooher opened this issue Mar 18, 2014 · 12 comments
Closed

Deleting Prefs File Causes Headache #7220

JeffryBooher opened this issue Mar 18, 2014 · 12 comments
Assignees
Milestone

Comments

@JeffryBooher
Copy link
Contributor

Deleting your prefs file causes a world of hurt:

  1. Delete your prefs file ~/config/Brackets/brackets.json
  2. Restart brackets.
    You are warned that the config file is missing.
  3. Open the prefs file for editing from the debug menu
  4. Quit brackets
    You are warned that "brackets failed to read config file and it will be opened on startup"

At some point I had an empty prefs file and could toggle spaces to tabs and it wouldn't update the empty file with the preferences change but I cannot seem to reproduce that at the moment.

IMHO we should just silently create the file with the default settings if it is missing. There's no need to tell the user and/or open the file for their review in that case.

The Failed to Read prefs warning is alarming to the user -- we just use the default settings in that case anyway, you just can't save changes. I would prefer some other affordance and skip the whole preferences corrupt warning on startup. Since we just use the defaults in that case, it may be worth while displaying any corrupt message until the user tries to change a preference. I'm not sure how difficult it is to make that distinction, though.

Brackets' edge is to get into the editor quickly and have less in your way. This dialog violates both of thoese treaties. Perhaps @larz0 can come up with an affordance for things this like a status bar indicator that the user can deal with when they get bored or lonely.

@JeffryBooher JeffryBooher added this to the Brackets 1.0 milestone Mar 18, 2014
@peterflynn
Copy link
Member

This does seem odd. On first launch there's no brackets.json file either, and we happily create a blank one without showing any warnings.

I think the error dialog is intended for the case where the file is present, but its syntax is invalid. In that case we really do want to show something in-your-face -- since the choices are to overwrite the file (losing the user's old prefs), or silently run with non-persisted prefs (losing the user's new pref changes), and both of those are pretty bad without explanation or a chance to intervene.

@peterflynn
Copy link
Member

The bug seems more narrowly scoped for me. I did the following steps:

  1. Launch Brackets
  2. Delete brackets.json
  3. Reload
  4. Debug > Open Preferences File
  5. Reload

Result:
No error message after step 3... (@JeffryBooher, are there additional steps you needed to take to trigger this' missing file' warning?)
After step 5, error message saying the prefs file is invalid. (I guess since it's blank). This seems easy to fix.

Medium priority @dangoor since the blank-file gotcha seems easy to hit (seeing as how we create the file for you).

@dangoor
Copy link
Contributor

dangoor commented Mar 18, 2014

Agreed that a blank file needs to be treated as a valid one.

@marcelgerber
Copy link
Contributor

I think that should be fixed in Sprint 38 instead of 1.0

@peterflynn
Copy link
Member

Yeah, I agree (given that our own menu command foregrounds this case) -- switching milestone

@peterflynn peterflynn modified the milestones: Release #38, Brackets 1.0 Mar 19, 2014
dangoor added a commit that referenced this issue Mar 20, 2014
@JeffryBooher
Copy link
Contributor Author

@peterflynn Usually comments with my name in them percolate in to my inbox but, for some reason, I never saw this one. The steps that I went through to reproduce this wasn't a reload of Brackets it was a Restart of Brackets which means quitting altogether, deleting the prefs file and restarting brackets. Not deleting it then reloading it or restarting. I'm not sure why we weren't warned before when there wasn't a prefs file but opening the prefs file and saving an empty one caused an invalid case as does a missing comma, etc...

I don't think we write to the file if it's corrupt. In fact I had a corrupt file (missing comma) and changing the spaces to tabs did not write so I don't think there is a risk of us overwriting the prefs by not presenting it to the user.

It's hard to imagine a legitimate case that could cause the corruption of the prefs file without some sort of human intervention -- there would have to be some sort of algorithmic failure in @dangoor's code that corrupts the JSON or possibly an extension that doesn't use the prefs api to write to that file and is ill-behaved. Quite possibly a virus.

But I still think there are users who want to launch brackets from Explorer or Finder to edit a specific js file (right click on js file > open with brackets) and they get this warning dialog that the prefs are all screwed up. Now they are thrown into looking at some JSON that they have to deal with which may or may have been caused by them and now they have to figure out what's wrong and now they're derailed.

I agree that we need to tell them and provide them an easy way to solve the problem but how many times have you heard users get frustrated with Windows because of Registry errors?

@dangoor
Copy link
Contributor

dangoor commented Mar 21, 2014

There's no warning when the user prefs file is missing, because none is needed. The FileStorage for user prefs is configured to automatically create the file if it's missing. The case of a blank user prefs file is a very specific one.

I think the only times the prefs file would be readable but unparseable would be if the user edited the file. I'd consider a bug in JSON.stringify to be pretty unlikely. Given that, I think we're doing the right thing here (opening the prefs file and letting the user fix the problem). If we can't read the prefs file and we go to save over top of it, we would blow away all of their user-level prefs, which would be bad.

Be aware that changing spaces to tabs could write to a project-level prefs file and not user-level prefs, if that's where the useTabChar setting is set. I believe Brackets' .brackets.json file sets useTabChar.

@redmunds
Copy link
Contributor

FBNC to @JeffryBooher

@peterflynn
Copy link
Member

Yeah, I agree with Kevin -- if the user has hand-edited the prefs file and made it invalid, we should have an in-your-face notification of that. The fact that we drop future UI-driven prefs changes on the floor while in this state is another good reason to show a notification.

@alexanderkatz
Copy link

Hey,
I just started using Brackets and it is a fantastic editor. I wanted to weigh in on this issue because I was personally affected. I am not sure what version of Brackets I originally downloaded, but after downloading the Sprint 37 build and opening Brackets, I immediately saw the warning about my brackets.json file (brackets.json opened and was totally blank).
I was confused because I had no recollection of ever seeing this file, yet alone editing it. I also didn't know how to fix the problem.
I agree with the previous suggestions, but in cases where users are warned it might be helpful to provide a link to a sample brackets.json file or further explanation.

https://github.com/adobe/brackets/wiki/How-to-Use-Brackets#preferences

-Alex

@JeffryBooher
Copy link
Contributor Author

Confirmed fixed. Closing

@JeffryBooher
Copy link
Contributor Author

/cc: @dangoor

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

No branches or pull requests

6 participants