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

Issue 6201063: code review 6201063: regexp/syntax: replace internal error on unexpected ) w... (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 10 months ago by jan_mercl
Modified:
11 years, 10 months ago
Reviewers:
minux1, adg, bradfitz
CC:
r, rsc, golang-dev
Visibility:
Public.

Description

regexp/syntax: replace internal error on unexpected ) w/ ErrUnexpectedParen Unbalanced extra right parenthesis produced an internal error instead of a more descriptive one. Fixes issue 3406.

Patch Set 1 #

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

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

Patch Set 4 : diff -r 9182664c616f https://code.google.com/p/go #

Total comments: 6

Patch Set 5 : diff -r 9182664c616f https://code.google.com/p/go #

Unified diffs Side-by-side diffs Delta from patch set Stats (+12 lines, -2 lines) Patch
M api/go1.txt View 1 chunk +1 line, -0 lines 0 comments Download
M src/pkg/regexp/syntax/parse.go View 1 2 3 4 2 chunks +3 lines, -2 lines 0 comments Download
M src/pkg/regexp/syntax/parse_test.go View 1 2 3 4 1 chunk +8 lines, -0 lines 0 comments Download

Messages

Total messages: 14
jan_mercl
Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com, rsc@golang.org), I'd like you to review this change to https://code.google.com/p/go
11 years, 10 months ago (2012-05-08 22:54:30 UTC) #1
r
please s/Repleace/replace/ in the CL description. there are two characters to change.
11 years, 10 months ago (2012-05-09 17:36:19 UTC) #2
jan_mercl
Hello golang-dev@googlegroups.com, r@golang.org (cc: golang-dev@googlegroups.com, rsc@golang.org), Please take another look.
11 years, 10 months ago (2012-05-09 17:43:27 UTC) #3
jan_mercl
On 2012/05/09 17:36:19, r wrote: > please s/Repleace/replace/ in the CL description. there are two ...
11 years, 10 months ago (2012-05-09 17:44:58 UTC) #4
rsc
Thanks for fixing this. http://codereview.appspot.com/6201063/diff/4007/src/pkg/regexp/all_test.go File src/pkg/regexp/all_test.go (right): http://codereview.appspot.com/6201063/diff/4007/src/pkg/regexp/all_test.go#newcode8 src/pkg/regexp/all_test.go:8: "regexp/syntax" Instead of importing and ...
11 years, 10 months ago (2012-05-09 17:50:16 UTC) #5
jan_mercl
Hello r@golang.org, rsc@golang.org (cc: golang-dev@googlegroups.com), Please take another look.
11 years, 10 months ago (2012-05-09 18:48:11 UTC) #6
jan_mercl
http://codereview.appspot.com/6201063/diff/4007/src/pkg/regexp/all_test.go File src/pkg/regexp/all_test.go (right): http://codereview.appspot.com/6201063/diff/4007/src/pkg/regexp/all_test.go#newcode8 src/pkg/regexp/all_test.go:8: "regexp/syntax" On 2012/05/09 17:50:16, rsc wrote: > Instead of ...
11 years, 10 months ago (2012-05-09 18:48:42 UTC) #7
r
LGTM
11 years, 10 months ago (2012-05-14 18:49:25 UTC) #8
r
*** Submitted as http://code.google.com/p/go/source/detail?r=27d0a516b7eb *** regexp/syntax: replace internal error on unexpected ) w/ ErrUnexpectedParen Unbalanced ...
11 years, 10 months ago (2012-05-14 18:50:32 UTC) #9
minux1
is it ok to change api/go1.txt now?
11 years, 10 months ago (2012-05-17 06:58:21 UTC) #10
bradfitz
The plan is to never change it. We'll only add new files, one per promise. ...
11 years, 10 months ago (2012-05-17 07:01:57 UTC) #11
minux1
On Thu, May 17, 2012 at 3:01 PM, Brad Fitzpatrick <bradfitz@golang.org>wrote: > The plan is ...
11 years, 10 months ago (2012-05-17 07:05:43 UTC) #12
jan_mercl
Dne 17.5.2012 9:05 "minux" <minux.ma@gmail.com> napsal(a): > > > On Thu, May 17, 2012 at ...
11 years, 10 months ago (2012-05-17 07:23:04 UTC) #13
adg
11 years, 10 months ago (2012-05-17 07:59:30 UTC) #14
On 17 May 2012 17:23, Honza <befelemepeseveze@gmail.com> wrote:
> Dne 17.5.2012 9:05 "minux" <minux.ma@gmail.com> napsal(a):
>
>
>>
>>
>> On Thu, May 17, 2012 at 3:01 PM, Brad Fitzpatrick <bradfitz@golang.org>
>> wrote:
>>>
>>> The plan is to never change it.
>>>
>>> We'll only add new files, one per promise.
>>>
>>> go1 is now a frozen promise.
>>
>> but this CL changed it.
>> I meant to ask this before its submission, but I forgot to do so.
>
> Then it's my fault. I thought that bug fixes may not change existing API
> entities semantics nor delete anything to keep all existing Go1 code
> working. That said, I thought adding an entity to fix an issue is allowed.
> What to do now?

The api tool now permits API additions. We can back out the go1.txt change.

Andrew
Sign in to reply to this message.

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