You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by sjcorbett <gi...@git.apache.org> on 2014/09/26 19:51:56 UTC

[GitHub] incubator-brooklyn pull request: jsgui: Replace _.template with cu...

GitHub user sjcorbett opened a pull request:

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

    jsgui: Replace _.template with custom function that strips comments

    Makes the source much more readable. Before:
    ![screen shot 2014-09-26 at 18 48 39](https://cloud.githubusercontent.com/assets/837527/4425034/67da2018-45a5-11e4-8041-43486b1ea6b3.png)
    After:
    ![screen shot 2014-09-26 at 18 45 43](https://cloud.githubusercontent.com/assets/837527/4425006/07877b70-45a5-11e4-9aca-54c0c1340964.png)
    
    The rendered page itself still contains the Apache license header at the top.
    
    Do we want to replace `_.template` or add an alternate function for use with templates likely to contain a license header? I figured the former saves having to replace lots of function calls and will automatically include all templates added in the future.

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

    $ git pull https://github.com/sjcorbett/incubator-brooklyn jsgui-html-apache-license

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

    https://github.com/apache/incubator-brooklyn/pull/192.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 #192
    
----
commit ae9ed76b98cb91ee1b4c7902c43428a6063fbcc1
Author: Sam Corbett <sa...@cloudsoftcorp.com>
Date:   2014-09-26T17:44:21Z

    jsgui: Replace _.template with custom function that strips comments

----


---
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: jsgui: Replace _.template with cu...

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

    https://github.com/apache/incubator-brooklyn/pull/192#issuecomment-57008604
  
    +1 for patching the existing template method.


---
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: jsgui: Replace _.template with cu...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

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


---
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: jsgui: Replace _.template with cu...

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/192#discussion_r18108388
  
    --- Diff: usage/jsgui/src/main/webapp/assets/js/util/brooklyn.js ---
    @@ -60,6 +60,23 @@ define([
             }
         })();
     
    +    var template = _.template;
    +    _.mixin({
    +        /**
    +         * @param {string} text
    +         * @return string The text with HTML comments removed.
    +         */
    +        stripComments: function (text) {
    +            return text.replace(/<!--(.|[\n\r\t])*?-->[\n\r]?/g, "");
    --- End diff --
    
    Doesn't catch windows new line after the comment. This one should work:
    /<!--(.|[\n\r\t])*?-->\r?\n?/g



---
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.
---