Re: [xwiki-devs] Bug in getusers.vm?
Hi Caleb,
On 25 May 2015 at 02:54:44, Caleb James DeLisle
(c...@cjdns.fr(mailto:c...@cjdns.fr)) wrote:
That's not bad velocity, that's bad javascript.
ah indeed… Not easy to read :)
Ideally you never
mix velocity and js but the best way I've found when it must be done
is to isolate all of the generated js at the top of the script, eg:
// BEGIN VELOCITY
var EXTRA_ANCHOR_LINK = ${extraAnchor}link;
var TM_SHOW_EXTRA_ANCHOR = tmShow${extraAnchor}
// END VELOCITY
...
later
...
if ($(EXTRA_ANCHOR_LINK) != null) { ...
Note #1: I've still not found a solution to stopping the velocity
parser from evaluating below a certain point.
#stop ?
See http://velocity.apache.org/engine/releases/velocity-1.7/user-guide.html#Stop
Note #2: There is also the issue of using != instead of !== which
increases bugs in js (but don't change it now because it might be
relying on the implicit casting).
Thanks
-Vincent
Thanks,
Caleb
On 05/23/2015 06:02 PM, vinc...@massol.net wrote:
BTW I see several places where we have:
if ($(${extraAnchor}link) != null) {
if ($(tmShow${extraAnchor}) != null) {
…
This doesn’t seem right either…
According to http://wiki.apache.org/velocity/CheckingForNull this is not
how to check for null.
WDYT?
Thanks
-Vincent
On 23 May 2015 at 17:58:51, vinc...@massol.net (vinc...@massol.net) wrote:
Hi Caleb,
On 23 May 2015 at 16:28:17, Caleb James DeLisle
(c...@cjdns.fr(mailto:c...@cjdns.fr)) wrote:
Agreed, I can't see what that line should do, if anything.
The best it can do is add a null element to the list, kicking the value to
index 1.
“null” doesn’t exist in velocity AFAIK so the statement is even invalid IMO
(see below).
As for the thinking of the author, the comment implies he didn't clarify
his thinking
about what was being done and why so I'd favor simply dropping the line
and doing a
few basic (manual) does it still work tests to be sure.
No it’s more complex than that, I think the author meant:
#set( $discard = $arr.add( $NULL ) ) ## this may be variable...
If you check the vm the code is:
#set( $discard = $arr.add( null ) ) ## this may be variable...
#set( $discard = $arr.add( $value ) )
#set( $discard = $filterMap.put($key, $arr))
Thus it builds a 2 element list which is then put in a map. This map is
passed to getAllMatchedLocalUsers() (for ex), which says in its javadoc:
* @param matchFields the fields to match. It is a Map with field name as
key and for value :
*
*
matching string for document fields
*
or [field type, matching string] for object fields
*
The javadoc doesn’t mention the support for null as the key. However
following the source code it seems to lead to
getAllMatchedLocalUsersOrGroups() which has a more useful javadoc:
fieldtype : for example StringProperty. If null the field is considered as
document field
Thus it seems the author really wanted to pass null and that would be $NULL.
Ok I’ve checked in a page the result of:
{{velocity}}
#set ($arr = [])
#set( $discard = $arr.add( null ) ) ## this may be variable...
#set( $discard = $arr.add( value ) )
$arr
{{/velocity}}
and strangely enough it gives [null, value] :)
So even if invalid it still puts null.
Anyway fixing by using $NULL
Thanks
-Vincent
Thanks,
Caleb
On 05/23/2015 10:56 AM, vinc...@massol.net wrote:
Hi devs,
I’ve noticed the following in getusers.vm:
#set ($arr = [])
#set( $discard = $arr.add( null ) ) ## this may be variable...
#set( $discard = $arr.add( $value ) )
The “null” part doesn’t seem correct at all and I don’t understand the
comment ## this may be variable…”
Any idea what the original author wanted to do?
Thanks
-Vincent
___
devs mailing list
devs@xwiki.org
http://lists.xwiki.org/mailman/listinfo/devs
Re: [xwiki-devs] Bug in getusers.vm?
Note #1: I've still not found a solution to stopping the velocity parser from evaluating below a certain point. #stop ? See http://velocity.apache.org/engine/releases/velocity-1.7/user-guide.html#Stop That stops the rendering as well, and we want to get all the JS code that comes afterwards. Maybe #[[ ... ]]# is what we want? http://velocity.apache.org/engine/devel/vtl-reference-guide.html#Unparsed_Content It depends on not having ]]# present in the source, but I guess that's fine since we would explicitly use this when needed. -- Sergiu Dumitriu http://purl.org/net/sergiu ___ devs mailing list devs@xwiki.org http://lists.xwiki.org/mailman/listinfo/devs
Re: [xwiki-devs] Bug in getusers.vm?
That's not bad velocity, that's bad javascript. Ideally you never
mix velocity and js but the best way I've found when it must be done
is to isolate all of the generated js at the top of the script, eg:
// BEGIN VELOCITY
var EXTRA_ANCHOR_LINK = ${extraAnchor}link;
var TM_SHOW_EXTRA_ANCHOR = tmShow${extraAnchor}
// END VELOCITY
...
later
...
if ($(EXTRA_ANCHOR_LINK) != null) { ...
Note #1: I've still not found a solution to stopping the velocity
parser from evaluating below a certain point.
Note #2: There is also the issue of using != instead of !== which
increases bugs in js (but don't change it now because it might be
relying on the implicit casting).
Thanks,
Caleb
On 05/23/2015 06:02 PM, vinc...@massol.net wrote:
BTW I see several places where we have:
if ($(${extraAnchor}link) != null) {
if ($(tmShow${extraAnchor}) != null) {
…
This doesn’t seem right either…
According to http://wiki.apache.org/velocity/CheckingForNull this is not how
to check for null.
WDYT?
Thanks
-Vincent
On 23 May 2015 at 17:58:51, vinc...@massol.net (vinc...@massol.net) wrote:
Hi Caleb,
On 23 May 2015 at 16:28:17, Caleb James DeLisle
(c...@cjdns.fr(mailto:c...@cjdns.fr)) wrote:
Agreed, I can't see what that line should do, if anything.
The best it can do is add a null element to the list, kicking the value to
index 1.
“null” doesn’t exist in velocity AFAIK so the statement is even invalid IMO
(see below).
As for the thinking of the author, the comment implies he didn't clarify his
thinking
about what was being done and why so I'd favor simply dropping the line and
doing a
few basic (manual) does it still work tests to be sure.
No it’s more complex than that, I think the author meant:
#set( $discard = $arr.add( $NULL ) ) ## this may be variable...
If you check the vm the code is:
#set( $discard = $arr.add( null ) ) ## this may be variable...
#set( $discard = $arr.add( $value ) )
#set( $discard = $filterMap.put($key, $arr))
Thus it builds a 2 element list which is then put in a map. This map is
passed to getAllMatchedLocalUsers() (for ex), which says in its javadoc:
* @param matchFields the fields to match. It is a Map with field name as
key and for value :
*ul
*limatching string for document fields/li
*lior [field type, matching string] for object
fields/li
*/ul
The javadoc doesn’t mention the support for null as the key. However
following the source code it seems to lead to
getAllMatchedLocalUsersOrGroups() which has a more useful javadoc:
lifieldtype : for example StringProperty. If null the field is considered
as document field/li
Thus it seems the author really wanted to pass null and that would be $NULL.
Ok I’ve checked in a page the result of:
{{velocity}}
#set ($arr = [])
#set( $discard = $arr.add( null ) ) ## this may be variable...
#set( $discard = $arr.add( value ) )
$arr
{{/velocity}}
and strangely enough it gives [null, value] :)
So even if invalid it still puts null.
Anyway fixing by using $NULL
Thanks
-Vincent
Thanks,
Caleb
On 05/23/2015 10:56 AM, vinc...@massol.net wrote:
Hi devs,
I’ve noticed the following in getusers.vm:
#set ($arr = [])
#set( $discard = $arr.add( null ) ) ## this may be variable...
#set( $discard = $arr.add( $value ) )
The “null” part doesn’t seem correct at all and I don’t understand the
comment ## this may be variable…”
Any idea what the original author wanted to do?
Thanks
-Vincent
___
devs mailing list
devs@xwiki.org
http://lists.xwiki.org/mailman/listinfo/devs
___
devs mailing list
devs@xwiki.org
http://lists.xwiki.org/mailman/listinfo/devs
--
Satire is the escape hatch from the cycle of sorrow, hatred and violence.
#JeSuisCharlie
___
devs mailing list
devs@xwiki.org
http://lists.xwiki.org/mailman/listinfo/devs
[xwiki-devs] Bug in getusers.vm?
Hi devs, I’ve noticed the following in getusers.vm: #set ($arr = []) #set( $discard = $arr.add( null ) ) ## this may be variable... #set( $discard = $arr.add( $value ) ) The “null” part doesn’t seem correct at all and I don’t understand the comment ## this may be variable…” Any idea what the original author wanted to do? Thanks -Vincent ___ devs mailing list devs@xwiki.org http://lists.xwiki.org/mailman/listinfo/devs
Re: [xwiki-devs] Bug in getusers.vm?
Agreed, I can't see what that line should do, if anything. The best it can do is add a null element to the list, kicking the value to index 1. As for the thinking of the author, the comment implies he didn't clarify his thinking about what was being done and why so I'd favor simply dropping the line and doing a few basic (manual) does it still work tests to be sure. Thanks, Caleb On 05/23/2015 10:56 AM, vinc...@massol.net wrote: Hi devs, I’ve noticed the following in getusers.vm: #set ($arr = []) #set( $discard = $arr.add( null ) ) ## this may be variable... #set( $discard = $arr.add( $value ) ) The “null” part doesn’t seem correct at all and I don’t understand the comment ## this may be variable…” Any idea what the original author wanted to do? Thanks -Vincent ___ devs mailing list devs@xwiki.org http://lists.xwiki.org/mailman/listinfo/devs -- Satire is the escape hatch from the cycle of sorrow, hatred and violence. #JeSuisCharlie ___ devs mailing list devs@xwiki.org http://lists.xwiki.org/mailman/listinfo/devs
Re: [xwiki-devs] Bug in getusers.vm?
BTW I see several places where we have:
if ($(${extraAnchor}link) != null) {
if ($(tmShow${extraAnchor}) != null) {
…
This doesn’t seem right either…
According to http://wiki.apache.org/velocity/CheckingForNull this is not how to
check for null.
WDYT?
Thanks
-Vincent
On 23 May 2015 at 17:58:51, vinc...@massol.net (vinc...@massol.net) wrote:
Hi Caleb,
On 23 May 2015 at 16:28:17, Caleb James DeLisle
(c...@cjdns.fr(mailto:c...@cjdns.fr)) wrote:
Agreed, I can't see what that line should do, if anything.
The best it can do is add a null element to the list, kicking the value to
index 1.
“null” doesn’t exist in velocity AFAIK so the statement is even invalid IMO
(see below).
As for the thinking of the author, the comment implies he didn't clarify his
thinking
about what was being done and why so I'd favor simply dropping the line and
doing a
few basic (manual) does it still work tests to be sure.
No it’s more complex than that, I think the author meant:
#set( $discard = $arr.add( $NULL ) ) ## this may be variable...
If you check the vm the code is:
#set( $discard = $arr.add( null ) ) ## this may be variable...
#set( $discard = $arr.add( $value ) )
#set( $discard = $filterMap.put($key, $arr))
Thus it builds a 2 element list which is then put in a map. This map is passed
to getAllMatchedLocalUsers() (for ex), which says in its javadoc:
* @param matchFields the fields to match. It is a Map with field name as
key and for value :
* ul
* limatching string for document fields/li
* lior [field type, matching string] for object
fields/li
* /ul
The javadoc doesn’t mention the support for null as the key. However following
the source code it seems to lead to getAllMatchedLocalUsersOrGroups() which has
a more useful javadoc:
lifieldtype : for example StringProperty. If null the field is considered as
document field/li
Thus it seems the author really wanted to pass null and that would be $NULL.
Ok I’ve checked in a page the result of:
{{velocity}}
#set ($arr = [])
#set( $discard = $arr.add( null ) ) ## this may be variable...
#set( $discard = $arr.add( value ) )
$arr
{{/velocity}}
and strangely enough it gives [null, value] :)
So even if invalid it still puts null.
Anyway fixing by using $NULL
Thanks
-Vincent
Thanks,
Caleb
On 05/23/2015 10:56 AM, vinc...@massol.net wrote:
Hi devs,
I’ve noticed the following in getusers.vm:
#set ($arr = [])
#set( $discard = $arr.add( null ) ) ## this may be variable...
#set( $discard = $arr.add( $value ) )
The “null” part doesn’t seem correct at all and I don’t understand the
comment ## this may be variable…”
Any idea what the original author wanted to do?
Thanks
-Vincent
___
devs mailing list
devs@xwiki.org
http://lists.xwiki.org/mailman/listinfo/devs
___
devs mailing list
devs@xwiki.org
http://lists.xwiki.org/mailman/listinfo/devs
Re: [xwiki-devs] Bug in getusers.vm?
Hi Caleb,
On 23 May 2015 at 16:28:17, Caleb James DeLisle
(c...@cjdns.fr(mailto:c...@cjdns.fr)) wrote:
Agreed, I can't see what that line should do, if anything.
The best it can do is add a null element to the list, kicking the value to
index 1.
“null” doesn’t exist in velocity AFAIK so the statement is even invalid IMO
(see below).
As for the thinking of the author, the comment implies he didn't clarify his
thinking
about what was being done and why so I'd favor simply dropping the line and
doing a
few basic (manual) does it still work tests to be sure.
No it’s more complex than that, I think the author meant:
#set( $discard = $arr.add( $NULL ) ) ## this may be variable...
If you check the vm the code is:
#set( $discard = $arr.add( null ) ) ## this may be variable...
#set( $discard = $arr.add( $value ) )
#set( $discard = $filterMap.put($key, $arr))
Thus it builds a 2 element list which is then put in a map. This map is passed
to getAllMatchedLocalUsers() (for ex), which says in its javadoc:
* @param matchFields the fields to match. It is a Map with field name as
key and for value :
* ul
* limatching string for document fields/li
* lior [field type, matching string] for object
fields/li
* /ul
The javadoc doesn’t mention the support for null as the key. However following
the source code it seems to lead to getAllMatchedLocalUsersOrGroups() which has
a more useful javadoc:
lifieldtype : for example StringProperty. If null the field is considered as
document field/li
Thus it seems the author really wanted to pass null and that would be $NULL.
Ok I’ve checked in a page the result of:
{{velocity}}
#set ($arr = [])
#set( $discard = $arr.add( null ) ) ## this may be variable...
#set( $discard = $arr.add( value ) )
$arr
{{/velocity}}
and strangely enough it gives [null, value] :)
So even if invalid it still puts null.
Anyway fixing by using $NULL
Thanks
-Vincent
Thanks,
Caleb
On 05/23/2015 10:56 AM, vinc...@massol.net wrote:
Hi devs,
I’ve noticed the following in getusers.vm:
#set ($arr = [])
#set( $discard = $arr.add( null ) ) ## this may be variable...
#set( $discard = $arr.add( $value ) )
The “null” part doesn’t seem correct at all and I don’t understand the
comment ## this may be variable…”
Any idea what the original author wanted to do?
Thanks
-Vincent
___
devs mailing list
devs@xwiki.org
http://lists.xwiki.org/mailman/listinfo/devs
