angular-ui / ui-ace

This directive allows you to add ACE editor elements.
http://angular-ui.github.io/ui-ace
MIT License
578 stars 172 forks source link

feat(directive): require and advanced options #60

Closed ciel closed 10 years ago

ciel commented 10 years ago

add "require" array for specifying ace.require loads, and add "advanced" object in the directive options for specifying unlisted ace options. This is done to try and make customizing the editor more robust from the directive alone, and possibly eliminate the need for controller code related to the editor.

ciel commented 10 years ago

This pull request has 3 major changes.

  1. the issue where settings are loaded twice is fixed.
  2. the ability to support the ace.require function is now implemented, and added to the docs.
  3. the ability to support other options not specifically declared is now available, and added to the docs.

This allows for more configuration, the example I give here is adding autocompletion to the directive without any more javascript code, like this;

<div ui-ace="{
  require: ['ace/ext/language_tools'],
  advanced: {
      enableSnippets: true,
      enableBasicAutocompletion: true,
      enableLiveAutocompletion: true
  }
}"></div>

I am sorry for the previous pull request and how messy it got. This is my first time doing a pull request, so I am still a bit confused. I hope this is more usable.

douglasduteil commented 10 years ago

Merged :) Congrats @ciel

ciel commented 10 years ago

Thank you so much for helping me through this. I cannot express to you how much I appreciate it. I learned a lot of new things just going through this process alone, and am in the process of formulating a second pull request based on something new I discovered. It's going to be a fix for defining the "worker path" if you are running from concatenated or minified source (which doesn't work, by default)

rebornix commented 9 years ago

@ciel seems after this pull request merged into master branch, CI build failed, it would be great you can have a look at this :+1:

PowerKiKi commented 9 years ago

That previous build was successful, but i just re-ran and it failed. That means Travis environment changed and make the build fails, not the code itself. Which is unfortunate because more annoying to debug...

rebornix commented 9 years ago

I reverted AngularJS from 1.3.x to 1.2.x, the number of failed unit tests comes down to 2, originally it's 18. Maybe it's a dependency issue, some breaking changes in AngularJS 1.3 leads to the test failure.

rebornix commented 9 years ago

@PowerKiKi Currently our code base can pass all tests only with angular-mock 1.3.0. It's dev Dependency and looks like a test framework issue instead of production bug. Yah it'd be better we change our code (whose dependency is still ^1.1.x) to make it pass with latest version but just like you said, it's annoying to debug...

I have added this change into my pull request, both build failure and ngModel synchronization can be fixed for now. Do you think this can be merged?