Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(3092)

Issue 183044: code review 183044: Add query to find number of subexpressions. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
14 years, 3 months ago by phf
Modified:
14 years, 3 months ago
Reviewers:
CC:
r, rsc, golang-dev
Visibility:
Public.

Description

Add query to find number of subexpressions. This was convenient for me to have without being forced to parse the regexp myself. I'd understand if it's not really wanted, but I also think that some meta information about compiled regexps would be fine.

Patch Set 1 #

Patch Set 2 : code review 183044: Add query to find number of subexpressions. #

Patch Set 3 : code review 183044: Add query to find number of subexpressions. #

Patch Set 4 : code review 183044: Add query to find number of subexpressions. #

Patch Set 5 : code review 183044: Add query to find number of subexpressions. #

Total comments: 2

Patch Set 6 : code review 183044: Add query to find number of subexpressions. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+31 lines, -0 lines) Patch
M src/pkg/regexp/all_test.go View 5 1 chunk +28 lines, -0 lines 0 comments Download
M src/pkg/regexp/regexp.go View 1 2 3 4 5 1 chunk +3 lines, -0 lines 0 comments Download

Messages

Total messages: 15
phf
Hello r, rsc (cc: golang-dev@googlegroups.com), I'd like you to review the following change.
14 years, 3 months ago (2009-12-23 19:12:07 UTC) #1
r2
The idea is fine but the name is too long for my sensibilities. How about ...
14 years, 3 months ago (2009-12-23 20:36:18 UTC) #2
phf
NumSubexps? Shouldn't it be "plural"? But I am fine with all of these names, just ...
14 years, 3 months ago (2009-12-23 20:40:31 UTC) #3
rsc
I suggest NumSub, which dodges the whole question of how to abbreviate expression. Singular is ...
14 years, 3 months ago (2009-12-23 20:42:57 UTC) #4
r2
On 24/12/2009, at 7:42 AM, Russ Cox wrote: > I suggest NumSub, which dodges the ...
14 years, 3 months ago (2009-12-23 20:45:44 UTC) #5
rsc
sure
14 years, 3 months ago (2009-12-23 20:47:01 UTC) #6
phf
Doh! Forgot a test case or two. Working on it... :-D
14 years, 3 months ago (2009-12-23 20:57:15 UTC) #7
phf
How do I add a file (the test cases) to this changelist?
14 years, 3 months ago (2009-12-23 21:12:58 UTC) #8
phf
PTAL
14 years, 3 months ago (2009-12-23 21:15:39 UTC) #9
rsc
it's more common in the tree to s/Case/Test/
14 years, 3 months ago (2009-12-23 21:17:34 UTC) #10
r
http://codereview.appspot.com/183044/diff/1014/1015 File src/pkg/regexp/all_test.go (right): http://codereview.appspot.com/183044/diff/1014/1015#newcode470 src/pkg/regexp/all_test.go:470: numSubexpCase{"(.*)((a)b)(.*)a", 4}, this is overkill but good. for black ...
14 years, 3 months ago (2009-12-23 21:20:33 UTC) #11
r2
On 24/12/2009, at 8:17 AM, Russ Cox wrote: > it's more common in the tree ...
14 years, 3 months ago (2009-12-23 21:21:10 UTC) #12
phf
PTAL
14 years, 3 months ago (2009-12-23 21:34:50 UTC) #13
r
LGTM
14 years, 3 months ago (2009-12-23 21:42:39 UTC) #14
r
14 years, 3 months ago (2009-12-23 21:43:38 UTC) #15
*** Submitted as http://code.google.com/p/go/source/detail?r=55d4c0d9c6e9 ***

Add query to find number of subexpressions.

This was convenient for me to have without being forced
to parse the regexp myself. I'd understand if it's not
really wanted, but I also think that some meta information
about compiled regexps would be fine.

R=r, rsc
CC=golang-dev
http://codereview.appspot.com/183044

Committer: Rob Pike <r@golang.org>
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b