code review 4526095: unicode: add the first few property tests for printing.

unicode: add the first few property tests for printing.

The long-term goal is that %q will use IsPrint to decide
what to show natively vs. as hexadecimal.

[r, 2011-06-01 04:44:44]

Hello golang-dev@googlegroups.com,

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

[r, 2011-06-02 07:43:24]

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

Please take another look.

[rsc, 2011-06-02 14:29:39]

The Latin-1 table looks fine, but I don't think
isOneOf is the right thing to do for the rest.
Generating the full ranges in maketables is only
a little larger, and it makes the tables available
to other packages that might want to enumerate
the code points in, say, M in order.

[r, 2011-06-03 05:13:30]

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

Please take another look.

[rsc, 2011-06-03 05:29:30]

LGTM

http://codereview.appspot.com/4526095/diff/8001/src/pkg/unicode/graphic.go
File src/pkg/unicode/graphic.go (right):

http://codereview.appspot.com/4526095/diff/8001/src/pkg/unicode/graphic.go#ne...
src/pkg/unicode/graphic.go:14: pU             // an upper-case letter.
pLu, pLl?

http://codereview.appspot.com/4526095/diff/8001/src/pkg/unicode/graphic.go#ne...
src/pkg/unicode/graphic.go:26: L, M, N, P, S, // U+0020 is added by hand.
s/added by hand/handled by the properties table/

http://codereview.appspot.com/4526095/diff/8001/src/pkg/unicode/graphic.go#ne...
src/pkg/unicode/graphic.go:42: // characters include letters, marks, numbers,
punctuation, symbols, and the
... symbols, from categories L, M, N, P, S, and the ASCII space character.

http://codereview.appspot.com/4526095/diff/8001/src/pkg/unicode/graphic.go#ne...
src/pkg/unicode/graphic.go:65: // The C* (Other) Unicode categories includes
more code points
C (Other) Unicode category includes

etc.  the unicode regexp syntax does not have the *
and i don't think i've seen it elsewhere; also the * is
not used in the lists above.

same change below

http://codereview.appspot.com/4526095/diff/8001/src/pkg/unicode/graphic.go#ne...
src/pkg/unicode/graphic.go:71: // All control characters are <= 0x100.
s/<=/</

http://codereview.appspot.com/4526095/diff/8001/src/pkg/unicode/letter.go
File src/pkg/unicode/letter.go (right):

http://codereview.appspot.com/4526095/diff/8001/src/pkg/unicode/letter.go#new...
src/pkg/unicode/letter.go:149: // IsUpper reports whether the rune is an upper
case letter.
is the plan to use the table for these too?

http://codereview.appspot.com/4526095/diff/8001/src/pkg/unicode/tables.go
File src/pkg/unicode/tables.go (right):

http://codereview.appspot.com/4526095/diff/8001/src/pkg/unicode/tables.go#new...
src/pkg/unicode/tables.go:2711: Letter = _L  // Letter/L is the set of Unicode
letters, categories L*.
same comment about * (in generator)

http://codereview.appspot.com/4526095/diff/8001/src/pkg/unicode/tables.go#new...
src/pkg/unicode/tables.go:5415: 0X00: pC,      // '\x00'
use 0x00 not 0X00 just for readability?

[r, 2011-06-03 07:34:47]

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

Please take another look.

[r2, 2011-06-03 07:36:38]

This is now ready for proper review.

-rob

[rsc, 2011-06-03 15:08:34]

LGTM

http://codereview.appspot.com/4526095/diff/13001/src/pkg/unicode/graphic.go
File src/pkg/unicode/graphic.go (right):

http://codereview.appspot.com/4526095/diff/13001/src/pkg/unicode/graphic.go#n...
src/pkg/unicode/graphic.go:14: pUl             // an upper-case letter.
s/pUl/pLu/

http://codereview.appspot.com/4526095/diff/13001/src/pkg/unicode/graphic.go#n...
src/pkg/unicode/graphic.go:100: // (category P*).
s/P*/P/

http://codereview.appspot.com/4526095/diff/13001/src/pkg/unicode/graphic.go#n...
src/pkg/unicode/graphic.go:113: // Z* and property Pattern_White_Space.
s/Z*/Z/

http://codereview.appspot.com/4526095/diff/13001/src/pkg/unicode/graphic.go#n...
src/pkg/unicode/graphic.go:115: // This property isn't the same as Z*;
special-case it.
s/Z*/Z/

http://codereview.appspot.com/4526095/diff/13001/src/pkg/unicode/graphic.go#n...
src/pkg/unicode/graphic.go:116: if rune <= 0xFF { // quick Latin-1 check
different from all the others
uint32(rune) < 0x100 ?
rune < 0x100 ?

[r, 2011-06-03 21:46:30]

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

unicode: add the first few property tests for printing.

The long-term goal is that %q will use IsPrint to decide
what to show natively vs. as hexadecimal.

R=rsc, r
CC=golang-dev
http://codereview.appspot.com/4526095

[peterGo, 2011-06-03 21:56:24]

http://codereview.appspot.com/4526095/diff/5002/src/pkg/unicode/graphic.go
File src/pkg/unicode/graphic.go (right):

http://codereview.appspot.com/4526095/diff/5002/src/pkg/unicode/graphic.go#ne...
src/pkg/unicode/graphic.go:1: // Copyright 2011 The Go Authors. All rights
reserved.
The file name graphic.go is no longer an accurate name, it does a lot more than
the IsGraphic function. It's more about about the Unicode basic types in Table
2-3.

http://codereview.appspot.com/4526095/diff/5002/src/pkg/unicode/graphic.go#ne...
src/pkg/unicode/graphic.go:43: // IsPrint reports whether the rune is defined as
printable by Go. Such
Whether or not the rune is actually printed or displayed depends on the glyphs
in the font.

[rsc, 2011-06-03 21:58:20]

>
http://codereview.appspot.com/4526095/diff/5002/src/pkg/unicode/graphic.go#ne...
> src/pkg/unicode/graphic.go:43: // IsPrint reports whether the rune is
> defined as printable by Go. Such
> Whether or not the rune is actually printed or displayed depends on the
> glyphs in the font.

defined as printable *by Go*

[r2, 2011-06-03 22:04:57]

On 04/06/2011, at 7:56 AM, go.peter.90@gmail.com wrote:

> 
> http://codereview.appspot.com/4526095/diff/5002/src/pkg/unicode/graphic.go
> File src/pkg/unicode/graphic.go (right):
> 
>
http://codereview.appspot.com/4526095/diff/5002/src/pkg/unicode/graphic.go#ne...
> src/pkg/unicode/graphic.go:1: // Copyright 2011 The Go Authors. All
> rights reserved.
> The file name graphic.go is no longer an accurate name, it does a lot
> more than the IsGraphic function. It's more about about the Unicode
> basic types in Table 2-3.

perhaps, but the reason it exists is to implement IsGraphic and IsPrint.

the package is still evolving. things may get rearranged.

>
http://codereview.appspot.com/4526095/diff/5002/src/pkg/unicode/graphic.go#ne...
> src/pkg/unicode/graphic.go:43: // IsPrint reports whether the rune is
> defined as printable by Go. Such
> Whether or not the rune is actually printed or displayed depends on the
> glyphs in the font.

true, but that has nothing to do with whether Go defines it as printable.

-rob