Issue 5865056: code review 5865056: godoc: use shorter titles for tabs (Closed)
|
Created:
13 years ago by gri Modified:
13 years ago Reviewers:
CC:
golang-dev, bradfitz Visibility:
Public. |
Descriptiongodoc: use shorter titles for tabs
In a browser with many open tabs, the tab titles become short
and uninformative because they all start with the same prefix
("Package ", "Directory ", etc.).
Permit use of shorter tab titles that start with the relevant
information first.
Fixes issue 3365.
Patch Set 1 #Patch Set 2 : diff -r 509fce3ba4e1 https://code.google.com/p/go #Patch Set 3 : diff -r 509fce3ba4e1 https://code.google.com/p/go #Patch Set 4 : diff -r 509fce3ba4e1 https://code.google.com/p/go #Patch Set 5 : diff -r 509fce3ba4e1 https://code.google.com/p/go #Patch Set 6 : diff -r 74edd315eec5 https://code.google.com/p/go #
MessagesTotal messages: 7
Hello golang-dev@googlegroups.com, I'd like you to review this change to https://code.google.com/p/go
Sign in to reply to this message.
I think servePage needs more positional parameters. On Wed, Mar 21, 2012 at 10:33 AM, <gri@golang.org> wrote: > Reviewers: golang-dev_googlegroups.com, > > Message: > Hello golang-dev@googlegroups.com, > > I'd like you to review this change to > https://code.google.com/p/go > > > Description: > godoc: use shorter titles for tabs > > In a browser with many open tabs, the tab titles become short > and uninformative because they all start with the same prefix > ("Package ", "Directory ", etc.). > > Permit use of shorter tab titles that start with the relevant > information first. > > Fixes issue 3365. > > Please review this at http://codereview.appspot.com/**5865056/<http://codereview.appspot.com/5865056/> > > Affected files: > M lib/godoc/godoc.html > M src/cmd/godoc/codewalk.go > M src/cmd/godoc/godoc.go > M src/cmd/godoc/main.go > > > Index: lib/godoc/godoc.html > ==============================**==============================**======= > --- a/lib/godoc/godoc.html > +++ b/lib/godoc/godoc.html > @@ -2,7 +2,7 @@ > <html> > <head> > <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> > -{{with .Title}} > +{{with .Tabtitle}} > <title>{{html .}} - The Go Programming Language</title> > {{else}} > <title>The Go Programming Language</title> > Index: src/cmd/godoc/codewalk.go > ==============================**==============================**======= > --- a/src/cmd/godoc/codewalk.go > +++ b/src/cmd/godoc/codewalk.go > @@ -69,7 +69,7 @@ > } > > b := applyTemplate(codewalkHTML, "codewalk", cw) > - servePage(w, "Codewalk: "+cw.Title, "", "", b) > + servePage(w, cw.Title, "Codewalk: "+cw.Title, "", "", b) > } > > // A Codewalk represents a single codewalk read from an XML file. > @@ -200,7 +200,7 @@ > } > > b := applyTemplate(codewalkdirHTML, "codewalkdir", v) > - servePage(w, "Codewalks", "", "", b) > + servePage(w, "", "Codewalks", "", "", b) > } > > // codewalkFileprint serves requests with ?fileprint=f&lo=lo&hi=hi. > Index: src/cmd/godoc/godoc.go > ==============================**==============================**======= > --- a/src/cmd/godoc/godoc.go > +++ b/src/cmd/godoc/godoc.go > @@ -546,8 +546,12 @@ > // ------------------------------**------------------------------** > ---------------- > // Generic HTML wrapper > > -func servePage(w http.ResponseWriter, title, subtitle, query string, > content []byte) { > +func servePage(w http.ResponseWriter, tabtitle, title, subtitle, query > string, content []byte) { > + if tabtitle == "" { > + tabtitle = title > + } > d := struct { > + Tabtitle string > Title string > Subtitle string > SearchBox bool > @@ -556,6 +560,7 @@ > Menu []byte > Content []byte > }{ > + tabtitle, > title, > subtitle, > *indexEnabled, > @@ -630,7 +635,7 @@ > src = buf.Bytes() > } > > - servePage(w, meta.Title, meta.Subtitle, "", src) > + servePage(w, "", meta.Title, meta.Subtitle, "", src) > } > > func applyTemplate(t *template.Template, name string, data interface{}) > []byte { > @@ -666,7 +671,7 @@ > FormatText(&buf, src, 1, pathpkg.Ext(abspath) == ".go", > r.FormValue("h"), rangeSelection(r.FormValue("s"**))) > buf.WriteString("</pre>") > > - servePage(w, title+" "+relpath, "", "", buf.Bytes()) > + servePage(w, relpath, title+" "+relpath, "", "", buf.Bytes()) > } > > func serveDirectory(w http.ResponseWriter, r *http.Request, abspath, > relpath string) { > @@ -681,7 +686,7 @@ > } > > contents := applyTemplate(dirlistHTML, "dirlistHTML", list) > - servePage(w, "Directory "+relpath, "", "", contents) > + servePage(w, relpath, "Directory "+relpath, "", "", contents) > } > > func serveFile(w http.ResponseWriter, r *http.Request) { > @@ -1073,30 +1078,33 @@ > return > } > > - var title, subtitle string > + var tabtitle, title, subtitle string > switch { > case info.PAst != nil: > - title = "Package " + info.PAst.Name.Name > + tabtitle = info.PAst.Name.Name > + title = "Package " + tabtitle > case info.PDoc != nil: > - switch { > - case info.IsPkg: > - title = "Package " + info.PDoc.Name > - case info.PDoc.Name == fakePkgName: > + if info.PDoc.Name == fakePkgName { > // assume that the directory name is the command > name > - _, pkgname := pathpkg.Split(relpath) > - title = "Command " + pkgname > - default: > - title = "Command " + info.PDoc.Name > + _, tabtitle = pathpkg.Split(relpath) > + } else { > + tabtitle = info.PDoc.Name > + } > + if info.IsPkg { > + title = "Package " + tabtitle > + } else { > + title = "Command " + tabtitle > } > default: > - title = "Directory " + info.Dirname > + tabtitle = info.Dirname > + title = "Directory " + tabtitle > if *showTimestamps { > subtitle = "Last update: " + info.DirTime.String() > } > } > > contents := applyTemplate(packageHTML, "packageHTML", info) > - servePage(w, title, subtitle, "", contents) > + servePage(w, tabtitle, title, subtitle, "", contents) > } > > // ------------------------------**------------------------------** > ---------------- > @@ -1186,7 +1194,7 @@ > } > > contents := applyTemplate(searchHTML, "searchHTML", result) > - servePage(w, title, "", query, contents) > + servePage(w, query, title, "", query, contents) > } > > // ------------------------------**------------------------------** > ---------------- > Index: src/cmd/godoc/main.go > ==============================**==============================**======= > --- a/src/cmd/godoc/main.go > +++ b/src/cmd/godoc/main.go > @@ -73,7 +73,7 @@ > func serveError(w http.ResponseWriter, r *http.Request, relpath string, > err error) { > contents := applyTemplate(errorHTML, "errorHTML", err) // err may > contain an absolute path! > w.WriteHeader(http.**StatusNotFound) > - servePage(w, "File "+relpath, "", "", contents) > + servePage(w, relpath, "File "+relpath, "", "", contents) > } > > func usage() { > > >
Sign in to reply to this message.
I agree :-) I want to change it to use a field-indexed struct (instead of setting it up inside servePage), but I'd like to do that in a next step, just to make it easier to review the changes. - gri On Wed, Mar 21, 2012 at 10:35 AM, Brad Fitzpatrick <bradfitz@golang.org> wrote: > I think servePage needs more positional parameters. > > > On Wed, Mar 21, 2012 at 10:33 AM, <gri@golang.org> wrote: >> >> Reviewers: golang-dev_googlegroups.com, >> >> Message: >> Hello golang-dev@googlegroups.com, >> >> I'd like you to review this change to >> https://code.google.com/p/go >> >> >> Description: >> godoc: use shorter titles for tabs >> >> In a browser with many open tabs, the tab titles become short >> and uninformative because they all start with the same prefix >> ("Package ", "Directory ", etc.). >> >> Permit use of shorter tab titles that start with the relevant >> information first. >> >> Fixes issue 3365. >> >> Please review this at http://codereview.appspot.com/5865056/ >> >> Affected files: >> M lib/godoc/godoc.html >> M src/cmd/godoc/codewalk.go >> M src/cmd/godoc/godoc.go >> M src/cmd/godoc/main.go >> >> >> Index: lib/godoc/godoc.html >> =================================================================== >> --- a/lib/godoc/godoc.html >> +++ b/lib/godoc/godoc.html >> @@ -2,7 +2,7 @@ >> <html> >> <head> >> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> >> -{{with .Title}} >> +{{with .Tabtitle}} >> <title>{{html .}} - The Go Programming Language</title> >> {{else}} >> <title>The Go Programming Language</title> >> Index: src/cmd/godoc/codewalk.go >> =================================================================== >> --- a/src/cmd/godoc/codewalk.go >> +++ b/src/cmd/godoc/codewalk.go >> @@ -69,7 +69,7 @@ >> } >> >> b := applyTemplate(codewalkHTML, "codewalk", cw) >> - servePage(w, "Codewalk: "+cw.Title, "", "", b) >> + servePage(w, cw.Title, "Codewalk: "+cw.Title, "", "", b) >> } >> >> // A Codewalk represents a single codewalk read from an XML file. >> @@ -200,7 +200,7 @@ >> } >> >> b := applyTemplate(codewalkdirHTML, "codewalkdir", v) >> - servePage(w, "Codewalks", "", "", b) >> + servePage(w, "", "Codewalks", "", "", b) >> } >> >> // codewalkFileprint serves requests with ?fileprint=f&lo=lo&hi=hi. >> Index: src/cmd/godoc/godoc.go >> =================================================================== >> --- a/src/cmd/godoc/godoc.go >> +++ b/src/cmd/godoc/godoc.go >> @@ -546,8 +546,12 @@ >> // >> ---------------------------------------------------------------------------- >> // Generic HTML wrapper >> >> -func servePage(w http.ResponseWriter, title, subtitle, query string, >> content []byte) { >> +func servePage(w http.ResponseWriter, tabtitle, title, subtitle, query >> string, content []byte) { >> + if tabtitle == "" { >> + tabtitle = title >> + } >> d := struct { >> + Tabtitle string >> Title string >> Subtitle string >> SearchBox bool >> @@ -556,6 +560,7 @@ >> Menu []byte >> Content []byte >> }{ >> + tabtitle, >> title, >> subtitle, >> *indexEnabled, >> @@ -630,7 +635,7 @@ >> src = buf.Bytes() >> } >> >> - servePage(w, meta.Title, meta.Subtitle, "", src) >> + servePage(w, "", meta.Title, meta.Subtitle, "", src) >> } >> >> func applyTemplate(t *template.Template, name string, data interface{}) >> []byte { >> @@ -666,7 +671,7 @@ >> FormatText(&buf, src, 1, pathpkg.Ext(abspath) == ".go", >> r.FormValue("h"), rangeSelection(r.FormValue("s"))) >> buf.WriteString("</pre>") >> >> - servePage(w, title+" "+relpath, "", "", buf.Bytes()) >> + servePage(w, relpath, title+" "+relpath, "", "", buf.Bytes()) >> } >> >> func serveDirectory(w http.ResponseWriter, r *http.Request, abspath, >> relpath string) { >> @@ -681,7 +686,7 @@ >> } >> >> contents := applyTemplate(dirlistHTML, "dirlistHTML", list) >> - servePage(w, "Directory "+relpath, "", "", contents) >> + servePage(w, relpath, "Directory "+relpath, "", "", contents) >> } >> >> func serveFile(w http.ResponseWriter, r *http.Request) { >> @@ -1073,30 +1078,33 @@ >> return >> } >> >> - var title, subtitle string >> + var tabtitle, title, subtitle string >> switch { >> case info.PAst != nil: >> - title = "Package " + info.PAst.Name.Name >> + tabtitle = info.PAst.Name.Name >> + title = "Package " + tabtitle >> case info.PDoc != nil: >> - switch { >> - case info.IsPkg: >> - title = "Package " + info.PDoc.Name >> - case info.PDoc.Name == fakePkgName: >> + if info.PDoc.Name == fakePkgName { >> // assume that the directory name is the command >> name >> - _, pkgname := pathpkg.Split(relpath) >> - title = "Command " + pkgname >> - default: >> - title = "Command " + info.PDoc.Name >> + _, tabtitle = pathpkg.Split(relpath) >> + } else { >> + tabtitle = info.PDoc.Name >> + } >> + if info.IsPkg { >> + title = "Package " + tabtitle >> + } else { >> + title = "Command " + tabtitle >> } >> default: >> - title = "Directory " + info.Dirname >> + tabtitle = info.Dirname >> + title = "Directory " + tabtitle >> if *showTimestamps { >> subtitle = "Last update: " + info.DirTime.String() >> } >> } >> >> contents := applyTemplate(packageHTML, "packageHTML", info) >> - servePage(w, title, subtitle, "", contents) >> + servePage(w, tabtitle, title, subtitle, "", contents) >> } >> >> // >> ---------------------------------------------------------------------------- >> @@ -1186,7 +1194,7 @@ >> } >> >> contents := applyTemplate(searchHTML, "searchHTML", result) >> - servePage(w, title, "", query, contents) >> + servePage(w, query, title, "", query, contents) >> } >> >> // >> ---------------------------------------------------------------------------- >> Index: src/cmd/godoc/main.go >> =================================================================== >> --- a/src/cmd/godoc/main.go >> +++ b/src/cmd/godoc/main.go >> @@ -73,7 +73,7 @@ >> func serveError(w http.ResponseWriter, r *http.Request, relpath string, >> err error) { >> contents := applyTemplate(errorHTML, "errorHTML", err) // err may >> contain an absolute path! >> w.WriteHeader(http.StatusNotFound) >> - servePage(w, "File "+relpath, "", "", contents) >> + servePage(w, relpath, "File "+relpath, "", "", contents) >> } >> >> func usage() { >> >> >
Sign in to reply to this message.
I patched this in and am clicking around. Overall, very nice. Clicking "Packages" at the top, though, results in a tabtitle of "/src/pkg - The Go Pr..." which is a bit weird. I'd expect something like "Packages - The Go Pr..." On Wed, Mar 21, 2012 at 10:33 AM, <gri@golang.org> wrote: > Reviewers: golang-dev_googlegroups.com, > > Message: > Hello golang-dev@googlegroups.com, > > I'd like you to review this change to > https://code.google.com/p/go > > > Description: > godoc: use shorter titles for tabs > > In a browser with many open tabs, the tab titles become short > and uninformative because they all start with the same prefix > ("Package ", "Directory ", etc.). > > Permit use of shorter tab titles that start with the relevant > information first. > > Fixes issue 3365. > > Please review this at http://codereview.appspot.com/**5865056/ > > Affected files: > M lib/godoc/godoc.html > M src/cmd/godoc/codewalk.go > M src/cmd/godoc/godoc.go > M src/cmd/godoc/main.go > > > Index: lib/godoc/godoc.html > ==============================**==============================**======= > --- a/lib/godoc/godoc.html > +++ b/lib/godoc/godoc.html > @@ -2,7 +2,7 @@ > <html> > <head> > <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> > -{{with .Title}} > +{{with .Tabtitle}} > <title>{{html .}} - The Go Programming Language</title> > {{else}} > <title>The Go Programming Language</title> > Index: src/cmd/godoc/codewalk.go > ==============================**==============================**======= > --- a/src/cmd/godoc/codewalk.go > +++ b/src/cmd/godoc/codewalk.go > @@ -69,7 +69,7 @@ > } > > b := applyTemplate(codewalkHTML, "codewalk", cw) > - servePage(w, "Codewalk: "+cw.Title, "", "", b) > + servePage(w, cw.Title, "Codewalk: "+cw.Title, "", "", b) > } > > // A Codewalk represents a single codewalk read from an XML file. > @@ -200,7 +200,7 @@ > } > > b := applyTemplate(codewalkdirHTML, "codewalkdir", v) > - servePage(w, "Codewalks", "", "", b) > + servePage(w, "", "Codewalks", "", "", b) > } > > // codewalkFileprint serves requests with ?fileprint=f&lo=lo&hi=hi. > Index: src/cmd/godoc/godoc.go > ==============================**==============================**======= > --- a/src/cmd/godoc/godoc.go > +++ b/src/cmd/godoc/godoc.go > @@ -546,8 +546,12 @@ > // ------------------------------**------------------------------** > ---------------- > // Generic HTML wrapper > > -func servePage(w http.ResponseWriter, title, subtitle, query string, > content []byte) { > +func servePage(w http.ResponseWriter, tabtitle, title, subtitle, query > string, content []byte) { > + if tabtitle == "" { > + tabtitle = title > + } > d := struct { > + Tabtitle string > Title string > Subtitle string > SearchBox bool > @@ -556,6 +560,7 @@ > Menu []byte > Content []byte > }{ > + tabtitle, > title, > subtitle, > *indexEnabled, > @@ -630,7 +635,7 @@ > src = buf.Bytes() > } > > - servePage(w, meta.Title, meta.Subtitle, "", src) > + servePage(w, "", meta.Title, meta.Subtitle, "", src) > } > > func applyTemplate(t *template.Template, name string, data interface{}) > []byte { > @@ -666,7 +671,7 @@ > FormatText(&buf, src, 1, pathpkg.Ext(abspath) == ".go", > r.FormValue("h"), rangeSelection(r.FormValue("s"**))) > buf.WriteString("</pre>") > > - servePage(w, title+" "+relpath, "", "", buf.Bytes()) > + servePage(w, relpath, title+" "+relpath, "", "", buf.Bytes()) > } > > func serveDirectory(w http.ResponseWriter, r *http.Request, abspath, > relpath string) { > @@ -681,7 +686,7 @@ > } > > contents := applyTemplate(dirlistHTML, "dirlistHTML", list) > - servePage(w, "Directory "+relpath, "", "", contents) > + servePage(w, relpath, "Directory "+relpath, "", "", contents) > } > > func serveFile(w http.ResponseWriter, r *http.Request) { > @@ -1073,30 +1078,33 @@ > return > } > > - var title, subtitle string > + var tabtitle, title, subtitle string > switch { > case info.PAst != nil: > - title = "Package " + info.PAst.Name.Name > + tabtitle = info.PAst.Name.Name > + title = "Package " + tabtitle > case info.PDoc != nil: > - switch { > - case info.IsPkg: > - title = "Package " + info.PDoc.Name > - case info.PDoc.Name == fakePkgName: > + if info.PDoc.Name == fakePkgName { > // assume that the directory name is the command > name > - _, pkgname := pathpkg.Split(relpath) > - title = "Command " + pkgname > - default: > - title = "Command " + info.PDoc.Name > + _, tabtitle = pathpkg.Split(relpath) > + } else { > + tabtitle = info.PDoc.Name > + } > + if info.IsPkg { > + title = "Package " + tabtitle > + } else { > + title = "Command " + tabtitle > } > default: > - title = "Directory " + info.Dirname > + tabtitle = info.Dirname > + title = "Directory " + tabtitle > if *showTimestamps { > subtitle = "Last update: " + info.DirTime.String() > } > } > > contents := applyTemplate(packageHTML, "packageHTML", info) > - servePage(w, title, subtitle, "", contents) > + servePage(w, tabtitle, title, subtitle, "", contents) > } > > // ------------------------------**------------------------------** > ---------------- > @@ -1186,7 +1194,7 @@ > } > > contents := applyTemplate(searchHTML, "searchHTML", result) > - servePage(w, title, "", query, contents) > + servePage(w, query, title, "", query, contents) > } > > // ------------------------------**------------------------------** > ---------------- > Index: src/cmd/godoc/main.go > ==============================**==============================**======= > --- a/src/cmd/godoc/main.go > +++ b/src/cmd/godoc/main.go > @@ -73,7 +73,7 @@ > func serveError(w http.ResponseWriter, r *http.Request, relpath string, > err error) { > contents := applyTemplate(errorHTML, "errorHTML", err) // err may > contain an absolute path! > w.WriteHeader(http.**StatusNotFound) > - servePage(w, "File "+relpath, "", "", contents) > + servePage(w, relpath, "File "+relpath, "", "", contents) > } > > func usage() { > > >
Sign in to reply to this message.
PTAL. Special-cased top-level package and command pages. Not sure it's worth it. - gri On Wed, Mar 21, 2012 at 10:53 AM, Brad Fitzpatrick <bradfitz@golang.org> wrote: > I patched this in and am clicking around. Overall, very nice. > > Clicking "Packages" at the top, though, results in a tabtitle of "/src/pkg - > The Go Pr..." which is a bit weird. I'd expect something like "Packages - > The Go Pr..." > > > On Wed, Mar 21, 2012 at 10:33 AM, <gri@golang.org> wrote: >> >> Reviewers: golang-dev_googlegroups.com, >> >> Message: >> Hello golang-dev@googlegroups.com, >> >> I'd like you to review this change to >> https://code.google.com/p/go >> >> >> Description: >> godoc: use shorter titles for tabs >> >> In a browser with many open tabs, the tab titles become short >> and uninformative because they all start with the same prefix >> ("Package ", "Directory ", etc.). >> >> Permit use of shorter tab titles that start with the relevant >> information first. >> >> Fixes issue 3365. >> >> Please review this at http://codereview.appspot.com/5865056/ >> >> Affected files: >> M lib/godoc/godoc.html >> M src/cmd/godoc/codewalk.go >> M src/cmd/godoc/godoc.go >> M src/cmd/godoc/main.go >> >> >> Index: lib/godoc/godoc.html >> =================================================================== >> --- a/lib/godoc/godoc.html >> +++ b/lib/godoc/godoc.html >> @@ -2,7 +2,7 @@ >> <html> >> <head> >> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> >> -{{with .Title}} >> +{{with .Tabtitle}} >> <title>{{html .}} - The Go Programming Language</title> >> {{else}} >> <title>The Go Programming Language</title> >> Index: src/cmd/godoc/codewalk.go >> =================================================================== >> --- a/src/cmd/godoc/codewalk.go >> +++ b/src/cmd/godoc/codewalk.go >> @@ -69,7 +69,7 @@ >> } >> >> b := applyTemplate(codewalkHTML, "codewalk", cw) >> - servePage(w, "Codewalk: "+cw.Title, "", "", b) >> + servePage(w, cw.Title, "Codewalk: "+cw.Title, "", "", b) >> } >> >> // A Codewalk represents a single codewalk read from an XML file. >> @@ -200,7 +200,7 @@ >> } >> >> b := applyTemplate(codewalkdirHTML, "codewalkdir", v) >> - servePage(w, "Codewalks", "", "", b) >> + servePage(w, "", "Codewalks", "", "", b) >> } >> >> // codewalkFileprint serves requests with ?fileprint=f&lo=lo&hi=hi. >> Index: src/cmd/godoc/godoc.go >> =================================================================== >> --- a/src/cmd/godoc/godoc.go >> +++ b/src/cmd/godoc/godoc.go >> @@ -546,8 +546,12 @@ >> // >> ---------------------------------------------------------------------------- >> // Generic HTML wrapper >> >> -func servePage(w http.ResponseWriter, title, subtitle, query string, >> content []byte) { >> +func servePage(w http.ResponseWriter, tabtitle, title, subtitle, query >> string, content []byte) { >> + if tabtitle == "" { >> + tabtitle = title >> + } >> d := struct { >> + Tabtitle string >> Title string >> Subtitle string >> SearchBox bool >> @@ -556,6 +560,7 @@ >> Menu []byte >> Content []byte >> }{ >> + tabtitle, >> title, >> subtitle, >> *indexEnabled, >> @@ -630,7 +635,7 @@ >> src = buf.Bytes() >> } >> >> - servePage(w, meta.Title, meta.Subtitle, "", src) >> + servePage(w, "", meta.Title, meta.Subtitle, "", src) >> } >> >> func applyTemplate(t *template.Template, name string, data interface{}) >> []byte { >> @@ -666,7 +671,7 @@ >> FormatText(&buf, src, 1, pathpkg.Ext(abspath) == ".go", >> r.FormValue("h"), rangeSelection(r.FormValue("s"))) >> buf.WriteString("</pre>") >> >> - servePage(w, title+" "+relpath, "", "", buf.Bytes()) >> + servePage(w, relpath, title+" "+relpath, "", "", buf.Bytes()) >> } >> >> func serveDirectory(w http.ResponseWriter, r *http.Request, abspath, >> relpath string) { >> @@ -681,7 +686,7 @@ >> } >> >> contents := applyTemplate(dirlistHTML, "dirlistHTML", list) >> - servePage(w, "Directory "+relpath, "", "", contents) >> + servePage(w, relpath, "Directory "+relpath, "", "", contents) >> } >> >> func serveFile(w http.ResponseWriter, r *http.Request) { >> @@ -1073,30 +1078,33 @@ >> return >> } >> >> - var title, subtitle string >> + var tabtitle, title, subtitle string >> switch { >> case info.PAst != nil: >> - title = "Package " + info.PAst.Name.Name >> + tabtitle = info.PAst.Name.Name >> + title = "Package " + tabtitle >> case info.PDoc != nil: >> - switch { >> - case info.IsPkg: >> - title = "Package " + info.PDoc.Name >> - case info.PDoc.Name == fakePkgName: >> + if info.PDoc.Name == fakePkgName { >> // assume that the directory name is the command >> name >> - _, pkgname := pathpkg.Split(relpath) >> - title = "Command " + pkgname >> - default: >> - title = "Command " + info.PDoc.Name >> + _, tabtitle = pathpkg.Split(relpath) >> + } else { >> + tabtitle = info.PDoc.Name >> + } >> + if info.IsPkg { >> + title = "Package " + tabtitle >> + } else { >> + title = "Command " + tabtitle >> } >> default: >> - title = "Directory " + info.Dirname >> + tabtitle = info.Dirname >> + title = "Directory " + tabtitle >> if *showTimestamps { >> subtitle = "Last update: " + info.DirTime.String() >> } >> } >> >> contents := applyTemplate(packageHTML, "packageHTML", info) >> - servePage(w, title, subtitle, "", contents) >> + servePage(w, tabtitle, title, subtitle, "", contents) >> } >> >> // >> ---------------------------------------------------------------------------- >> @@ -1186,7 +1194,7 @@ >> } >> >> contents := applyTemplate(searchHTML, "searchHTML", result) >> - servePage(w, title, "", query, contents) >> + servePage(w, query, title, "", query, contents) >> } >> >> // >> ---------------------------------------------------------------------------- >> Index: src/cmd/godoc/main.go >> =================================================================== >> --- a/src/cmd/godoc/main.go >> +++ b/src/cmd/godoc/main.go >> @@ -73,7 +73,7 @@ >> func serveError(w http.ResponseWriter, r *http.Request, relpath string, >> err error) { >> contents := applyTemplate(errorHTML, "errorHTML", err) // err may >> contain an absolute path! >> w.WriteHeader(http.StatusNotFound) >> - servePage(w, "File "+relpath, "", "", contents) >> + servePage(w, relpath, "File "+relpath, "", "", contents) >> } >> >> func usage() { >> >> >
Sign in to reply to this message.
LGTM Or just "Packages" and "Commands". Either way. On Wed, Mar 21, 2012 at 11:05 AM, Robert Griesemer <gri@golang.org> wrote: > PTAL. > > Special-cased top-level package and command pages. Not sure it's worth it. > - gri > > On Wed, Mar 21, 2012 at 10:53 AM, Brad Fitzpatrick <bradfitz@golang.org> > wrote: > > I patched this in and am clicking around. Overall, very nice. > > > > Clicking "Packages" at the top, though, results in a tabtitle of > "/src/pkg - > > The Go Pr..." which is a bit weird. I'd expect something like "Packages > - > > The Go Pr..." > > > > > > On Wed, Mar 21, 2012 at 10:33 AM, <gri@golang.org> wrote: > >> > >> Reviewers: golang-dev_googlegroups.com, > >> > >> Message: > >> Hello golang-dev@googlegroups.com, > >> > >> I'd like you to review this change to > >> https://code.google.com/p/go > >> > >> > >> Description: > >> godoc: use shorter titles for tabs > >> > >> In a browser with many open tabs, the tab titles become short > >> and uninformative because they all start with the same prefix > >> ("Package ", "Directory ", etc.). > >> > >> Permit use of shorter tab titles that start with the relevant > >> information first. > >> > >> Fixes issue 3365. > >> > >> Please review this at http://codereview.appspot.com/5865056/ > >> > >> Affected files: > >> M lib/godoc/godoc.html > >> M src/cmd/godoc/codewalk.go > >> M src/cmd/godoc/godoc.go > >> M src/cmd/godoc/main.go > >> > >> > >> Index: lib/godoc/godoc.html > >> =================================================================== > >> --- a/lib/godoc/godoc.html > >> +++ b/lib/godoc/godoc.html > >> @@ -2,7 +2,7 @@ > >> <html> > >> <head> > >> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> > >> -{{with .Title}} > >> +{{with .Tabtitle}} > >> <title>{{html .}} - The Go Programming Language</title> > >> {{else}} > >> <title>The Go Programming Language</title> > >> Index: src/cmd/godoc/codewalk.go > >> =================================================================== > >> --- a/src/cmd/godoc/codewalk.go > >> +++ b/src/cmd/godoc/codewalk.go > >> @@ -69,7 +69,7 @@ > >> } > >> > >> b := applyTemplate(codewalkHTML, "codewalk", cw) > >> - servePage(w, "Codewalk: "+cw.Title, "", "", b) > >> + servePage(w, cw.Title, "Codewalk: "+cw.Title, "", "", b) > >> } > >> > >> // A Codewalk represents a single codewalk read from an XML file. > >> @@ -200,7 +200,7 @@ > >> } > >> > >> b := applyTemplate(codewalkdirHTML, "codewalkdir", v) > >> - servePage(w, "Codewalks", "", "", b) > >> + servePage(w, "", "Codewalks", "", "", b) > >> } > >> > >> // codewalkFileprint serves requests with ?fileprint=f&lo=lo&hi=hi. > >> Index: src/cmd/godoc/godoc.go > >> =================================================================== > >> --- a/src/cmd/godoc/godoc.go > >> +++ b/src/cmd/godoc/godoc.go > >> @@ -546,8 +546,12 @@ > >> // > >> > ---------------------------------------------------------------------------- > >> // Generic HTML wrapper > >> > >> -func servePage(w http.ResponseWriter, title, subtitle, query string, > >> content []byte) { > >> +func servePage(w http.ResponseWriter, tabtitle, title, subtitle, query > >> string, content []byte) { > >> + if tabtitle == "" { > >> + tabtitle = title > >> + } > >> d := struct { > >> + Tabtitle string > >> Title string > >> Subtitle string > >> SearchBox bool > >> @@ -556,6 +560,7 @@ > >> Menu []byte > >> Content []byte > >> }{ > >> + tabtitle, > >> title, > >> subtitle, > >> *indexEnabled, > >> @@ -630,7 +635,7 @@ > >> src = buf.Bytes() > >> } > >> > >> - servePage(w, meta.Title, meta.Subtitle, "", src) > >> + servePage(w, "", meta.Title, meta.Subtitle, "", src) > >> } > >> > >> func applyTemplate(t *template.Template, name string, data interface{}) > >> []byte { > >> @@ -666,7 +671,7 @@ > >> FormatText(&buf, src, 1, pathpkg.Ext(abspath) == ".go", > >> r.FormValue("h"), rangeSelection(r.FormValue("s"))) > >> buf.WriteString("</pre>") > >> > >> - servePage(w, title+" "+relpath, "", "", buf.Bytes()) > >> + servePage(w, relpath, title+" "+relpath, "", "", buf.Bytes()) > >> } > >> > >> func serveDirectory(w http.ResponseWriter, r *http.Request, abspath, > >> relpath string) { > >> @@ -681,7 +686,7 @@ > >> } > >> > >> contents := applyTemplate(dirlistHTML, "dirlistHTML", list) > >> - servePage(w, "Directory "+relpath, "", "", contents) > >> + servePage(w, relpath, "Directory "+relpath, "", "", contents) > >> } > >> > >> func serveFile(w http.ResponseWriter, r *http.Request) { > >> @@ -1073,30 +1078,33 @@ > >> return > >> } > >> > >> - var title, subtitle string > >> + var tabtitle, title, subtitle string > >> switch { > >> case info.PAst != nil: > >> - title = "Package " + info.PAst.Name.Name > >> + tabtitle = info.PAst.Name.Name > >> + title = "Package " + tabtitle > >> case info.PDoc != nil: > >> - switch { > >> - case info.IsPkg: > >> - title = "Package " + info.PDoc.Name > >> - case info.PDoc.Name == fakePkgName: > >> + if info.PDoc.Name == fakePkgName { > >> // assume that the directory name is the command > >> name > >> - _, pkgname := pathpkg.Split(relpath) > >> - title = "Command " + pkgname > >> - default: > >> - title = "Command " + info.PDoc.Name > >> + _, tabtitle = pathpkg.Split(relpath) > >> + } else { > >> + tabtitle = info.PDoc.Name > >> + } > >> + if info.IsPkg { > >> + title = "Package " + tabtitle > >> + } else { > >> + title = "Command " + tabtitle > >> } > >> default: > >> - title = "Directory " + info.Dirname > >> + tabtitle = info.Dirname > >> + title = "Directory " + tabtitle > >> if *showTimestamps { > >> subtitle = "Last update: " + > info.DirTime.String() > >> } > >> } > >> > >> contents := applyTemplate(packageHTML, "packageHTML", info) > >> - servePage(w, title, subtitle, "", contents) > >> + servePage(w, tabtitle, title, subtitle, "", contents) > >> } > >> > >> // > >> > ---------------------------------------------------------------------------- > >> @@ -1186,7 +1194,7 @@ > >> } > >> > >> contents := applyTemplate(searchHTML, "searchHTML", result) > >> - servePage(w, title, "", query, contents) > >> + servePage(w, query, title, "", query, contents) > >> } > >> > >> // > >> > ---------------------------------------------------------------------------- > >> Index: src/cmd/godoc/main.go > >> =================================================================== > >> --- a/src/cmd/godoc/main.go > >> +++ b/src/cmd/godoc/main.go > >> @@ -73,7 +73,7 @@ > >> func serveError(w http.ResponseWriter, r *http.Request, relpath string, > >> err error) { > >> contents := applyTemplate(errorHTML, "errorHTML", err) // err may > >> contain an absolute path! > >> w.WriteHeader(http.StatusNotFound) > >> - servePage(w, "File "+relpath, "", "", contents) > >> + servePage(w, relpath, "File "+relpath, "", "", contents) > >> } > >> > >> func usage() { > >> > >> > > >
Sign in to reply to this message.
*** Submitted as http://code.google.com/p/go/source/detail?r=f180ac52630b *** godoc: use shorter titles for tabs In a browser with many open tabs, the tab titles become short and uninformative because they all start with the same prefix ("Package ", "Directory ", etc.). Permit use of shorter tab titles that start with the relevant information first. Fixes issue 3365. R=golang-dev, bradfitz CC=golang-dev http://codereview.appspot.com/5865056
Sign in to reply to this message.
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
