You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by nakomis <gi...@git.apache.org> on 2015/07/17 17:05:12 UTC
[GitHub] incubator-brooklyn pull request: Unescapes sensor and config value...
GitHub user nakomis opened a pull request:
https://github.com/apache/incubator-brooklyn/pull/754
Unescapes sensor and config values before copying to clipboard
Previously `foo&bar` was being copied to the clipboard as `foo&bar`
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/nakomis/incubator-brooklyn fix/unescape-sensor-value
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-brooklyn/pull/754.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 #754
----
commit 53b11624fdfaff2172a4c1b8ff791e152bd5fb7d
Author: Martin Harris <gi...@nakomis.com>
Date: 2015-07-17T15:04:25Z
Unescapes sensor and config values before copying to clipboard
----
---
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: Unescapes sensor and config value...
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/754#discussion_r35437528
--- Diff: usage/jsgui/src/main/webapp/assets/js/util/brooklyn-utils.js ---
@@ -33,6 +33,11 @@ define([
return _.escape(s);
};
+ Util.unescape = function (s) {
+ if (s == undefined || s == null) return "";
--- End diff --
The preferred way is to use `typeof` and compare against the string `'undefined'`:
http://stackoverflow.com/questions/3390396/how-to-check-for-undefined-in-javascript
---
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: Unescapes sensor and config value...
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/754#discussion_r35296227
--- Diff: usage/jsgui/src/main/webapp/assets/js/view/entity-config.js ---
@@ -181,7 +181,7 @@ define([
if (!$(this).closest("#config-table").length) return;
var text = $(this).attr('copy-value');
if (!text) text = $(this).closest('.floatGroup').find('.value').html();
--- End diff --
Would be easier to use .text() instead of trying to unescape the html. Also `_.unescape` works for a limited html escapes.
---
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: Unescapes sensor and config value...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/754#issuecomment-124569657
+1 to @neykov 's suggestion to use `text()` which I think should remove the need for unescaping. Worth testing with a sensor whose value is something perverse like `I use Brooklyn & & <lol>` .
---
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: Unescapes sensor and config value...
Posted by hzbarcea <gi...@git.apache.org>.
Github user hzbarcea commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/754#issuecomment-122313906
This PR causes a build problem for me:
```
[INFO] Brooklyn Policies .................................. FAILURE [01:31 min]
```
Will look into it later.
---
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: Unescapes sensor and config value...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/754#issuecomment-125778427
Thanks @nakomis - did you manually test this with a perverse sensor value?
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: Unescapes sensor and config value...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-brooklyn/pull/754
---
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: Unescapes sensor and config value...
Posted by nakomis <gi...@git.apache.org>.
Github user nakomis commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/754#issuecomment-122314866
Hi @hzbarcea. This PR only changes a couple of .js files, so I'd be surprised if it caused an issue with the Brooklyn Policies project. Can you post the details of the failure?
---
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: Unescapes sensor and config value...
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/754#discussion_r35341958
--- Diff: usage/jsgui/src/main/webapp/assets/js/view/entity-config.js ---
@@ -181,7 +181,7 @@ define([
if (!$(this).closest("#config-table").length) return;
var text = $(this).attr('copy-value');
if (!text) text = $(this).closest('.floatGroup').find('.value').html();
--- End diff --
+1, it will make thing easier and shorter.
---
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: Unescapes sensor and config value...
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/754#issuecomment-124163108
@hzbarcea, @nakomis: Fetched on my local machine, all tests passed. :+1:
---
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: Unescapes sensor and config value...
Posted by hzbarcea <gi...@git.apache.org>.
Github user hzbarcea commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/754#issuecomment-122332828
@nakomis yes, I know, that's why it's surprising. Looks like the asfbot was slower than me, but reports success. The asfbot builds in a clean environment, I did an `mvn clean install`, but I fear the build picked up something from the local maven repo from a previous build. I'd have to look into it, but can only do that later today.
---
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: Unescapes sensor and config value...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/754#issuecomment-124329714
Thanks @nakomis - do you want to switch to the suggestion from @neykov and then we can merge?
---
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.
---