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

Can now add color schemes as separate json files in {dir}/themes #749

Closed
wants to merge 1 commit into from

Conversation

AviFS
Copy link

@AviFS AviFS commented Jul 27, 2021

Let me know how it looks! I'll be more than happy to refactor it if the project style is different.

I have @abrudz's blessing here to implement the idea. Of course, whether or not this implementation hits the mark is TBD.

@AviFS
Copy link
Author

AviFS commented Jul 27, 2021

As hacky as it is, if we doubt the safeness, we can just wrap the piece that I committed in a big try/catch:

// Add JSON color schemes from {path}/themes/ to SCMS
// The {path} is the same as in {path}/prefs.json
try {
    let SCMScustom;
    const fs = nodeRequire('fs');
    const el = nodeRequire('electron').remote;
    const dirPath = `${el.app.getPath('userData')}/themes/`;
    fs.readdirSync(dirPath).forEach(file => {
      // Get JSON file contents & validate files
      let fileContents; let fileJSON;
      try { fileContents = fs.readFileSync(dirPath+file, {encoding: "utf8"}); } 
      catch(e) { console.error(`Couldn't open themes/${file}.`); return; }
      try { fileJSON = JSON.parse(fileContents); }
      catch (e) { console.error(`themes/${file} could not be parsed as JSON.`); return; }

      // If files validated, then add scheme to SCMScustom
      if (fileJSON.name && fileJSON.theme && fileJSON.styles) { SCMScustom.push(decScm(fileJSON)); }
      else { console.error(`themes/${file} missing one or more of 'name', 'theme' or 'styles'.`) }
  })
}
catch (e) { console.error("See PR #749: " + e); }
// If everything worked, we can safely modify SCMS
finally { SCMS.push(SCMScustom); }

It looks silly, but worst-case scenario, it would just fall back to the behavior implemented before, while leaving a trace. The built-in themes would stay put in SCMS, and the prefs.json are already loaded into D.prefs.colourSchemes. It just wouldn't add the ones in themes/. No variables outside of the scope get modified, unless it runs successfully, in which case all custom themes are added to SCMS.

NOTE: I'm still working on the syntax of this version. Should be finalized in a few minutes.

@abrudz
Copy link
Contributor

abrudz commented Jul 27, 2021

I'm closing this for now, pending conclusions from #751

@abrudz abrudz closed this Jul 27, 2021
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.

None yet

2 participants