code review 12978043: cmd/godoc: bake templates and scripts into godoc binary

cmd/godoc: bake templates and scripts into godoc binary

Add godoc/vfs/mapfs package for serving baked files.

Fixes issue 6010.

[adg, 2013-08-15 06:13:58]

Hello golang-dev@googlegroups.com,

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

[bradfitz, 2013-08-15 06:24:07]

You've seen camlistore's fileembed, right?  It generates embed files that
have similar line breaks to minimize diff sizes, and uses strings instead
of []byte and gives you an io interface to read it.




On Wed, Aug 14, 2013 at 11:13 PM, <adg@golang.org> wrote:

> Reviewers: golang-dev1,
>
> Message:
> Hello golang-dev@googlegroups.com,
>
> I'd like you to review this change to
> https://code.google.com/p/go.**tools <https://code.google.com/p/go.tools>
>
>
> Description:
> cmd/godoc: bake templates and scripts into godoc binary
>
> Add bake utility to cmd/bake.
>
> Fixes issue 6010.
>
> Please review this at
https://codereview.appspot.**com/12978043/<https://codereview.appspot.com/129...
>
> Affected files:
>   A cmd/bake/bake.go
>   A cmd/godoc/bake.sh
>   M cmd/godoc/handlers.go
>   A cmd/godoc/template.go
>
>
> --
>
> ---You received this message because you are subscribed to the Google
> Groups "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to
golang-dev+unsubscribe@**googlegroups.com<golang-dev%2Bunsubscribe@googlegrou...
> .
> For more options, visit
https://groups.google.com/**groups/opt_out<https://groups.google.com/groups/o...
> .
>

[r, 2013-08-15 06:26:46]

not sure why we need any of this.

https://codereview.appspot.com/12978043/diff/1/cmd/bake/bake.go
File cmd/bake/bake.go (right):

https://codereview.appspot.com/12978043/diff/1/cmd/bake/bake.go#newcode86
cmd/bake/bake.go:86: // Replace BOM with `+"\xEF\xBB\xBF"+`
this isn't nearly good enough. you need to worry about lots of other things.

instead, just make it a byte string and do string(bytestring) if you want a
string. avoid the UTF-8 issue altogether.

[adg, 2013-08-15 06:42:01]

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

Please take another look.

[adg, 2013-08-15 06:44:52]

On 15 August 2013 16:24, Brad Fitzpatrick <bradfitz@golang.org> wrote:

> You've seen camlistore's fileembed, right?  It generates embed files that
> have similar line breaks to minimize diff sizes, and uses strings instead
> of []byte and gives you an io interface to read it.
>

I'd forgotten about fileembed. But this shouldn't be a go installable tool,
anyway. I've made it simpler still.

[r, 2013-08-15 06:47:26]

https://codereview.appspot.com/12978043/diff/7001/cmd/godoc/bake.go
File cmd/godoc/bake.go (right):

