Modify

Opened 2 years ago

Closed 15 months ago

Last modified 15 months ago

#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 22 months ago by rjollos

Yeah, that makes sense. +1 from me.

comment:2 Changed 16 months ago by rjollos

  • Cc hasienda added

Same issue was noted with the VotePlugin in comment:5:ticket:10942.

comment:3 Changed 15 months ago by rjollos

It looks like fnmatch will accomplish what we want. How does the following patch look?

  • tracbookmark/__init__.py

     
    99# you should have received as part of this distribution. 
    1010 
    1111import re 
     12from fnmatch import fnmatch 
    1213 
    1314from genshi.builder import tag 
    1415from trac.config import ListOption 
     
    168169        # Show bookmarks context menu except when on the bookmark page 
    169170        if self._authorize(req) and not self.match_request(req): 
    170171            for path in self.bookmarkable_paths: 
    171                 if re.match(path, req.path_info): 
     172                if fnmatch(req.path_info, path): 
    172173                    self.render_bookmarker(req) 
    173174                    break 
    174175        return template, data, content_type 

comment:4 follow-up: Changed 15 months ago by rjollos

On second look, we might want to use fnmatchcase.

comment:5 in reply to: ↑ 4 Changed 15 months 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 15 months ago by rjollos

  • Resolution set to fixed
  • Status changed from new to closed

(In [13011]) Fixes #10226:

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 15 months 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.

Add Comment

Modify Ticket

Action
as closed .
as The resolution will be set. Next status will be 'closed'.
to The owner will be changed from saigon. Next status will be 'closed'.
The resolution will be deleted. Next status will be 'reopened'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.