You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by azbarcea <gi...@git.apache.org> on 2015/08/24 00:48:30 UTC

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

GitHub user azbarcea opened a pull request:

    https://github.com/apache/incubator-brooklyn/pull/865

    [BROOKLYN-166] add codemirror, yaml syntax-highlighting, autocompletion)

    I was looking into the `codemirror` integration.
    
    I didn't integrate it in all the views, only in `application-add-wizard.js`, and I look forward to your feedback.
    
    I tried to improve the `scroll down` and now, there aren't 2 scrollers. There is only one for the editor.
    
    The following things are still open:
    * I excluded `rat-plugin` for the files added (actually, right now excludes the whole `src`).
    * I had to modify the original codemirror addons and modes to include `codemirror` instead of `../../lib/codemirror`
    * The way `angular.js` views and `require.js` is used (using the `config.js`), make the presumptions the `javascript` libraries are static in `libs`. Similar to `mvn`, I propose `grunt` and `npm` to be used, so that the whole `libs` directory to disappear.
    * I have no `yaml-hint.js`, or even `camp-hint.js`. I need to study further the CAMP spec for it. I think also that `brooklyn` has a little different spec for `.yaml` files, extending the language. I am right? Right now, I used the `anyword-hint.js`, which adds to the hints (auto-completion), the words that already exist in the document.
    * There are many themes that can be added, I will also add a configuration for: addons, modes, themes to be used globally. Maybe this should be at user profile level ?!?
    


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/azbarcea/incubator-brooklyn codemirror-yaml

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-brooklyn/pull/865.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #865
    
----
commit b0aff5d92826df8fbd2f673c8b8d728613ce4115
Author: Alexandru Zbarcea <al...@apache.org>
Date:   2015-08-23T22:27:45Z

    [BROOKLYN-166] add codemirror, yaml syntax-highlighting, autocompletion)

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by azbarcea <gi...@git.apache.org>.
Github user azbarcea commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#issuecomment-146174309
  
    I cannot move ```lib/codemirror.js```,``` addon/*``` to ```libs/codemirror``` folder, see explanation above ... 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#issuecomment-151840285
  
    @azbarcea what's the status of this?
    
    the latest changes address the licensing and rat issues, however still outstanding are:
    
    * check/fix `overflow-x: scroll` for other textareas
    * error in JS at rungime trying to load `/assets/css/complete.css`
    * 3rd party deps into libs/ like the others (if not too hard, else comment)
    * provide more info where you have added TODO comments
    
    plus new issue:
    
    * revert auto-scanning of classpath for REST API; it significantly increases startup time and typically we don't want it (you can trigger a scan by POSTing a suitable `catalog.bom` if you need to, or add those explicit items you wish to have); if you think it should be there, please give more details


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#issuecomment-134880370
  
    Tested, it works great. There are indeed two scroll but that should be easily fixable with a bit of CSS. Also, the editor is only one line height, could we extend to the height of the pop-up by default?
    
    Regarding the autocompletion, my view on that is first to pull out the catalogue entities when we are to a `type:` line. We can even imagine to have specific hints based on where we are, like having all the provisioning properties after a `provisioning.properties:` and so on.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#issuecomment-160105246
  
    the following issues are still outstanding:
    
    * check/fix `overflow-x: scroll` for textareas, likely others depend on this style you've removed -- https://github.com/apache/incubator-brooklyn/pull/865/files#diff-2c00b2e9ba84934a94b937199763e780L28
    * 3rd party deps into `libs/` like the others, not in `lib/` and `addons/` (or say why not)
    * revert auto-scanning of classpath for REST API; it significantly increases startup time and typically we don't want it (you can trigger a scan by POSTing a suitable catalog.bom if you need to, or add those explicit items you wish to have); if you think it should be there, please give more details -- https://github.com/apache/incubator-brooklyn/pull/865/files#diff-b8eba103a5475b653aaba9dd4fe8827cR55
    
    new ones:
    
    * UI does not run:  `:8081/assets/js/router.js:39 Uncaught TypeError: serverStatus.addCallback is not a function` (and we get the same error with JS jasmine tests: `[ERROR] java.lang.RuntimeException: org.openqa.selenium.WebDriverException: com.gargoylesoftware.htmlunit.ScriptException: Wrapped com.gargoylesoftware.htmlunit.ScriptException: Wrapped com.gargoylesoftware.htmlunit.ScriptException: TypeError: Cannot find function addCallback in object [object Object]. (http://localhost:57004/src/js/router.js#39)`)
    
    * license metadata needs to be added to `overrides.yaml` (the `LICENSE` files are autogenerated from this) - https://github.com/apache/incubator-brooklyn/pull/865/files#diff-001a0bdd771ea5cf50f47cd08ce6a0dcR356
    
    ability to test is limited by the above issues!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by CMoH <gi...@git.apache.org>.
