Opened 8 years ago

Closed 3 years ago

#28135 closed Cleanup/optimization (fixed)

simplify_regex() doesn't handle non-capturing groups

Reported by: German M. Bravo Owned by: Ayush Joshi
Component: contrib.admindocs Version: 1.11
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description (last modified by Mariusz Felisiak)

While using Django REST Framework's Schema generator, I found out they're using simplify_regex(); however, current version has a few shortcomings, namely non-capturing groups are broken.

Change History (21)

comment:1 by Tim Graham, 8 years ago

Component: Uncategorized → contrib.admindocs
Has patch: set
Triage Stage: Unreviewed → Accepted
Type: Uncategorized → Cleanup/optimization

comment:2 by Tim Graham, 7 years ago

Patch needs improvement: set

I left comments for improvement on the PR. Please uncheck "Patch needs improvement" after updating.

comment:3 by David Smith, 4 years ago

Easy pickings: set

comment:4 by Rohith P R, 4 years ago

​https://github.com/django/django/pull/14070 : Refs #28135 -- Added replace_metacharacters to simplify regexes

Version 0, edited 4 years ago by Rohith P R (next)

comment:5 by Rohith P R, 4 years ago

Has patch: unset
Patch needs improvement: unset

comment:6 by Felix Zhang, 3 years ago

Description: modified (diff)
Owner: changed from nobody to Felix Zhang
Status: new → assigned

comment:7 by Felix Zhang, 3 years ago

Description: modified (diff)
Has patch: set

I have opened a ​pull request that allows simplify_regex() to handle non-capturing groups and additional tests for them in test_simplify_regex().

Last edited 3 years ago by Mariusz Felisiak (previous) (diff)

comment:8 by Mariusz Felisiak, 3 years ago

Description: modified (diff)

comment:9 by Mariusz Felisiak, 3 years ago

Patch needs improvement: set

comment:10 by Ayush Joshi, 3 years ago

Owner: changed from Felix Zhang to Ayush Joshi

comment:11 by Ayush Joshi, 3 years ago

I've submitted a patch here ​https://github.com/django/django/pull/15256.

Last edited 3 years ago by Ayush Joshi (previous) (diff)

comment:12 by Tim Graham, 3 years ago

Patch needs improvement: unset

You need to uncheck "Patch needs improvement" so the ticket appears in the review queue. Please don't use mass @ mentions on the PR to request a review.

in reply to:  12 comment:13 by Ayush Joshi, 3 years ago

Replying to Tim Graham:

You need to uncheck "Patch needs improvement" so the ticket appears in the review queue. Please don't use mass @ mentions on the PR to request a review.

Sorry, Felisiak never came so I have to ask someone else lol

comment:14 by Mariusz Felisiak, 3 years ago

Patch needs improvement: set

in reply to:  14 comment:15 by Ayush Joshi, 3 years ago

Patch needs improvement: unset

Replying to Mariusz Felisiak:

Okay I updated that PR to only include feature, I'll create new PR for code refactor :thumbsup:.

comment:16 by Mariusz Felisiak <felisiak.mariusz@…>, 3 years ago

In 827bc070:

Refs #28135 -- Refactored out _find_groups()/_get_group_start_end() hooks in admindocs.

comment:17 by Mariusz Felisiak, 3 years ago

Has patch: unset

comment:19 by Ayush Joshi, 3 years ago

Has patch: set

comment:20 by Mariusz Felisiak, 3 years ago

Triage Stage: Accepted → Ready for checkin

comment:21 by Mariusz Felisiak <felisiak.mariusz@…>, 3 years ago

Resolution: → fixed
Status: assigned → closed

In 0a17666:

Fixed #28135 -- Made simplify_regex() handle non-capturing groups.

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