https://codereview.appspot.com/12978043/diff/7001/cmd/godoc/bake.go#newcode27
cmd/godoc/bake.go:27: varName = flag.String("var", "bakedFiles", "Map variable
name")
s/M/m/

https://codereview.appspot.com/12978043/diff/7001/cmd/godoc/bake.go#newcode42
cmd/godoc/bake.go:42: os.Stdout.Write(src)
instead do

os.Stdout = bufio.NewWriter(os.Stdout)

then you can use Printf and stream it all nicely. don't forget to flush. 

(or create a new variable if you prefer)

https://codereview.appspot.com/12978043/diff/7001/cmd/godoc/bake.sh
File cmd/godoc/bake.sh (right):

https://codereview.appspot.com/12978043/diff/7001/cmd/godoc/bake.sh#newcode9
cmd/godoc/bake.sh:9: gofmt -w template.go

one line is fine

go run ... | gofmt > template.go

[adg, 2013-08-15 06:52:17]

https://codereview.appspot.com/12978043/diff/7001/cmd/godoc/bake.go
File cmd/godoc/bake.go (right):

https://codereview.appspot.com/12978043/diff/7001/cmd/godoc/bake.go#newcode27
cmd/godoc/bake.go:27: varName = flag.String("var", "bakedFiles", "Map variable
name")
On 2013/08/15 06:47:27, r wrote:
> s/M/m/

Done.

https://codereview.appspot.com/12978043/diff/7001/cmd/godoc/bake.go#newcode42
cmd/godoc/bake.go:42: os.Stdout.Write(src)
On 2013/08/15 06:47:27, r wrote:
> instead do
> 
> os.Stdout = bufio.NewWriter(os.Stdout)
> 
> then you can use Printf and stream it all nicely. don't forget to flush. 
> 
> (or create a new variable if you prefer)
> 

Done.

https://codereview.appspot.com/12978043/diff/7001/cmd/godoc/bake.sh
File cmd/godoc/bake.sh (right):

https://codereview.appspot.com/12978043/diff/7001/cmd/godoc/bake.sh#newcode9
cmd/godoc/bake.sh:9: gofmt -w template.go
On 2013/08/15 06:47:27, r wrote:
> 
> one line is fine
> 
> go run ... | gofmt > template.go

Done.

[r, 2013-08-15 06:55:33]

LGTM

[ahormann, 2013-08-15 08:09:10]

https://codereview.appspot.com/12978043/diff/14001/cmd/godoc/bake.go
File cmd/godoc/bake.go (right):

https://codereview.appspot.com/12978043/diff/14001/cmd/godoc/bake.go#newcode13
cmd/godoc/bake.go:13: // of foo.txt and bar.txt.
s/foo.txt/foo.html/

[adg, 2013-08-15 23:45:48]

PTAL

I forgot about the -templates flag, which is actually pretty handy. So I
introduced a mapfs type to serve the embedded data.

https://codereview.appspot.com/12978043/diff/14001/cmd/godoc/bake.go
File cmd/godoc/bake.go (right):

https://codereview.appspot.com/12978043/diff/14001/cmd/godoc/bake.go#newcode13
cmd/godoc/bake.go:13: // of foo.txt and bar.txt.
On 2013/08/15 08:09:11, ahormann wrote:
> s/foo.txt/foo.html/

Done.

[bradfitz, 2013-08-16 00:07:22]

I still don't like the template.go auto-generated file.

It will forever produce gigantic diffs too large to email and get angry
with tools in other ways (hg diff, etc).

I would really like you to do it Camlistore's fileembed way, with a string
and line breaks in the same place, which makes a small diff.  I don't care
about fileembed as a tool being goinstallable.  I'm only caring here about
the look of the auto-generated file.




On Wed, Aug 14, 2013 at 11:13 PM, <adg@golang.org> wrote:

> Reviewers: golang-dev1,
>
> Message:
> Hello golang-dev@googlegroups.com,
>
> I'd like you to review this change to
> https://code.google.com/p/go.**tools <https://code.google.com/p/go.tools>
>
>
> Description:
> cmd/godoc: bake templates and scripts into godoc binary
>
> Add bake utility to cmd/bake.
>
> Fixes issue 6010.
>
> Please review this at
https://codereview.appspot.**com/12978043/<https://codereview.appspot.com/129...
>
> Affected files:
>   A cmd/bake/bake.go
>   A cmd/godoc/bake.sh
>   M cmd/godoc/handlers.go
>   A cmd/godoc/template.go
>
>
> --
>
> ---You received this message because you are subscribed to the Google
> Groups "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to
golang-dev+unsubscribe@**googlegroups.com<golang-dev%2Bunsubscribe@googlegrou...
> .
> For more options, visit
https://groups.google.com/**groups/opt_out<https://groups.google.com/groups/o...
> .
>

[adg, 2013-08-16 00:11:59]

On 16 August 2013 10:07, Brad Fitzpatrick <bradfitz@golang.org> wrote:

> with a string and line breaks in the same place, which makes a small diff


I don't understand this.

[bradfitz, 2013-08-16 00:17:00]

On Thu, Aug 15, 2013 at 5:11 PM, Andrew Gerrand <adg@golang.org> wrote:

>
> On 16 August 2013 10:07, Brad Fitzpatrick <bradfitz@golang.org> wrote:
>
>> with a string and line breaks in the same place, which makes a small diff
>
>
> I don't understand this.
>

My diffs look like this:

https://camlistore-review.googlesource.com/#/c/360/1/server/camlistored/ui/ze...

Readable.

Yours look like one line of 5 MB of numbers.

[adg, 2013-08-16 00:19:54]

+r

The previous version of this tool stored the data as raw strings. Rob asked
me to change it to []byte, and said he wouldn't approve the change
otherwise.

I'm past caring at this point. Please let me know what you two resolve to
do here.


On 16 August 2013 10:16, Brad Fitzpatrick <bradfitz@golang.org> wrote:

> On Thu, Aug 15, 2013 at 5:11 PM, Andrew Gerrand <adg@golang.org> wrote:
>
>>
>> On 16 August 2013 10:07, Brad Fitzpatrick <bradfitz@golang.org> wrote:
>>
>>> with a string and line breaks in the same place, which makes a small diff
>>
>>
>> I don't understand this.
>>
>
> My diffs look like this:
>
>
>
https://camlistore-review.googlesource.com/#/c/360/1/server/camlistored/ui/ze...
>
> Readable.
>
> Yours look like one line of 5 MB of numbers.
>

[bradfitz, 2013-08-16 00:26:07]

My version doesn't use a raw string and only a subset of ASCII goes through
unescaped.

I like seeing clean diffs.

At least put in newlines in-between your numbers both every so often and
whenever you see a 0x0a byte, so they're aligned with human-authored html
and css.  That will also produce diffs that fit in emails, even if half the
diff is gibberish looking.




On Thu, Aug 15, 2013 at 5:19 PM, Andrew Gerrand <adg@golang.org> wrote:

> +r
>
> The previous version of this tool stored the data as raw strings. Rob
> asked me to change it to []byte, and said he wouldn't approve the change
> otherwise.
>
> I'm past caring at this point. Please let me know what you two resolve to
> do here.
>
>
> On 16 August 2013 10:16, Brad Fitzpatrick <bradfitz@golang.org> wrote:
>
>> On Thu, Aug 15, 2013 at 5:11 PM, Andrew Gerrand <adg@golang.org> wrote:
>>
>>>
>>> On 16 August 2013 10:07, Brad Fitzpatrick <bradfitz@golang.org> wrote:
>>>
>>>> with a string and line breaks in the same place, which makes a small
>>>> diff
>>>
>>>
>>> I don't understand this.
>>>
>>
>> My diffs look like this:
>>
>>
>>
https://camlistore-review.googlesource.com/#/c/360/1/server/camlistored/ui/ze...
>>
>> Readable.
>>
>> Yours look like one line of 5 MB of numbers.
>>
>
>

[r, 2013-08-16 00:39:26]

The engineering level of this makes the work on the garbage collector
seem a mere bagatelle.

Send a CL if you really care.

-rob

[rsc, 2013-08-16 00:45:59]

i haven't looked at the cl but fwiw

var x = `big string`

will compile much more efficiently than

var x = []byte{lots of hex}

[adg, 2013-08-16 00:59:15]

To address the concerns raised by Brad and Russ I have switched to using
raw strings. bake.go is still much simpler than it was in the first
revision of this CL.

Andrew

[r, 2013-08-16 00:59:39]

That's unfortunate and silly but you play the hand you're dealt.

Andrew, with apologies I withdraw all my requests to make this the
simple obvious thing, since that's clearly stupid. Complexity rules.

[r, 2013-08-16 01:02:01]

LGTM

[adg, 2013-08-16 01:44:40]

*** Submitted as
https://code.google.com/p/go/source/detail?r=2c0ddaee5449&repo=tools ***

cmd/godoc: bake templates and scripts into godoc binary

Add godoc/vfs/mapfs package for serving baked files.

Fixes issue 6010.

R=golang-dev, bradfitz, r, arnehormann, rsc
CC=golang-dev
https://codereview.appspot.com/12978043