code review 6202062: exp/locale/collate: implementation of main collation fu...

exp/locale/collate: implementation of main collation functionality for
key and simple comparisson. Search is not yet implemented in this CL.
Changed some of the types of table_test.go to allow reuse in the new test.
Also reduced number of primary values for illegal runes to 1 (both map to
the same).

[mpvl, 2012-05-09 13:10:20]

Hello r@golang.org (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://go.googlecode.com/hg

[r, 2012-05-10 18:59:17]

LGTM
nice

http://codereview.appspot.com/6202062/diff/4001/src/pkg/exp/locale/collate/co...
File src/pkg/exp/locale/collate/collate.go (right):

http://codereview.appspot.com/6202062/diff/4001/src/pkg/exp/locale/collate/co...
src/pkg/exp/locale/collate/collate.go:127: // Compares calls ResetKeys, thereby
invalidating keys
s/Compares/Compare/

http://codereview.appspot.com/6202062/diff/4001/src/pkg/exp/locale/collate/co...
src/pkg/exp/locale/collate/collate.go:143: // Compares calls ResetKeys, thereby
invalidating keys
s/Compares/CompareString/

http://codereview.appspot.com/6202062/diff/4001/src/pkg/exp/locale/collate/co...
src/pkg/exp/locale/collate/collate.go:171: // The returned slice will point to
an allocation in Buffer and will retain
s/retain/remain/

http://codereview.appspot.com/6202062/diff/4001/src/pkg/exp/locale/collate/co...
File src/pkg/exp/locale/collate/collate_test.go (right):

http://codereview.appspot.com/6202062/diff/4001/src/pkg/exp/locale/collate/co...
src/pkg/exp/locale/collate/collate_test.go:33: }
add blank line

http://codereview.appspot.com/6202062/diff/4001/src/pkg/exp/locale/collate/co...
src/pkg/exp/locale/collate/collate_test.go:202: t.Errorf("%d: len(ws) was %d;
want %d (%v vs %v)", i, len(res), len(tt.out), res, tt.out)
s/vs/should be/

http://codereview.appspot.com/6202062/diff/4001/src/pkg/exp/locale/collate/co...
src/pkg/exp/locale/collate/collate_test.go:307: t.Errorf("%d: len(ws) was %d;
want %d (%X vs %X)", i, len(res), len(tt.out), res, tt.out)
ditto

http://codereview.appspot.com/6202062/diff/4001/src/pkg/exp/locale/collate/co...
src/pkg/exp/locale/collate/collate_test.go:319: fmt.Printf("\n")
d

http://codereview.appspot.com/6202062/diff/4001/src/pkg/exp/locale/collate/co...
src/pkg/exp/locale/collate/collate_test.go:417: t.Errorf(`%d: Compare("%s",
"%s") == %d; want %d`, i, tt.a, tt.b, res, tt.res)
use %q on these string args?

http://codereview.appspot.com/6202062/diff/4001/src/pkg/exp/locale/collate/co...
src/pkg/exp/locale/collate/collate_test.go:420: t.Errorf(`%d:
CompareString("%s", "%s") == %d; want %d`, i, tt.a, tt.b, res, tt.res)
ditto

[mpvl, 2012-05-17 17:49:04]

*** Submitted as http://code.google.com/p/go/source/detail?r=0581aafd0aaf ***

exp/locale/collate: implementation of main collation functionality for
key and simple comparisson. Search is not yet implemented in this CL.
Changed some of the types of table_test.go to allow reuse in the new test.
Also reduced number of primary values for illegal runes to 1 (both map to
the same).

R=r
CC=golang-dev
http://codereview.appspot.com/6202062

http://codereview.appspot.com/6202062/diff/4001/src/pkg/exp/locale/collate/co...
File src/pkg/exp/locale/collate/collate.go (right):

http://codereview.appspot.com/6202062/diff/4001/src/pkg/exp/locale/collate/co...
src/pkg/exp/locale/collate/collate.go:127: // Compares calls ResetKeys, thereby
invalidating keys
On 2012/05/10 18:59:17, r wrote:
> s/Compares/Compare/

Done.

http://codereview.appspot.com/6202062/diff/4001/src/pkg/exp/locale/collate/co...
src/pkg/exp/locale/collate/collate.go:143: // Compares calls ResetKeys, thereby
invalidating keys
On 2012/05/10 18:59:17, r wrote:
> s/Compares/CompareString/

Done.

http://codereview.appspot.com/6202062/diff/4001/src/pkg/exp/locale/collate/co...
src/pkg/exp/locale/collate/collate.go:171: // The returned slice will point to
an allocation in Buffer and will retain
On 2012/05/10 18:59:17, r wrote:
> s/retain/remain/

Done.

http://codereview.appspot.com/6202062/diff/4001/src/pkg/exp/locale/collate/co...
File src/pkg/exp/locale/collate/collate_test.go (right):

http://codereview.appspot.com/6202062/diff/4001/src/pkg/exp/locale/collate/co...
src/pkg/exp/locale/collate/collate_test.go:33: }
On 2012/05/10 18:59:17, r wrote:
> add blank line

Done.

http://codereview.appspot.com/6202062/diff/4001/src/pkg/exp/locale/collate/co...
src/pkg/exp/locale/collate/collate_test.go:202: t.Errorf("%d: len(ws) was %d;
want %d (%v vs %v)", i, len(res), len(tt.out), res, tt.out)
On 2012/05/10 18:59:17, r wrote:
> s/vs/should be/

Done.

http://codereview.appspot.com/6202062/diff/4001/src/pkg/exp/locale/collate/co...
src/pkg/exp/locale/collate/collate_test.go:307: t.Errorf("%d: len(ws) was %d;
want %d (%X vs %X)", i, len(res), len(tt.out), res, tt.out)
On 2012/05/10 18:59:17, r wrote:
> ditto

Done.

http://codereview.appspot.com/6202062/diff/4001/src/pkg/exp/locale/collate/co...
src/pkg/exp/locale/collate/collate_test.go:319: fmt.Printf("\n")
On 2012/05/10 18:59:17, r wrote:
> d

Done.

http://codereview.appspot.com/6202062/diff/4001/src/pkg/exp/locale/collate/co...
src/pkg/exp/locale/collate/collate_test.go:417: t.Errorf(`%d: Compare("%s",
"%s") == %d; want %d`, i, tt.a, tt.b, res, tt.res)
On 2012/05/10 18:59:17, r wrote:
> use %q on these string args?

Done.

http://codereview.appspot.com/6202062/diff/4001/src/pkg/exp/locale/collate/co...
src/pkg/exp/locale/collate/collate_test.go:420: t.Errorf(`%d:
CompareString("%s", "%s") == %d; want %d`, i, tt.a, tt.b, res, tt.res)
On 2012/05/10 18:59:17, r wrote:
> ditto

Done.