Github user CMoH commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#issuecomment-135332375
  
    I think it's this page, right?
    
    https://brooklyn.incubator.apache.org/v/latest/yaml/yaml-reference.html


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by hzbarcea <gi...@git.apache.org>.
Github user hzbarcea commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#issuecomment-134951235
  
    @tbouron, regarding extending to the hight of the pop-up, we could integrate the tree view you proposed a while back. I don't know if that could be done without enlarging the pop-up which will impact the other tabs. Unless we make it a different pop-up or view. WDYT?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by azbarcea <gi...@git.apache.org>.
Github user azbarcea commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#issuecomment-157944052
  
    @ahgittin I'll rename it to composer! +1
    
    Regarding the Catalog, I think is a good proposal (+1), and I can have propose it as a different PR, based also on the community feedback. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#issuecomment-140460021
  
    ping @azbarcea 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#discussion_r43279276
  
    --- Diff: usage/jsgui/src/main/webapp/index.html ---
    @@ -26,6 +26,10 @@
     
         <title>Brooklyn JS REST client</title>
     
    +	<link rel="stylesheet" href="/assets/css/codemirror.css">
    +    <link rel="stylesheet" href="/assets/css/addon/hint/show-hint.css">
    +    <link rel="stylesheet" href="/assets/css/complete.css">
    --- End diff --
    
    + these files should not be referenced here. They should be included in [styles.css](https://github.com/apache/incubator-brooklyn/blob/master/usage/jsgui/src/main/webapp/assets/css/styles.css) so that they can be concatenated into one file at build time.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#issuecomment-135335504
  
    Yes @CMoH, this is the page that @hzbarcea was looking for.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#issuecomment-135043666
  
    A few comments:
    
    * If I have nothing in the catalog, it shows me YAML editor and it looks nice.  Surprising at first how lines are added but it's nice.  Cut and paste and other things work as expected.
    * If I then click on the "catalog" tab, with nothing there, i have option to write YAML blueprint.  I click that button and it returns me to the YAML tab, but a *new* editor is added.  This looks like a bug.
    * If I leave the wizard, go to the catalog and add some YAML (e.g. default.catalog.bom from CLI project), then open the add wizard, I have the catalog tab selected (good), but then when I go to the YAML editor I get the *old* YAML view.
    
    Of course we need RAT re-enabled.
    
    Code completion will require some REST API enhancements or better understanding of the model in the client.  I'd also like to see this as a separate main tab, rather than a wizard.  But we can leave these for a follow-on PR, as with the bug fixes before this paragraph resolved, this is a nice improvement.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#discussion_r41129553
  
    --- Diff: usage/jsgui/src/main/webapp/assets/js/view/application-add-wizard.js ---
    @@ -180,14 +183,13 @@ define([
                 }
                 
                 // finish from config step, preview step, and from first step if yaml tab selected (and valid)
    -            var finishVisible = (this.currentStep >= 1)
    -            var finishEnabled = finishVisible
    +            var finishVisible = (this.currentStep >= 1);
    +            var finishEnabled = finishVisible;
                 if (!finishEnabled && this.currentStep==0) {
                     if (this.model.mode == "yaml") {
                         // should do better validation than non-empty
                         finishVisible = true;
    -                    var yaml_code = this.$("#yaml_code").val()
    -                    if (yaml_code) {
    +                    if (self.editor && self.editor && self.editor.getValue()) {
    --- End diff --
    
    Second `self.editor` is not necessary.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#issuecomment-160100229
  
    merge conflicts with recent swagger changes but i've resolved those; now reviewing


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by azbarcea <gi...@git.apache.org>.
Github user azbarcea commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#discussion_r43457600
  
    --- Diff: usage/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncher.java ---
    @@ -314,6 +314,7 @@ public static void main(String[] args) throws Exception {
         public static Server startRestResourcesViaFilter() {
             return new BrooklynRestApiLauncher()
                     .mode(StartMode.FILTER)
    +                .forceUseOfDefaultCatalogWithJavaClassPath(true)
    --- End diff --
    
    Well this is the only way I know to see something in the catalog and test brooklyn.  `$ bin\brooklyn launch` uses `.forceUseOfDefaultCatalogWithJavaClassPath(true)`. Don't you want the test to be identical with the way is used? I think it slows down the build (the tests), but has no other consequence.
    
    Do you have any suggestions?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#issuecomment-160108239
  
    i've fixed the problem with startup.  now looks much better, and tests work.
    
    however:
    * i thought "Editor" was going to be called "Composer"
    * when using the pop-up wizard there should be a flow by which i can go to the composer populated with the YAML for what i've built (this is what the `preview` button typically does; maybe we should have the preview enabled even on the first step of the wizard if i have something selected)
    * on catalog page, add, if i click "yaml" to add a yaml catalog item, nothing happens -- i think it should take me to the blueprint editor, perhaps pre-populated with a sample item a la http://brooklyn.apache.org/v/latest/ops/catalog/index.html (NB catalog has a different syntax than normal deployments; it also accepts the normal deployment syntax), or see below
    * do we want a "Help" button on the composer, e.g. with a dropdown that has "insert template blueprint", "insert template catalog addition", "go to online documentation"
    * on the editor page what is "Delete" supposed to do?  currently seems to do nothing.  perhaps remove?  or comment out with TODO if you think it might do something in future.  (i'm not sure it does.)
    * on the editor page the "Run" button doesn't work; it constructs a POST but it is malformed
    * no feedback is given after clicking "Run"; on success it should take you to the "Applications" tab with the app highlighted (you have the app ID in the response code so that should be easy), and on failure it should show a message
    * there should be an "Add to Catalog" button on the editor/composer which has same feedback after clicking, taking you to catalog page (same as if you clicked the old catalog add yaml button)



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#issuecomment-135336551
  
    @hzbarcea To be honest, I'm not so sure anymore about the drag and drop editor. It is certainly a good feature for a sales point of view but I'm wondering about the usability. I think that having a good YAML editor with a good autocompletion feature over Brooklyn CAMP spec is a better approach for now.
    
    We could also imagine to have something a bit like swagger, i.e. having a live representation of the YAML as a tree or nodes. That would be great!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#issuecomment-138565653
  
    I think a drag-and-drop editor is very important for new users, especially if it generates the YAML which will help them move to become advanced users.
    



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#discussion_r41135461
  
    --- Diff: usage/jsgui/src/main/webapp/assets/js/config.js ---
    @@ -48,6 +50,15 @@ require.config({
             "zeroclipboard":"libs/ZeroClipboard",
             "js-yaml":"libs/js-yaml",
             
    +        "codemirror":"lib/codemirror",
    --- End diff --
    
    From: https://codemirror.net/doc/manual.html#modloader
    `Do not use RequireJS' paths option to configure the path...`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#discussion_r43249975
  
    --- Diff: usage/rest-server/src/test/java/org/apache/brooklyn/rest/BrooklynRestApiLauncher.java ---
    @@ -314,6 +314,7 @@ public static void main(String[] args) throws Exception {
         public static Server startRestResourcesViaFilter() {
             return new BrooklynRestApiLauncher()
                     .mode(StartMode.FILTER)
    +                .forceUseOfDefaultCatalogWithJavaClassPath(true)
    --- End diff --
    
    we don't want to do this by default, it slows things down.  can you revert?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by azbarcea <gi...@git.apache.org>.
Github user azbarcea commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#issuecomment-152356747
  
    I am addressing all the issues above ...


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by azbarcea <gi...@git.apache.org>.
Github user azbarcea commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#discussion_r46050141
  
    --- Diff: usage/jsgui/src/main/license/files/LICENSE ---
    @@ -353,6 +353,13 @@ This project includes the software: ZeroClipboard
       Used under the following license: The MIT License (http://opensource.org/licenses/MIT)
       Copyright (c) Jon Rohan, James M. Greene (2014)
     
    +This project includes the software: CodeMirror
    --- End diff --
    
    I didn't know that. I will address this issue too ...


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by azbarcea <gi...@git.apache.org>.
Github user azbarcea commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#issuecomment-160152180
  
    merged with @ahgittin `ui` branch ... addressing the above issues   ...


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by azbarcea <gi...@git.apache.org>.
Github user azbarcea commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#issuecomment-141291880
  
    @ahgittin, @CMoH - fixed CodeMirror integration.
    
    Please review. I look forward to your feedback.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#discussion_r41524871
  
    --- Diff: usage/jsgui/src/main/webapp/index.html ---
    @@ -26,6 +26,10 @@
     
         <title>Brooklyn JS REST client</title>
     
    +	<link rel="stylesheet" href="/assets/css/codemirror.css">
    +    <link rel="stylesheet" href="/assets/css/addon/hint/show-hint.css">
    +    <link rel="stylesheet" href="/assets/css/complete.css">
    --- End diff --
    
    `complete.css` is not included, gives 404's at runtime


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by azbarcea <gi...@git.apache.org>.
Github user azbarcea commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#discussion_r46050024
  
    --- Diff: usage/jsgui/src/main/webapp/index.html ---
    @@ -26,6 +26,11 @@
     
         <title>Brooklyn JS REST client</title>
     
    +    <!-- TODO: following CSS to config.js -->
    --- End diff --
    
    this line means that the CSS from index.html should be minimized as well ... I will address this issue!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by azbarcea <gi...@git.apache.org>.
Github user azbarcea commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#discussion_r41380610
  
    --- Diff: usage/jsgui/src/main/license/source-inclusions.yaml ---
    @@ -38,3 +38,5 @@
     - id: js-uri
     - id: js-yaml.js
     - id: jquery.form.js
    +
    +- id: codemirror.js
    --- End diff --
    
    yes! actually if this development would be with bower, those files would be part of the ```codemirror``` itself, and then selected and aggregated which addons and modes are required.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#discussion_r41135680
  
    --- Diff: usage/jsgui/src/main/webapp/assets/js/util/brooklyn-utils.js ---
    @@ -113,6 +113,7 @@ define([
             if ($input.attr("type") === "checkbox") {
                 return $input.is(":checked");
             } else {
    +            // TODO: get codemirror.getValue() in case this is a textarea ...
    --- End diff --
    
    Either delete or address.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by azbarcea <gi...@git.apache.org>.
Github user azbarcea commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#issuecomment-157710293
  
    Having issues with the current Modal Wizzard and adding the CodeMirror editor, I'm proposing having the Editor as a tab next to Home.
    
    ![apachebrooklyn-editor-blueprint](https://cloud.githubusercontent.com/assets/661912/11241952/df2b58ec-8dcc-11e5-9f5a-e17a90193fda.png)
    
    ![apachebrooklyn-editor-detail](https://cloud.githubusercontent.com/assets/661912/11241947/d9919b08-8dcc-11e5-8fea-1149d0959742.png)
    
    The proposal and the updates will continue on this PR.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#discussion_r39827822
  
    --- Diff: usage/jsgui/src/main/webapp/assets/js/view/application-add-wizard.js ---
    @@ -564,6 +579,7 @@ define([
             },
     
             validate:function () {
    +        	log ("validate");
    --- End diff --
    
    This line has a wrong indentation.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#issuecomment-157750064
  
    @azbarcea +1.  maybe call it "Composer" rather than "Editor", and I think don't show the catalog on this tab (we have the catalog tab).



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#discussion_r39827848
  
    --- Diff: usage/jsgui/src/main/webapp/assets/js/view/catalog.js ---
    @@ -134,54 +135,62 @@ define([
             initialize: function() {
                 _.bindAll(this);
             },
    +        editor: null,
             render: function (initialView) {
                 this.$el.html(this.template());
    -            if (initialView) {
    -                if (initialView == "entity") initialView = "yaml";
    -                
    -                this.$("[data-context='"+initialView+"']").addClass("active");
    -                this.showFormForType(initialView)
    -            }
                 return this;
             },
             clearWithHtml: function(template) {
                 if (this.contextView) this.contextView.close();
                 this.context = undefined;
    -            this.$(".btn").removeClass("active");
                 this.$("#catalog-add-form").html(template);
             },
             beforeClose: function () {
                 if (this.contextView) this.contextView.close();
             },
    +        setupCodeEditor: function() {
    +        	if (this.editor === null) {
    +            	this.editor = CodeMirror.fromTextArea(document.getElementById("new-blueprint"), {
    +            		height: "150px",
    +                	lineNumbers: true,
    +                    extraKeys: {"Ctrl-Space": "autocomplete"},
    +                    textWrapping: true,
    +                    mode: {name: "yaml", globalVars: true}
    +            	});
    +
    +                this.editor.setValue("# Please add your blueprint here\n");
    +        	}
    +        },
             showContext: function(event) {
    +        	log("showContext");
    --- End diff --
    
    This line has a wrong indentation.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by azbarcea <gi...@git.apache.org>.
Github user azbarcea commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#issuecomment-133954991
  
    Testing from `usage/dist/target/brooklyn-dist/brooklyn/bin/brooklyn launch` doesn't work.
    
    Tested with 
    
    ```shell
    org.apache.brooklyn.rest.jsgui.BrooklynJavascriptGuiLauncher -agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=5005
    ```
    
    I'll take a look at what's missing, tomorrow


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#discussion_r41135216
  
    --- Diff: usage/jsgui/src/main/license/source-inclusions.yaml ---
    @@ -38,3 +38,5 @@
     - id: js-uri
     - id: js-yaml.js
     - id: jquery.form.js
    +
    +- id: codemirror.js
    --- End diff --
    
    Should add addon & mode files as well?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by azbarcea <gi...@git.apache.org>.
Github user azbarcea commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#issuecomment-135128315
  
    @ahgittin - I will take a look to the bug. Also keep in mind that is not integrated in all the views, neither the _events_ aren't propagated.
    
    @tbouron, is there a documentation for the CAMP syntax/semantics (for hint) besides the [spec](http://docs.oasis-open.org/camp/camp-spec/v1.1/camp-spec-v1.1.html)?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#discussion_r39827764
  
    --- Diff: usage/jsgui/src/main/webapp/assets/js/view/application-add-wizard.js ---
    @@ -591,10 +607,14 @@ define([
                         return true
                     }
                 } else if (tabName=='#yamlTab') {
    -                this.model.yaml = this.$("#yaml_code").val();
    -                if (this.model.yaml) {
    -                    return true;
    -                }
    +            	if (self.editor && self.editor) {
    +            		this.model.yaml = self.editor.getValue();
    +	        		if (this.model.yaml) {
    +	                    return true;
    +	        		}
    +            	} else {
    +            		console.info("No text in the editor!");
    +            	}
    --- End diff --
    
    This block has a wrong indentation.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#discussion_r41135624
  
    --- Diff: usage/jsgui/src/main/webapp/assets/js/view/application-add-wizard.js ---
    @@ -304,6 +306,9 @@ define([
                     this.renderCurrentStep(function callback(view) {
                         // Drop any "None" locations.
                         that.model.spec.pruneLocations();
    +
    +                    // TODO: see if this preview has conflicts with codemirror. maybe the id="code"
    +                    //       has to be switched back to yaml_code
    --- End diff --
    
    Looks like this needs to be addressed before merging?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#issuecomment-153768987
  
    @azbarcea many of those issues don't look like they've been addressed.  what's the latest?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#issuecomment-160101572
  
    a lot of `log` statements in the JS -- probably okay but looks a little weird if someone opens the JS console.  maybe logging should be disabled by default (ie `log(...)` has no effect) but there would be an easy way to enable logging.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#issuecomment-135149286
  
    There is a Brooklyn camp yaml reference on the website. Most useful thin
    would be autocomplete for catalog item types and their configuration keys
    both of which are exposed through rest api.
    On 26 Aug 2015 19:15, "Alexandru Zbarcea" <no...@github.com> wrote:
    
    > @ahgittin <https://github.com/ahgittin> - I will take a look to the bug.
    > Also keep in mind that is not integrated in all the views, neither the
    > *events* aren't propagated.
    >
    > @tbouron <https://github.com/tbouron>, is there a documentation for the
    > CAMP syntax/semantics (for hint) besides the spec
    > <http://docs.oasis-open.org/camp/camp-spec/v1.1/camp-spec-v1.1.html>?
    >
    > —
    > Reply to this email directly or view it on GitHub
    > <https://github.com/apache/incubator-brooklyn/pull/865#issuecomment-135128315>
    > .
    >
    
    -- 
    Cloudsoft Corporation Limited, Registered in Scotland No: SC349230. 
     Registered Office: 13 Dryden Place, Edinburgh, EH9 1RP
     
    This e-mail message is confidential and for use by the addressee only. If 
    the message is received by anyone other than the addressee, please return 
    the message to the sender by replying to it and then delete the message 
    from your computer. Internet e-mails are not necessarily secure. Cloudsoft 
    Corporation Limited does not accept responsibility for changes made to this 
    message after it was sent.
    
    Whilst all reasonable care has been taken to avoid the transmission of 
    viruses, it is the responsibility of the recipient to ensure that the 
    onward transmission, opening or use of this message and any attachments 
    will not adversely affect its systems or data. No responsibility is 
    accepted by Cloudsoft Corporation Limited in this regard and the recipient 
    should carry out such virus and other checks as it considers appropriate.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by CMoH <gi...@git.apache.org>.
Github user CMoH commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#issuecomment-146180791
  
    `bin/brooklyn launch` works - perhaps there was some external reason it didn't on you machine, @neykov 
    



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#discussion_r39827767
  
    --- Diff: usage/jsgui/src/main/webapp/assets/js/util/brooklyn-utils.js ---
    @@ -113,6 +113,7 @@ define([
             if ($input.attr("type") === "checkbox") {
                 return $input.is(":checked");
             } else {
    +        	// TODO: get codemirror.getValue() in case this is a textarea ...
    --- End diff --
    
    This line has a wrong indentation.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by azbarcea <gi...@git.apache.org>.
Github user azbarcea commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#issuecomment-141570195
  
    I fixed the indentation ... any thoughts?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#discussion_r39827844
  
    --- Diff: usage/jsgui/src/main/webapp/assets/js/view/catalog.js ---
    @@ -134,54 +135,62 @@ define([
             initialize: function() {
                 _.bindAll(this);
             },
    +        editor: null,
             render: function (initialView) {
                 this.$el.html(this.template());
    -            if (initialView) {
    -                if (initialView == "entity") initialView = "yaml";
    -                
    -                this.$("[data-context='"+initialView+"']").addClass("active");
    -                this.showFormForType(initialView)
    -            }
                 return this;
             },
             clearWithHtml: function(template) {
                 if (this.contextView) this.contextView.close();
                 this.context = undefined;
    -            this.$(".btn").removeClass("active");
                 this.$("#catalog-add-form").html(template);
             },
             beforeClose: function () {
                 if (this.contextView) this.contextView.close();
             },
    +        setupCodeEditor: function() {
    +        	if (this.editor === null) {
    +            	this.editor = CodeMirror.fromTextArea(document.getElementById("new-blueprint"), {
    +            		height: "150px",
    +                	lineNumbers: true,
    --- End diff --
    
    The 2 lines above have a wrong indentation.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#issuecomment-160109093
  
    my fix for the `router.js` failure and merge conflicts is at https://github.com/ahgittin/incubator-brooklyn/tree/ui
    
    @azbarcea I suggest you merge it in here


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by azbarcea <gi...@git.apache.org>.
Github user azbarcea commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#discussion_r41380989
  
    --- Diff: usage/jsgui/src/main/webapp/assets/js/config.js ---
    @@ -48,6 +50,15 @@ require.config({
             "zeroclipboard":"libs/ZeroClipboard",
             "js-yaml":"libs/js-yaml",
             
    +        "codemirror":"lib/codemirror",
    --- End diff --
    
    this is true. This is the reason I put ```addon```, ```mode``` in the lib, and not in the codemirror folder. I tried modifying the ```config.js``` as explained in doc but it didn't work. 
    
    Let me know if it works for you ...


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#discussion_r46033290
  
    --- Diff: usage/jsgui/src/main/license/files/LICENSE ---
    @@ -353,6 +353,13 @@ This project includes the software: ZeroClipboard
       Used under the following license: The MIT License (http://opensource.org/licenses/MIT)
       Copyright (c) Jon Rohan, James M. Greene (2014)
     
    +This project includes the software: CodeMirror
    --- End diff --
    
    looks like you manually added this?  this file is autogenerated based on `overrides.yaml` which is where this data should be added.  (and it will update other `LICENSE` files as well because we are obligated to include info this not just in the UI license file but in the source and binary dists as well.)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#issuecomment-146573248
  
    also note that the catalog yaml syntax is different, as described at https://brooklyn.incubator.apache.org/v/latest/ops/catalog/index.html .  it allows you to paste a simple blueprint or several such.  not a blocker for this but important when completion is enabled; maybe for now a quick win would be to have a sample template with the right syntax, or comments showing the right syntax.
    
    i confirm `bin/brooklyn launch` works nicely.  and switching between tabs in the "add application" wizard causes no problems (but it isn't using the new codemirror any more.)
    
    newly found issues:
    
    * check/fix `overflow-x: scroll` for other textareas
    
    * add license info for codemirror in `overrides.yaml`
    
    * the JS console gives an error trying to load `/assets/css/complete.css`
    
    others still outstanding:
    
    * if possible, put 3rd party deps into `libs/` like the others, but if that's too hard leave as is
    
    * the rat check needs to scan all files or have an explanation.  if it's really really hard to have the codemirror files live under `libs/` then call them out explicitly here with a comment explaining why they have the unusual structure
    
    * update the two `TODO` comments, or expand on who/when they'll get done
    
    if we could add this to the wizard -- or completely rewrite the wizard as part of drag-and-drop -- then that would be good, but not needed in this PR.
    
    i think very soon we should start on a new UI based on angular.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#issuecomment-138565732
  
    @azbarcea can you address the issues blocking this being merged?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#issuecomment-141938774
  
    LGTM


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#discussion_r41522774
  
    --- Diff: usage/jsgui/src/main/webapp/assets/css/brooklyn.css ---
    @@ -25,7 +25,6 @@ BODY {
     textarea {
         white-space: pre;
         word-wrap: normal;
    -    overflow-x: scroll;
    --- End diff --
    
    i think we rely on textareas scrolling e.g. when we show a blueprint or other formatted code in a textarea.
    
    it perhaps would be better to have a special style for this but if we're removing this style have you made sure that no textareas suffer bad consequences?  if needed either update them or if you need a different `overflow-x` behaviour in the codemirror views define a new css rule.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#discussion_r41522483
  
    --- Diff: usage/jsgui/src/main/license/source-inclusions.yaml ---
    @@ -38,3 +38,5 @@
     - id: js-uri
     - id: js-yaml.js
     - id: jquery.form.js
    +
    +- id: codemirror.js
    --- End diff --
    
    no need to add the codemirror dependencies here, but we do need a corresponding entry in `usage/dist/licensing/overrides.yaml` and there under `files` list the files pulled in (as done for `jquery.dataTables`) -- it's only used to generate the proper license files


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#issuecomment-145507091
  
    Also move `lib/codemirror.js`, `addon/*`, `mode/*` to `libs/codemirror` folder.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#discussion_r46032809
  
    --- Diff: usage/jsgui/src/main/webapp/index.html ---
    @@ -26,6 +26,11 @@
     
         <title>Brooklyn JS REST client</title>
     
    +    <!-- TODO: following CSS to config.js -->
    --- End diff --
    
    what does this mean?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by azbarcea <gi...@git.apache.org>.
Github user azbarcea commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#issuecomment-141297761
  
    fixed merge ... with latest code base


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#issuecomment-145490194
  
    * `bin/brooklyn launch` doesn't work
    * RAT is still disabled (`**//src/*`)
    * Re-opening the dialog shows the old textarea


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#discussion_r46033729
  
    --- Diff: usage/jsgui/src/main/webapp/assets/js/router.js ---
    @@ -18,14 +18,15 @@
     */
     define([
         "brooklyn", "underscore", "jquery", "backbone",
    +    "codemirror",
    --- End diff --
    
    this is what is breaking the run -- we don't use it locally so we can remove it I think; but if it's added of course there needs to be a corresponding `CodeMirror` argument to the following `function`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-166] add codemirror, ya...

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/865#discussion_r39827135
  
    --- Diff: usage/jsgui/src/main/webapp/assets/css/brooklyn.css ---
    @@ -25,7 +25,7 @@ BODY {
     textarea {
         white-space: pre;
         word-wrap: normal;
    -    overflow-x: scroll;
    +/*    overflow-x: scroll; */
    --- End diff --
    
    This line can be removed


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---