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

Issue 7133051: code review 7133051: regexp: add (*Regexp).Longest (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 2 months ago by adg
Modified:
11 years, 1 month ago
Reviewers:
CC:
rsc, golang-dev
Visibility:
Public.

Description

regexp: add (*Regexp).Longest Fixes issue 3696.

Patch Set 1 #

Patch Set 2 : diff -r a55bd9f21507 https://code.google.com/p/go #

Total comments: 4

Patch Set 3 : diff -r 36925180b9ef https://code.google.com/p/go #

Unified diffs Side-by-side diffs Delta from patch set Stats (+22 lines, -0 lines) Patch
M src/pkg/regexp/exec_test.go View 1 2 1 chunk +14 lines, -0 lines 0 comments Download
M src/pkg/regexp/regexp.go View 1 1 chunk +8 lines, -0 lines 0 comments Download

Messages

Total messages: 16
adg
Hello golang-dev@googlegroups.com, I'd like you to review this change to https://code.google.com/p/go
11 years, 2 months ago (2013-01-17 04:24:52 UTC) #1
adg
I named the method LeftmostLongest as per the issue, but perhaps the name Longest would ...
11 years, 2 months ago (2013-01-17 04:27:37 UTC) #2
rsc
Longest sounds good (drop Leftmost). CompileLongest is not great because next you need MustCompileLongest.
11 years, 2 months ago (2013-01-17 04:39:16 UTC) #3
dsymonds
Can it return the *Regexp to allow the builder pattern? That's pretty common for regular ...
11 years, 2 months ago (2013-01-17 04:44:43 UTC) #4
adg
On 17 January 2013 15:44, David Symonds <dsymonds@golang.org> wrote: > Can it return the *Regexp ...
11 years, 2 months ago (2013-01-17 04:53:25 UTC) #5
adg
Hello golang-dev@googlegroups.com, rsc@golang.org, dsymonds@golang.org (cc: golang-dev@googlegroups.com), Please take another look.
11 years, 2 months ago (2013-01-17 04:54:09 UTC) #6
adg
On 17 January 2013 15:52, Andrew Gerrand <adg@golang.org> wrote: > > On 17 January 2013 ...
11 years, 2 months ago (2013-01-17 04:54:17 UTC) #7
dsymonds
On Thu, Jan 17, 2013 at 3:52 PM, Andrew Gerrand <adg@golang.org> wrote: > On 17 ...
11 years, 2 months ago (2013-01-17 04:55:53 UTC) #8
dsymonds
On Thu, Jan 17, 2013 at 3:53 PM, Andrew Gerrand <adg@golang.org> wrote: > The question ...
11 years, 2 months ago (2013-01-17 04:56:58 UTC) #9
dsymonds
https://codereview.appspot.com/7133051/diff/6001/src/pkg/regexp/exec_test.go File src/pkg/regexp/exec_test.go (right): https://codereview.appspot.com/7133051/diff/6001/src/pkg/regexp/exec_test.go#newcode712 src/pkg/regexp/exec_test.go:712: re, err := Compile(`a(|b)`) I found it curious that ...
11 years, 2 months ago (2013-01-17 05:02:19 UTC) #10
rsc
The builder pattern is not very common in the Go libraries, and I don't expect ...
11 years, 2 months ago (2013-01-17 14:08:22 UTC) #11
kevlar
On 2013/01/17 14:08:22, rsc wrote: > The builder pattern is not very common in the ...
11 years, 2 months ago (2013-01-18 17:14:44 UTC) #12
rsc
> Out of curiosity, is there a reason this couldn't be a (?...)-param? > That ...
11 years, 2 months ago (2013-01-18 17:46:08 UTC) #13
rsc
LGTM https://codereview.appspot.com/7133051/diff/6001/src/pkg/regexp/exec_test.go File src/pkg/regexp/exec_test.go (right): https://codereview.appspot.com/7133051/diff/6001/src/pkg/regexp/exec_test.go#newcode712 src/pkg/regexp/exec_test.go:712: re, err := Compile(`a(|b)`) On 2013/01/17 05:02:19, dsymonds ...
11 years, 1 month ago (2013-02-01 19:46:12 UTC) #14
rsc
ping (lgtm)
11 years, 1 month ago (2013-02-04 04:16:25 UTC) #15
adg
11 years, 1 month ago (2013-02-04 04:29:01 UTC) #16
*** Submitted as https://code.google.com/p/go/source/detail?r=3529e745c5d5 ***

regexp: add (*Regexp).Longest

Fixes issue 3696.

R=rsc
CC=golang-dev
https://codereview.appspot.com/7133051
Sign in to reply to this message.

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