Utilize system specific config locations. #991
Conversation
Linux configs utilize the XDG Specification `~/.config/soundnode/` Mac os utilizes what i could gather as `~/Library/Preferences/Soundnode` Not sure about the mac os location though.
app/public/js/system/guiConfig.js
Outdated
| @@ -28,7 +29,19 @@ guiConfig.maximize = function () { | |||
| }; | |||
|
|
|||
| guiConfig.logOut = function () { | |||
| fs.removeSync(`${__dirname}/userConfig.json`); | |||
| let _userConfigPath = `${__dirname}/app/public/js/system/userConfig.json`; // Windows specific for now | |||
There was a problem hiding this comment.
in that case we should add a check if it's windows here too otherwise this will always be the case.
There was a problem hiding this comment.
Let's name something else and not use _.
app/public/js/system/settings.js
Outdated
| const userConfig = JSON.parse(fs.readFileSync(`${__dirname}/userConfig.json`, 'utf-8')); | ||
| const userHome = require('user-home'); | ||
|
|
||
| let _userConfigPath = `${__dirname}/app/public/js/system/userConfig.json`; // Windows specific for now |
There was a problem hiding this comment.
Let's export this to a function in util that returns the userConfigPath
|
I set it up as a class of its own that has no internal dependencies, Did you mean as a function inside the utilsService file, or does this suffice? |
|
this is good @jakejarrett I will make some time to test. Thanks. |
| let userConfigPath = null; | ||
|
|
||
| /** Linux platforms - XDG Standard */ | ||
| if (process.platform === 'win32') { |
There was a problem hiding this comment.
could this also be win64?
There was a problem hiding this comment.
I do not believe so, This was written according to the nodejs documentation for process.platform which only lists win32 (I think it refers to the win32 api, not windows 32bit)
|
@jakejarrett I tested here and it's failing to start the app. Right after clicking on the Facebook button to login. |
This adds a private function (Possibly could abstract it out to a class, but it's not used anywhere else) to simply determine if a given location is a folder & if it exists.
|
Pushed the fix for that 👍 |
|
@weblancaster Any update on this? |
| */ | ||
| const folderExists = folder => { | ||
| let exists = false; | ||
| fs.stat(folder, (err, stats) => { if (stats.isDirectory()) exists = true; }); |
There was a problem hiding this comment.
can you format the code please @jakejarrett
There was a problem hiding this comment.
Use fs.statSync to avoid race conditions since Node is async and we are returning exists before this is done.
| } | ||
|
|
||
| if (!folderExists(userConfigPath)) { | ||
| mkdirp(userConfigPath, err => { if (err) console.error(err); }); |
There was a problem hiding this comment.
please format the code to be multiple lines.
|
@weblancaster Updated code w/ those changes 👍 |
|
@jakejarrett I have pushed a couple of commits with changes based on my suggestion on sync fs but I also refactored to make a bit easier to work with the get functions removing the |
Instead of using class we will use a obj that contain the get methods needed that way making simpler when importing in other files.
Linux configs utilize the XDG Specification
~/.config/soundnode/Mac os utilizes what i could gather as
~/Library/Preferences/SoundnodeNot sure about the mac os location though.
Related Issue(s)
#989 (JavaScript Error when login without ROOT permissions)