code review 3699041: godoc: support for regular expression full text search

godoc: support for regular expression full text search

Missing:
- indexing of non-.go files
- fine tuning of result presentation

[gri, 2010-12-21 02:35:34]

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

I'd like you to review this change.

[gri, 2010-12-23 23:12:06]

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

Please take another look.

[gri, 2011-01-07 01:33:28]

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

I'd like you to review this change.

[r2, 2011-01-08 23:57:09]

this change does a lot more than add regexp.  that's fine, but please update the
CL description to mention the new formatting code and other refactorings

[r, 2011-01-09 00:08:54]

LGTM but please update the CL description as requested in mail

the formatter is nice code.

http://codereview.appspot.com/3699041/diff/55001/src/cmd/godoc/doc.go
File src/cmd/godoc/doc.go (right):

http://codereview.appspot.com/3699041/diff/55001/src/cmd/godoc/doc.go#newcode53
src/cmd/godoc/doc.go:53: interpret queries as regular expressions for full text
search
instead of this, i would consider either:
1) two flags, -q for plain text -r for regular expressions
2) making -q be regexp always

i dislike the effect of this flag

http://codereview.appspot.com/3699041/diff/55001/src/cmd/godoc/format.go
File src/cmd/godoc/format.go (right):

http://codereview.appspot.com/3699041/diff/55001/src/cmd/godoc/format.go#newc...
src/cmd/godoc/format.go:1: // Copyright 2010 The Go Authors. All rights
reserved.
2011?

http://codereview.appspot.com/3699041/diff/55001/src/cmd/godoc/format.go#newc...
src/cmd/godoc/format.go:302: //	- each occurence of the regular expression
pattern gets the "highlight"
s/r/rr/

http://codereview.appspot.com/3699041/diff/55001/src/cmd/godoc/format.go#newc...
src/cmd/godoc/format.go:307: // HTML span classes are specified in the startTags
variable.
this comment should appear or at least be tied to the definition of the bits in
startTags; i spent a while decoding them

http://codereview.appspot.com/3699041/diff/55001/src/cmd/godoc/index.go
File src/cmd/godoc/index.go (right):

http://codereview.appspot.com/3699041/diff/55001/src/cmd/godoc/index.go#newco...
src/cmd/godoc/index.go:906: // A FileLines value specifies a file and line
numbers within that file.
s/numbers/number/

http://codereview.appspot.com/3699041/diff/55001/src/cmd/godoc/index.go#newco...
src/cmd/godoc/index.go:913: // LookupRegexp returns the number and list of
matches where a regular
"number and list"?

http://codereview.appspot.com/3699041/diff/55001/src/cmd/godoc/index.go#newco...
src/cmd/godoc/index.go:925: // This is unlikely, buf after eliminating them we
may end up with
what does "This" refer to?  "Such a span is unlikely"?

http://codereview.appspot.com/3699041/diff/55001/src/cmd/godoc/index.go#newco...
src/cmd/godoc/index.go:929: // returned fewer than that then there cannot be
more).
s/then //
it's not wrong but it's unnecessary and makes the sentence harder to read

[gri, 2011-01-10 23:15:01]

http://codereview.appspot.com/3699041/diff/55001/src/cmd/godoc/doc.go
File src/cmd/godoc/doc.go (right):

http://codereview.appspot.com/3699041/diff/55001/src/cmd/godoc/doc.go#newcode53
src/cmd/godoc/doc.go:53: interpret queries as regular expressions for full text
search
On 2011/01/09 00:08:55, r wrote:
> instead of this, i would consider either:
> 1) two flags, -q for plain text -r for regular expressions
> 2) making -q be regexp always
> 
> i dislike the effect of this flag

Use -fulltext instead, make it regular expression search always.

http://codereview.appspot.com/3699041/diff/55001/src/cmd/godoc/format.go
File src/cmd/godoc/format.go (right):

http://codereview.appspot.com/3699041/diff/55001/src/cmd/godoc/format.go#newc...
src/cmd/godoc/format.go:1: // Copyright 2010 The Go Authors. All rights
reserved.
On 2011/01/09 00:08:55, r wrote:
> 2011?

Done.

http://codereview.appspot.com/3699041/diff/55001/src/cmd/godoc/format.go#newc...
src/cmd/godoc/format.go:302: //	- each occurence of the regular expression
pattern gets the "highlight"
On 2011/01/09 00:08:55, r wrote:
> s/r/rr/

Done.

http://codereview.appspot.com/3699041/diff/55001/src/cmd/godoc/format.go#newc...
src/cmd/godoc/format.go:307: // HTML span classes are specified in the startTags
variable.
On 2011/01/09 00:08:55, r wrote:
> this comment should appear or at least be tied to the definition of the bits
in
> startTags; i spent a while decoding them

Done.

http://codereview.appspot.com/3699041/diff/55001/src/cmd/godoc/index.go
File src/cmd/godoc/index.go (right):

http://codereview.appspot.com/3699041/diff/55001/src/cmd/godoc/index.go#newco...
src/cmd/godoc/index.go:906: // A FileLines value specifies a file and line
numbers within that file.
On 2011/01/09 00:08:55, r wrote:
> s/numbers/number/

It's a list of numbers.

http://codereview.appspot.com/3699041/diff/55001/src/cmd/godoc/index.go#newco...
src/cmd/godoc/index.go:913: // LookupRegexp returns the number and list of
matches where a regular
On 2011/01/09 00:08:55, r wrote:
> "number and list"?

Done.

http://codereview.appspot.com/3699041/diff/55001/src/cmd/godoc/index.go#newco...
src/cmd/godoc/index.go:925: // This is unlikely, buf after eliminating them we
may end up with
On 2011/01/09 00:08:55, r wrote:
> what does "This" refer to?  "Such a span is unlikely"?

Done.

http://codereview.appspot.com/3699041/diff/55001/src/cmd/godoc/index.go#newco...
src/cmd/godoc/index.go:929: // returned fewer than that then there cannot be
more).
On 2011/01/09 00:08:55, r wrote:
> s/then //
> it's not wrong but it's unnecessary and makes the sentence harder to read

Done.

[gri, 2011-01-10 23:34:35]

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

godoc: support for regular expression full text search

Regular expressions may now be used in conjuction with full text
search. Godoc will show the first 10000 occurences in the source
code and highlight the respective text segments.

- added new flag -testDir to specify a small directory for testing
  (fast index creation; default = "")

- use new FormatText function to format text and Go source
  code in HTML, supporting multiple kinds of text selections
  simulatenously); this replaces the uses of go/printer
  Stylers

- for now removed currently unused mechanism for identifier-
  specific JS popups (will come back in some form once we
  have type or other useful information)

- various typo fixes and minor cleanups throughout

Missing:
- indexing of non-.go files

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

Committer: Robert Griesemer <gri@golang.org>