code review 12038049: go-tour: add -article flag

go-tour: add -article flag

Fixes issue 11

[dupoxy, 2013-07-29 23:57:44]

Hello golang-dev@googlegroups.com,

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

[adg, 2013-07-30 03:35:17]

This loads the file relative to the go-tour repo root, which is not necessarily
what you want when loading a custom file.

I suggest the default be empty string and it should use filepath.Join(root,
"tour.article") if the flag is not specified. Otherwise, the flag should specify
the absolute path to the desired content file. The flag should also be named
"content".

[adg, 2013-07-30 03:35:44]

(And by "absolute path" I mean "path relative to the current directory",
obviously ;-)

[dupoxy, 2013-08-11 22:38:24]

https://codereview.appspot.com/12038049/diff/18001/gotour/local.go
File gotour/local.go (left):

https://codereview.appspot.com/12038049/diff/18001/gotour/local.go#oldcode100
gotour/local.go:100: http.Handle("/talks/", fs)
I don't get what this Handle is for ?

https://codereview.appspot.com/12038049/diff/18001/gotour/local.go
File gotour/local.go (right):

https://codereview.appspot.com/12038049/diff/18001/gotour/local.go#newcode54
gotour/local.go:54: func findRoot() (string, string, error) {
Added a second string to return, for separation of the content from the template
and the js directory.

https://codereview.appspot.com/12038049/diff/18001/gotour/local.go#newcode55
gotour/local.go:55: if *content == "" {
This if does not look ok. I guess I'm missing something (if I don't do this I
can not find a way to make the content flag work on a different path if it's set
to "tour.article" by the user).
The rest of the code in the if is a verbatim copy except for the added string
returned.

[dupoxy, 2013-08-11 22:53:09]

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

Please take another look.

[r, 2013-08-13 01:12:34]

In the CL descrption, drop the space and period on the "Fixes" line.

[dupoxy, 2013-08-13 05:45:15]

On 2013/08/13 01:12:34, r wrote:
> In the CL descrption, drop the space and period on the "Fixes" line.

Done

[dupoxy, 2013-08-13 05:49:31]

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

Please take another look.

[adg, 2013-08-15 04:37:23]

https://codereview.appspot.com/12038049/diff/28001/gotour/tour.go
File gotour/tour.go (right):

https://codereview.appspot.com/12038049/diff/28001/gotour/tour.go#newcode36
gotour/tour.go:36: source := filepath.Join(root, *content)
I didn't expect all these changes. Can't you just say:

source := filepath.Join(root, "tour.article")
if *content != "" {
  source = *content
}

??

[dupoxy, 2013-08-17 14:48:44]

https://codereview.appspot.com/12038049/diff/28001/gotour/tour.go
File gotour/tour.go (right):

https://codereview.appspot.com/12038049/diff/28001/gotour/tour.go#newcode36
gotour/tour.go:36: source := filepath.Join(root, *content)
On 2013/08/15 04:37:24, adg wrote:
> I didn't expect all these changes. Can't you just say:
Yes I can !!
> source := filepath.Join(root, "tour.article")
> if *content != "" {
>   source = *content
> }
> 
> ??
Done
Sorry for the delay. I was away.

[dupoxy, 2013-08-17 14:51:01]

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

Please take another look.

[adg, 2013-08-19 00:44:15]

LGTM

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

*** Submitted as https://code.google.com/p/go-tour/source/detail?r=002f27949682
***

go-tour: add -content flag

Fixes issue 11

R=golang-dev, adg, r
CC=golang-dev
https://codereview.appspot.com/12038049

Committer: Andrew Gerrand <adg@golang.org>