code review 153220043: go.text/cases: Unicode tables for casing operations.

go.text/cases: Unicode tables for casing operations.

[nigeltao, 2014-10-08 08:34:31]

Hello mpvl@golang.org (cc: golang-codereviews@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go.text

[mpvl, 2014-10-08 09:27:27]

LGTM, with some notes.

Note that this was more or less the structure of the first iteration, but
without the Makefile. I'm okay not having a Makefile now, as we're getting the
generate mechanism.  Should we still call this maketables.go, though, to be
consistent? With the upcoming go generate tool, I'm okay with using this
intuitive name and not be consistent. Just making sure we consider it, though.

https://codereview.appspot.com/153220043/diff/40001/cases/gen.go
File cases/gen.go (right):

https://codereview.appspot.com/153220043/diff/40001/cases/gen.go#newcode807
cases/gen.go:807: // printProperties prints tables of rune properties from the
give UCD file.
s/give/given/

https://codereview.appspot.com/153220043/diff/40001/cases/gen.go#newcode813
cases/gen.go:813: varName := strings.ToLower(property)
How about a more generic approach to camel casing:

a := strings.Split(property, "_")
a[0] = strings.ToLower(a[0])
property = strings.Join(a, "")

or

property = strings.ToLower(property[:1]) + strings.Replace(property[1:], "_",
"") // property is always ASCII

This isn't more code, but would also work if we add other properties like
Soft_Dotted.

https://codereview.appspot.com/153220043/diff/40001/cases/gen.go#newcode851
cases/gen.go:851: }
Is this check really necessary?

[nigeltao, 2014-10-09 05:18:29]

Re file naming, my instinct went with "gen.go" to match e.g.:
http://golang.org/src/pkg/compress/flate/gen.go
http://golang.org/src/pkg/crypto/md5/gen.go
https://code.google.com/p/go/source/browse/gl/gendebug.go?repo=mobile
https://code.google.com/p/go/source/browse/html/atom/gen.go?repo=net
https://code.google.com/p/go/source/browse/ipv4/gen.go?repo=net
https://code.google.com/p/go/source/browse/publicsuffix/gen.go?repo=net
https://code.google.com/p/rsc/source/browse/qr/coding/gen.go

I suppose that the counter-examples are
http://golang.org/src/pkg/unicode/maketables.go
and all the existing maketables.go files in the go.text sub-repo.

Anyway, submitting as "gen.go"...

https://codereview.appspot.com/153220043/diff/40001/cases/gen.go
File cases/gen.go (right):

https://codereview.appspot.com/153220043/diff/40001/cases/gen.go#newcode807
cases/gen.go:807: // printProperties prints tables of rune properties from the
give UCD file.
On 2014/10/08 09:27:27, mpvl wrote:
> s/give/given/

Done.

https://codereview.appspot.com/153220043/diff/40001/cases/gen.go#newcode813
cases/gen.go:813: varName := strings.ToLower(property)
On 2014/10/08 09:27:27, mpvl wrote:
> How about a more generic approach to camel casing:
> 
> a := strings.Split(property, "_")
> a[0] = strings.ToLower(a[0])
> property = strings.Join(a, "")
> 
> or
> 
> property = strings.ToLower(property[:1]) + strings.Replace(property[1:], "_",
> "") // property is always ASCII
> 
> This isn't more code, but would also work if we add other properties like
> Soft_Dotted.

Done.

https://codereview.appspot.com/153220043/diff/40001/cases/gen.go#newcode851
cases/gen.go:851: }
On 2014/10/08 09:27:27, mpvl wrote:
> Is this check really necessary?

Not really. Removed.

[nigeltao, 2014-10-09 05:20:59]

*** Submitted as
https://code.google.com/p/go/source/detail?r=1ac75970ff9e&repo=text ***

go.text/cases: Unicode tables for casing operations.

LGTM=mpvl
R=mpvl
CC=golang-codereviews
https://codereview.appspot.com/153220043