Opened 15 years ago

Closed 11 years ago

#7117 closed Bug (fixed)

Replace backslash in reverse URL matcher

Reported by: Bastian Kleineidam <calvin@…> Owned by: nobody
Component: Core (Other) Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Design decision needed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


The reverse URL pattern matcher should replace backslashes
stemming from regex escapes in the result URL.
Added also a test case.

Attachments (1)

commit-454b8e2 (1.4 KB) - added by Bastian Kleineidam <calvin@…> 15 years ago.

Download all attachments as: .zip

Change History (6)

Changed 15 years ago by Bastian Kleineidam <calvin@…>

Attachment: commit-454b8e2 added

comment:1 Changed 15 years ago by Chris Beaven

Component: UncategorizedCore framework
Needs tests: set
Triage Stage: UnreviewedDesign decision needed

There are a lot more things to do to fix up the reverse URL matcher than this, but it is a simple change. I'll push to a design decision.

Should probably have a test case.

comment:2 Changed 15 years ago by Bastian Kleineidam <calvin@…>

Needs tests: unset

There is a test case in the patch, it's just very small :)

What is it that needs fixing up in the reverse matcher?
Perhaps it is the fact that the reverse matcher cannot be a (perfect) function, since there might be two or more URLs that resolve to the same view. For example this pattern:

('docs/index[12].doc', 'docs.display')

resolves two different URLs to the exact same view. Same with multiple patterns pointing to the same view:

('docs/index1.doc', 'docs.display')
('docs/index2.doc', 'docs.display')

A reverse function for docs.display does not exist then. Perhaps {{reverse()}}} should throw ValueError in these cases to make the user aware of this issue, but that is a more invasive change.

comment:3 Changed 15 years ago by Chris Beaven

Sorry Bastian, i 10 second scanned the patch and overlooked the test :)

The reverse matcher can be much more sane, even if there are multiple cases. My patch the ancient (still open) #2977 ticket covers most cases. But, like I said - yours is a simple fix and a reasonably common case.

comment:4 Changed 12 years ago by Luke Plant

Severity: Normal
Type: Bug

comment:5 Changed 11 years ago by Chris Beaven

Easy pickings: unset
Resolution: fixed
Status: newclosed
UI/UX: unset

Fixed (3 years ago in r8760)

Note: See TracTickets for help on using tickets.
Back to Top