#7334 closed defect (fixed)
[Patch] Case Sensitive Regex
Reported by: | uray | Owned by: | Russ Tyndall |
---|---|---|---|
Priority: | normal | Component: | AddStaticResourcesPlugin |
Severity: | normal | Keywords: | |
Cc: | Trac Release: | 0.11 |
Description
because the value of key in config file (trac.ini) will all be turned to lower case by system, so if any capital letter in url-regex (as key) settings will lost its meaning, following is my quick fix (also fix the class name issue, BTW):
Index: 0.11/addstaticresourcesplugin/api.py =================================================================== --- 0.11/addstaticresourcesplugin/api.py (revision 8215) +++ 0.11/addstaticresourcesplugin/api.py (working copy) @@ -8,7 +8,7 @@ def urljoin(*args): return '/'.join([i.strip('/') for i in args]) -class AddStaticResourcesComponent (Component): +class AddStaticResourceComponent (Component): implements (IRequestFilter, ITemplateProvider) # IRequestHandler methods def pre_process_request(self, req, handler): @@ -24,10 +24,12 @@ self.log.debug("AddStaticResourceComponent: about to add resources, %s ,%s"%(req, handler)) c = self.env.config resources = c.options(srkey) - for regex, value in resources: + for key, value in resources: + map = value.split(' ') + regex = map[0] + paths = map[1:] self.log.debug("AddStaticResourceComponent: Regex:'%s' matched:%s" %(regex,re.search(regex, req.path_info))) if re.search(regex, req.path_info): - paths = c.getlist(srkey, regex) for p in paths: if p.endswith("js"): add_script(req,urljoin(srkey, p))
by this fix, url-regex is moved to value part, and key is ignored, so the config setting will like this:
[static_resources] map1 = /wiki/SomePage list_of_files.js and_style.css
Attachments (1)
Change History (8)
comment:1 follow-up: 5 Changed 14 years ago by
comment:2 Changed 14 years ago by
comment:3 Changed 14 years ago by
I found the other place where it was not pluralized (the name field in the setup) and pluralized it to be consistent in spelling across usages
comment:4 Changed 14 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
Summary: | does not work if url-regex contains capital letter → [Patch] Case Sensitive Regex |
I am going to leave this here as a patch in case anyone wants case sensitivity. Then I am going to close this as fixed. Feel free to update with compelling case sensitivity arguments.
Thanks again for your help,
Russ
comment:5 Changed 14 years ago by
Replying to bobbysmith007:
Thanks for the patch!
Do you think that just making the regex matches case insensitive by default would solve the problem (it definitely matches, is there a reason case sensitivity might be prefered)? While I can contrive situations where case sensitivity might be desired, I cant imagine it coming up frequently in actual deployments (perhaps I am wrong). I like the current config scheme because I feel it is visually straightforward and aesthetically pleases me. For now I have just committed a case insensitivity patch.
Also why was the change to the class name desirable? Every other version of the name in the plugin is AddStaticResources so I as curious why you thought the component name should be singularized.
Thanks again for your input,
Russ
Making the regex case insensitive will fix this problem, thanks. But what if somebody needs case sensitive match indeed? (assume I am a perfectionist, and want be prepared in every situation ;))
For the class name issue, because Trac was refused to load this plugin after install and shows me the following error:
ERROR: Skipping "addstaticresourcesplugin = addstaticresourcesplugin": (can't import "ImportError: cannot import name AddStaticResourceComponent")
I am not familier with Trac plugin, but found after changing the class name, plugin works, so this may be the cause of the issue (correct me if I am wrong, thanks.)
comment:6 Changed 14 years ago by
Interesting about the plugin not loading. I had not seen that before, and it was loading fine for me.
My trac admin screen has these spellings and it is enabled:
addstaticresourcesplugin.api.*
AddStaticResourcesComponent - [check]
I'm curious why it was looking for the misspelling. Oh well, if others report the same problem I will look into it further. As to case sensitivity, my feeling is that it will probably not be required. That said, feel free to continue running with this patch. I don't foresee any further features, so you patch will probably keep working for quite some time.
Thanks for the patch!
Do you think that just making the regex matches case insensitive by default would solve the problem (it definitely matches, is there a reason case sensitivity might be prefered)? While I can contrive situations where case sensitivity might be desired, I cant imagine it coming up frequently in actual deployments (perhaps I am wrong). I like the current config scheme because I feel it is visually straightforward and aesthetically pleases me. For now I have just committed a case insensitivity patch.
Also why was the change to the class name desirable? Every other version of the name in the plugin is AddStaticResources so I as curious why you thought the component name should be singularized.
Thanks again for your input,
Russ