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

integrate ace.js settings with kibibit #118

Closed
neilkalman-redkix opened this issue Apr 26, 2016 · 5 comments · Fixed by #129
Closed

integrate ace.js settings with kibibit #118

neilkalman-redkix opened this issue Apr 26, 2016 · 5 comments · Fixed by #129

Comments

@neilkalman-redkix
Copy link
Contributor

INTEGRATE ACE.JS SETTINGS WITH KIBIBIT

FEATURE

Basically, there are a lot of settings we want to expose to our user through the menuBar or a settings modal we'll have in the future.

We need to start and create functions that will change those settings in the _editor object we get from ace.

basically, things like:

  • line wrapping (checkbox)
  • line length (3-4 options, based on what are the most common line lengths)
  • no gutter (the column with the line numbers)
  • Also, things like show error, warnings, and info might be useful
  • AND 😉 things like code highlight themes, highlight language (select if javascript, etc), but that might take a while so you don't have to do that.

Basically, it will work in the following way:

the settings component should have setters and getters for everything. The logic should be inside the settings service and we should try and avoid as many of these from being done outside of the settings service.

We need the settings service to have everything it needs from other open things to be able and do everything itself. So, it actually needs:

  • a pointer to our _editor variable. We can also have a pointer to our codeEditor component, but I'm not sure if this is needed since each "code editor" will behave according to what it needs and will be in-charge pointing to it's own editor on creation.
  • a pointer to our sidebar tree component (so, on initialization, we should connect the sidebar tree to the settings service. preferably not from inside the tree component, but from the HTML with ng-init)

beyond that, we have everything else we need I think. We can discuss this more if you want to.

Therefore, I introduce the setter-getter approach! 🎉 :

/* Contains all the watched variables. each variable should have a setter and getter. Those are how we bind extra functions to these variables */
    function WatchedVariables() {
      // all defaults should be initialized here
      var lineWrapping = false;

      this.__defineGetter__('lineWrapping', function() {
        return lineWrapping;
      });

      this.__defineSetter__('lineWrapping', function(newVal) {
        // need to check that the value is valid
        console.assert(newVal === true || newVal === false, {
          "message": "lineWrapping should only be a boolean, but was given some other type",
          "currentValue": lineWrapping,
          "newValue": newVal
        });
        // do all the extra work, like: setting the line wrapping of ace.js based on this
        if (newVal !== lineWrapping) {
          settings.editor.setLineWrap(newVal);
          lineWrapping = newVal;
       }
      });
    }

if you need help with this syntax, tell me and I'll try to help.

@neilkalman-redkix
Copy link
Contributor Author

Since this somewhat related to issue #83, We need to combine forces to make sure we both have the same idea about this

@thatkookooguy
Copy link
Member

here's the settings service after I refactored it. you should do something similar with the ace.js.

https://github.com/Kibibit/kibibit-code-editor/blob/new-syntax-for-settings/public/app/services/settingsService.js#L23

@thatkookooguy
Copy link
Member

setter should do something only if the value changed. so, make sure to check before you do that.

console.assert checks if the value of the expression is true. if it's not, it will throw an error and the app will crash. I'm writing it this way since we want to catch every-time we accidentally assign the wrong value to a setting

that's a code problem and not a product problem, so we don't want to catch it gracefully.

we want the settings to be as up to date as available. that's why, in the getter, we want to fetch the property from the module to have the latest value in case something else updated it. Notice how I use

settings.__defineGetter__('isFullscreen', function() {
  return currentFullscreenState();
});

instead of

settings.__defineGetter__('isFullscreen', function() {
  return isFullscreen;
});

and in the defineSetter, I'm checking the newValue against the same call:

if(newValue !== currentFullscreenState()) {...}

@thatkookooguy
Copy link
Member

some more good resources to make editor do things: https://cdn.rawgit.com/whitelynx/web-pgq/master/static/js/directives/editor.js

if you're changing our editor directive, you can (if you want), try and implement the scrollbars (ours is Malihu's jQuery Custom Scrollbar by Manos Malihutsakis), and update the cursor on the changeCursor event.

@thatkookooguy
Copy link
Member

I also suggest that you'll seperate all editor's settings into a specific field. Something like:

settings {
  // general settings here
  isFullscreen: true,
  ...
  editorSettings: {
    lineWrap: true,
    ...
  }
}

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

Successfully merging a pull request may close this issue.

3 participants