[xwiki-devs] [proposal] Deprecate the practice of using $xcontext.blah to call $xcontext.getBlah() in the templates.
When I was looking for places where hasProgrammingRights is called, I found calls to c.x.x.api.Context.get() in the templates. I discovered that there is code in the templates which implicitly calls $xcontext.get('something') by calling $xcontext.something where there is no function Context.getSomething(). Since this is so easy and there is no way to know that anything is wrong since Context.get() returns null if you don't have PR, I am proposing that we do away with the practice of implicitly referring to functions in $context entirely. I used the following command to find implicit context references and I have filled in the mappings which I determined manually user@debo8:~/wrk/xwiki.clone/xwiki/platform/web/trunk/standard/src/main/webapp/templates$ find ./ -name '*.vm' -exec grep '$xcontext\.[a-zA-Z0-9]*[^a-zA-Z0-9\(]' {} \; | sed -n -e 's/\($xcontext\.[a-zA-Z0-9]*[^a-zA-Z0-9(]\)/\n\1\n/p' | grep '^$xcontext' | sort $xcontext.action= $xcontext.action= $xcontext.action= $xcontext.action $xcontext.action $xcontext.action $xcontext.action $xcontext.action $xcontext.action $xcontext.action $xcontext.action, $xcontext.action, $xcontext.action, $xcontext.action, $xcontext.action, $xcontext.action, $xcontext.action, $xcontext.action, $xcontext.action, $xcontext.action, $xcontext.action, $xcontext.action, $xcontext.action. $xcontext.action) ^-- maps to Context.getAction() $xcontext.database $xcontext.database) $xcontext.database) $xcontext.database) $xcontext.database) $xcontext.database) $xcontext.database) ^-- maps to Context.getDatabase() $xcontext.language" $xcontext.language" $xcontext.language" ^-- maps to Context.getLanguage() $xcontext.tocData ^-- this line is broken entirely, it maps to context.get('tocData') but the line is: #set ($xcontext.tocData = $tocData) which is simply invalid and I can't even figure out what it would do. $xcontext.user $xcontext.user $xcontext.user $xcontext.user $xcontext.user $xcontext.user $xcontext.user $xcontext.user $xcontext.user $xcontext.user $xcontext.user $xcontext.user, $xcontext.user, $xcontext.user, $xcontext.user, $xcontext.user, $xcontext.user, $xcontext.user, $xcontext.user, $xcontext.user, $xcontext.user, $xcontext.user, $xcontext.user, $xcontext.user, $xcontext.user. $xcontext.user. $xcontext.user. $xcontext.user" $xcontext.user" $xcontext.user) $xcontext.user) $xcontext.user) $xcontext.user) $xcontext.user) ^-- Maps to $xcontext.getUser() $xcontext.wiki, $xcontext.wiki, $xcontext.wiki, ^-- Maps to $xcontext.get('wiki') I think this is the source of most of the messages and since it appears to be relatively new code, it convinces me that the $context.blah pattern is simply unsafe and should be abandoned at least for the xcontext object. I would like to fix these now and establish a best practice of avoiding the use of these in the future for $xcontext. I myself avoid this pattern entirely but I do not have a compelling reason to propose it as a best practice except for $context. WDYT? Caleb
Hi Caleb, On 02/27/2011 02:00 PM, Caleb James DeLisle wrote:
When I was looking for places where hasProgrammingRights is called, I found calls to c.x.x.api.Context.get() in the templates. I discovered that there is code in the templates which implicitly calls $xcontext.get('something') by calling $xcontext.something where there is no function Context.getSomething().
Since this is so easy and there is no way to know that anything is wrong since Context.get() returns null if you don't have PR, I am proposing that we do away with the practice of implicitly referring to functions in $context entirely.
I don't understand what is the problem. Can you be more explicit? Thanks, Marius
I used the following command to find implicit context references and I have filled in the mappings which I determined manually
user@debo8:~/wrk/xwiki.clone/xwiki/platform/web/trunk/standard/src/main/webapp/templates$ find ./ -name '*.vm' -exec grep '$xcontext\.[a-zA-Z0-9]*[^a-zA-Z0-9\(]' {} \; | sed -n -e 's/\($xcontext\.[a-zA-Z0-9]*[^a-zA-Z0-9(]\)/\n\1\n/p' | grep '^$xcontext' | sort $xcontext.action= $xcontext.action= $xcontext.action= $xcontext.action $xcontext.action $xcontext.action $xcontext.action $xcontext.action $xcontext.action $xcontext.action $xcontext.action, $xcontext.action, $xcontext.action, $xcontext.action, $xcontext.action, $xcontext.action, $xcontext.action, $xcontext.action, $xcontext.action, $xcontext.action, $xcontext.action, $xcontext.action, $xcontext.action. $xcontext.action) ^-- maps to Context.getAction()
$xcontext.database $xcontext.database) $xcontext.database) $xcontext.database) $xcontext.database) $xcontext.database) $xcontext.database) ^-- maps to Context.getDatabase()
$xcontext.language" $xcontext.language" $xcontext.language" ^-- maps to Context.getLanguage()
$xcontext.tocData ^-- this line is broken entirely, it maps to context.get('tocData') but the line is: #set ($xcontext.tocData = $tocData) which is simply invalid and I can't even figure out what it would do.
$xcontext.user $xcontext.user $xcontext.user $xcontext.user $xcontext.user $xcontext.user $xcontext.user $xcontext.user $xcontext.user $xcontext.user $xcontext.user $xcontext.user, $xcontext.user, $xcontext.user, $xcontext.user, $xcontext.user, $xcontext.user, $xcontext.user, $xcontext.user, $xcontext.user, $xcontext.user, $xcontext.user, $xcontext.user, $xcontext.user, $xcontext.user. $xcontext.user. $xcontext.user. $xcontext.user" $xcontext.user" $xcontext.user) $xcontext.user) $xcontext.user) $xcontext.user) $xcontext.user) ^-- Maps to $xcontext.getUser()
$xcontext.wiki, $xcontext.wiki, $xcontext.wiki, ^-- Maps to $xcontext.get('wiki') I think this is the source of most of the messages and since it appears to be relatively new code, it convinces me that the $context.blah pattern is simply unsafe and should be abandoned at least for the xcontext object.
I would like to fix these now and establish a best practice of avoiding the use of these in the future for $xcontext. I myself avoid this pattern entirely but I do not have a compelling reason to propose it as a best practice except for $context.
WDYT?
Caleb
_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
On 02/27/2011 03:31 PM, Marius Dumitru Florea wrote:
Hi Caleb,
On 02/27/2011 02:00 PM, Caleb James DeLisle wrote:
When I was looking for places where hasProgrammingRights is called, I found calls to c.x.x.api.Context.get() in the templates. I discovered that there is code in the templates which implicitly calls $xcontext.get('something') by calling $xcontext.something where there is no function Context.getSomething().
Since this is so easy and there is no way to know that anything is wrong since Context.get() returns null if you don't have PR, I am proposing that we do away with the practice of implicitly referring to functions in $context entirely.
I don't understand what is the problem. Can you be more explicit?
If someone makes a mistake and thinks there is a $xcontext.getWiki() for instance and they say $xcontext.wiki in a template, velocity will assume they meant $xcontext.get('wiki') since $xcontext.getWiki() does not exist. $xcontext.get() checks for programming right and returns null if there is none meaning that the mistake will be hidden. If once in a while that code is executed with PR and there happens to be an entry called wiki in the context hashmap, that entry will be returned which will probably cause the template calling $xcontext.wiki to throw an error and we will have a "once in a while bug". Caleb
Thanks, Marius
I used the following command to find implicit context references and I have filled in the mappings which I determined manually
user@debo8:~/wrk/xwiki.clone/xwiki/platform/web/trunk/standard/src/main/webapp/templates$ find ./ -name '*.vm' -exec grep '$xcontext\.[a-zA-Z0-9]*[^a-zA-Z0-9\(]' {} \; | sed -n -e 's/\($xcontext\.[a-zA-Z0-9]*[^a-zA-Z0-9(]\)/\n\1\n/p' | grep '^$xcontext' | sort $xcontext.action= $xcontext.action= $xcontext.action= $xcontext.action $xcontext.action $xcontext.action $xcontext.action $xcontext.action $xcontext.action $xcontext.action $xcontext.action, $xcontext.action, $xcontext.action, $xcontext.action, $xcontext.action, $xcontext.action, $xcontext.action, $xcontext.action, $xcontext.action, $xcontext.action, $xcontext.action, $xcontext.action, $xcontext.action. $xcontext.action) ^-- maps to Context.getAction()
$xcontext.database $xcontext.database) $xcontext.database) $xcontext.database) $xcontext.database) $xcontext.database) $xcontext.database) ^-- maps to Context.getDatabase()
$xcontext.language" $xcontext.language" $xcontext.language" ^-- maps to Context.getLanguage()
$xcontext.tocData ^-- this line is broken entirely, it maps to context.get('tocData') but the line is: #set ($xcontext.tocData = $tocData) which is simply invalid and I can't even figure out what it would do.
$xcontext.user $xcontext.user $xcontext.user $xcontext.user $xcontext.user $xcontext.user $xcontext.user $xcontext.user $xcontext.user $xcontext.user $xcontext.user $xcontext.user, $xcontext.user, $xcontext.user, $xcontext.user, $xcontext.user, $xcontext.user, $xcontext.user, $xcontext.user, $xcontext.user, $xcontext.user, $xcontext.user, $xcontext.user, $xcontext.user, $xcontext.user. $xcontext.user. $xcontext.user. $xcontext.user" $xcontext.user" $xcontext.user) $xcontext.user) $xcontext.user) $xcontext.user) $xcontext.user) ^-- Maps to $xcontext.getUser()
$xcontext.wiki, $xcontext.wiki, $xcontext.wiki, ^-- Maps to $xcontext.get('wiki') I think this is the source of most of the messages and since it appears to be relatively new code, it convinces me that the $context.blah pattern is simply unsafe and should be abandoned at least for the xcontext object.
I would like to fix these now and establish a best practice of avoiding the use of these in the future for $xcontext. I myself avoid this pattern entirely but I do not have a compelling reason to propose it as a best practice except for $context.
WDYT?
Caleb
_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
_______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
On 02/27/2011 01:00 PM, Caleb James DeLisle wrote:
When I was looking for places where hasProgrammingRights is called, I found calls to c.x.x.api.Context.get() in the templates. I discovered that there is code in the templates which implicitly calls $xcontext.get('something') by calling $xcontext.something where there is no function Context.getSomething(). Since this is so easy and there is no way to know that anything is wrong since Context.get() returns null if you don't have PR, I am proposing that we do away with the practice of implicitly referring to functions in $context entirely.
I don't quite agree, it should be allowed.
$xcontext.tocData ^-- this line is broken entirely, it maps to context.get('tocData') but the line is: #set ($xcontext.tocData = $tocData) which is simply invalid and I can't even figure out what it would do.
No, it's not invalid. In this case the line is equivalent to $xcontext.put("tocData", $tocData). Similar to the way $something.prop is resolved at runtime as one of getProp(), getprop(), get('prop'), isProp(), #set($something.prop = $value) also searches for setProp(), setprop(), and finally put("prop").
$xcontext.wiki, $xcontext.wiki, $xcontext.wiki, ^-- Maps to $xcontext.get('wiki') I think this is the source of most of the messages and since it appears to be relatively new code, it convinces me that the $context.blah pattern is simply unsafe and should be abandoned at least for the xcontext object.
I think this is wrong, it should be $xcontext.database (which itself is a bad name and should be deprecated in favor of a real xcontext.getWiki() which returns the current wiki name.
I would like to fix these now and establish a best practice of avoiding the use of these in the future for $xcontext. I myself avoid this pattern entirely but I do not have a compelling reason to propose it as a best practice except for $context.
It's a bit faster, since velocity tries the methods in a specific order until it can find one that exists and matches the current parameters. But it's faster and easier to read if the shorthand is used. -- Sergiu Dumitriu http://purl.org/net/sergiu/
participants (3)
-
Caleb James DeLisle -
Marius Dumitru Florea -
Sergiu Dumitriu