#10226 closed defect (fixed)
Uses wrongly `re.match` for `[bookmark] paths` settings
| Reported by: | jun66j5 | Owned by: | saigon |
|---|---|---|---|
| Priority: | normal | Component: | BookmarkPlugin |
| Severity: | normal | Keywords: | |
| Cc: | jun66j5, rjollos, hasienda | Trac Release: | 0.12 |
Description
It seems [bookmark] paths supports shell wildcards, not regular expression.
bookmarkable_paths = ListOption('bookmark', 'paths', '/*', doc='List of URL paths to allow bookmarking on. Globs are supported.')
But the plugin uses re.match for each value with no changes.
def post_process_request(self, req, template, data, content_type): # Show bookmarks context menu except when on the bookmark page if 'BOOKMARK_VIEW' in req.perm and not self.match_request(req): for path in self.bookmarkable_paths: if re.match(path, req.path_info): self.render_bookmarker(req) break return template, data, content_type
I think it should fix to be able to use shell wildcards.
Attachments (0)
Change History (7)
comment:1 Changed 8 months ago by rjollos
comment:2 Changed 3 months ago by rjollos
- Cc hasienda added
Same issue was noted with the VotePlugin in comment:5:ticket:10942.
comment:3 Changed 8 weeks ago by rjollos
It looks like fnmatch will accomplish what we want. How does the following patch look?
-
tracbookmark/__init__.py
9 9 # you should have received as part of this distribution. 10 10 11 11 import re 12 from fnmatch import fnmatch 12 13 13 14 from genshi.builder import tag 14 15 from trac.config import ListOption … … 168 169 # Show bookmarks context menu except when on the bookmark page 169 170 if self._authorize(req) and not self.match_request(req): 170 171 for path in self.bookmarkable_paths: 171 if re.match(path, req.path_info):172 if fnmatch(req.path_info, path): 172 173 self.render_bookmarker(req) 173 174 break 174 175 return template, data, content_type
comment:4 follow-up: ↓ 5 Changed 8 weeks ago by rjollos
On second look, we might want to use fnmatchcase.
comment:5 in reply to: ↑ 4 Changed 8 weeks ago by jun66j5
Replying to rjollos:
On second look, we might want to use fnmatchcase.
Looks good! Yes, we should use fnmatchcase because the URLs in Trac is case-sensitive. Please commit it!
comment:6 Changed 8 weeks ago by rjollos
- Resolution set to fixed
- Status changed from new to closed
BookmarkPlugin: Perform Unix shell-style wildcard matching (globs), as is documented, for the paths option. Previously, a regular expression was used to match paths.
comment:7 Changed 8 weeks ago by rjollos
(In [13012]) Fixes #11037, Refs #10226:
VotePlugin: Perform Unix shell-style wildcard matching (globs), as is documented, for the paths option. Previously, a regular expression was used to match paths.


Yeah, that makes sense. +1 from me.