Hello golang-dev@googlegroups.com, I'd like you to review this change to https://code.google.com/p/go
Hold your horses on this one - it causes all kinds of (testing) trouble. - gri On Mon, May 21, 2012 at 3:24 PM, <gri@golang.org> wrote: > Reviewers: golang-dev_googlegroups.com, > > Message: > Hello golang-dev@googlegroups.com, > > I'd like you to review this change to > https://code.google.com/p/go > > > Description: > go/ast: Remove irregularity from CommentGroup.Text. > > Also: > - more precise documentation > - added corresponding test case > > Please review this at http://codereview.appspot.com/6206096/ > > Affected files: > M src/pkg/go/ast/ast.go > A src/pkg/go/ast/ast_test.go > > > Index: src/pkg/go/ast/ast.go > =================================================================== > --- a/src/pkg/go/ast/ast.go > +++ b/src/pkg/go/ast/ast.go > @@ -87,8 +87,11 @@ > return s[0:i] > } > > -// Text returns the text of the comment, > -// with the comment markers - //, /*, and */ - removed. > +// Text returns the text of the comment. > +// Comment markers (//, /*, and */), leading and trailing empty lines are > +// removed, multiple empty lines are reduced to one, and trailing space on > +// lines is trimmed. Unless the result is empty, it is newline-terminated. > +// > func (g *CommentGroup) Text() string { > if g == nil { > return "" > @@ -104,14 +107,8 @@ > // The parser has given us exactly the comment text. > switch c[1] { > case '/': > - //-style comment > + //-style comment (no newline at the end) > c = c[2:] > - // Remove leading space after //, if there is one. > - // TODO(gri) This appears to be necessary in > isolated > - // cases (bignum.RatFromString) - why? > - if len(c) > 0 && c[0] == ' ' { > - c = c[1:] > - } > case '*': > /*-style comment */ > c = c[2 : len(c)-2] > Index: src/pkg/go/ast/ast_test.go > =================================================================== > new file mode 100644 > --- /dev/null > +++ b/src/pkg/go/ast/ast_test.go > @@ -0,0 +1,50 @@ > +// Copyright 2012 The Go Authors. All rights reserved. > +// Use of this source code is governed by a BSD-style > +// license that can be found in the LICENSE file. > + > +package ast > + > +import ( > + "testing" > +) > + > +var comments = []struct { > + list []string > + text string > +}{ > + {[]string{"//"}, ""}, > + {[]string{"// "}, ""}, > + {[]string{"//", "//", "// "}, ""}, > + {[]string{"// foo "}, " foo\n"}, > + {[]string{"//", "//", "// foo"}, " foo\n"}, > + {[]string{"// foo bar "}, " foo bar\n"}, > + {[]string{"// foo", "// bar"}, " foo\n bar\n"}, > + {[]string{"// foo", "//", "//", "//", "// bar"}, " foo\n\n bar\n"}, > + {[]string{"// foo", "/* bar */"}, " foo\n bar\n"}, > + {[]string{"//", "//", "//", "// foo", "//", "//", "//"}, " foo\n"}, > + > + {[]string{"/**/"}, ""}, > + {[]string{"/* */"}, ""}, > + {[]string{"/**/", "/**/", "/* */"}, ""}, > + {[]string{"/* Foo */"}, " Foo\n"}, > + {[]string{"/* Foo Bar */"}, " Foo Bar\n"}, > + {[]string{"/* Foo*/", "/* Bar*/"}, " Foo\n Bar\n"}, > + {[]string{"/* Foo*/", "/**/", "/**/", "/**/", "// Bar"}, " Foo\n\n > Bar\n"}, > + {[]string{"/* Foo*/", "/*\n*/", "//", "/*\n*/", "// Bar"}, " Foo\n\n > Bar\n"}, > + {[]string{"/* Foo*/", "// Bar"}, " Foo\n Bar\n"}, > + {[]string{"/* Foo\n Bar*/"}, " Foo\n Bar\n"}, > +} > + > +func TestCommentText(t *testing.T) { > + for i, c := range comments { > + list := make([]*Comment, len(c.list)) > + for i, s := range c.list { > + list[i] = &Comment{Text: s} > + } > + > + text := (&CommentGroup{list}).Text() > + if text != c.text { > + t.Errorf("case %d: got %q; expected %q", i, text, > c.text) > + } > + } > +} > >
Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com), Please take another look.
LGTM
*** Submitted as http://code.google.com/p/go/source/detail?r=3446429f1ec4 *** go/ast: document CommentGroup.Text and add test case. R=golang-dev, rsc CC=golang-dev http://codereview.appspot.com/6206096
