Open core-ai-bot opened 3 years ago
Comment by peterflynn Tuesday Mar 18, 2014 at 18:14 GMT
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.
Comment by peterflynn Tuesday Mar 18, 2014 at 19:29 GMT
The bug seems more narrowly scoped for me. I did the following steps:
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).
Comment by dangoor Tuesday Mar 18, 2014 at 19:56 GMT
Agreed that a blank file needs to be treated as a valid one.
Comment by MarcelGerber Wednesday Mar 19, 2014 at 19:59 GMT
I think that should be fixed in Sprint 38 instead of 1.0
Comment by peterflynn Wednesday Mar 19, 2014 at 20:02 GMT
Yeah, I agree (given that our own menu command foregrounds this case) -- switching milestone
Comment by JeffryBooher Friday Mar 21, 2014 at 07:47 GMT
@
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?
Comment by dangoor Friday Mar 21, 2014 at 15:05 GMT
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
.
Comment by peterflynn Friday Mar 21, 2014 at 17:31 GMT
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.
Comment by alexanderkatz Sunday Mar 23, 2014 at 14:07 GMT
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
Issue by JeffryBooher Tuesday Mar 18, 2014 at 01:40 GMT Originally opened as https://github.com/adobe/brackets/issues/7220
Deleting your prefs file causes a world of hurt:
~/config/Brackets/brackets.json
You are warned that the config file is missing.
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.