From 8b3cccae50961d427273d41c8a6ad18bfa623e08 Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Wed, 11 Apr 2018 02:08:02 +0530 Subject: [PATCH] godoc/vfs: improve implementation of RootType - Removed the StandAlone and Asset root types as they were just there for other vfses to satisfy the FileSystem interface and causing unnecessary confusion. Returning just empty strings in those scenarios now to clarify that it is a dummy placeholder. - Removed the prefix "Fs" from RootType as it was unnecessary. - Using the RootType type to pass down to the html templates instead of converting to string. The templates are capable of converting to the actual string representation when comparing the value. Change-Id: Iadc039f1354ecd814eec0af1e52cdbaaeff0cc89 Reviewed-on: https://go-review.googlesource.com/106196 Reviewed-by: Brad Fitzpatrick --- godoc/dirtrees.go | 46 ++++++++++++++++----------------- godoc/static/packageroot.html | 4 +-- godoc/static/static.go | 4 +-- godoc/vfs/emptyvfs.go | 2 +- godoc/vfs/gatefs/gatefs_test.go | 4 +-- godoc/vfs/mapfs/mapfs.go | 5 +--- godoc/vfs/namespace.go | 2 +- godoc/vfs/os.go | 2 -- godoc/vfs/os_test.go | 4 +-- godoc/vfs/vfs.go | 10 +++---- godoc/vfs/zipfs/zipfs.go | 2 -- 11 files changed, 39 insertions(+), 46 deletions(-) diff --git a/godoc/dirtrees.go b/godoc/dirtrees.go index a7de71a543..e9483a0b6e 100644 --- a/godoc/dirtrees.go +++ b/godoc/dirtrees.go @@ -26,13 +26,13 @@ import ( const testdataDirName = "testdata" type Directory struct { - Depth int - Path string // directory path; includes Name - Name string // directory name - HasPkg bool // true if the directory contains at least one package - Synopsis string // package documentation, if any - FsRootType string // string representation of vfs.RootType - Dirs []*Directory // subdirectories + Depth int + Path string // directory path; includes Name + Name string // directory name + HasPkg bool // true if the directory contains at least one package + Synopsis string // package documentation, if any + RootType vfs.RootType // root type of the filesystem containing the directory + Dirs []*Directory // subdirectories } func isGoFile(fi os.FileInfo) bool { @@ -198,13 +198,13 @@ func (b *treeBuilder) newDirTree(fset *token.FileSet, path, name string, depth i } return &Directory{ - Depth: depth, - Path: path, - Name: name, - HasPkg: hasPkgFiles && show, // TODO(bradfitz): add proper Hide field? - Synopsis: synopsis, - FsRootType: string(b.c.fs.RootType(path)), - Dirs: dirs, + Depth: depth, + Path: path, + Name: name, + HasPkg: hasPkgFiles && show, // TODO(bradfitz): add proper Hide field? + Synopsis: synopsis, + RootType: b.c.fs.RootType(path), + Dirs: dirs, } } @@ -302,13 +302,13 @@ func (dir *Directory) lookup(path string) *Directory { // are useful for presenting an entry in an indented fashion. // type DirEntry struct { - Depth int // >= 0 - Height int // = DirList.MaxHeight - Depth, > 0 - Path string // directory path; includes Name, relative to DirList root - Name string // directory name - HasPkg bool // true if the directory contains at least one package - Synopsis string // package documentation, if any - FsRootType string // string representation of vfs.RootType + Depth int // >= 0 + Height int // = DirList.MaxHeight - Depth, > 0 + Path string // directory path; includes Name, relative to DirList root + Name string // directory name + HasPkg bool // true if the directory contains at least one package + Synopsis string // package documentation, if any + RootType vfs.RootType // root type of the filesystem containing the direntry } type DirList struct { @@ -320,7 +320,7 @@ type DirList struct { // the standard library or not. func hasThirdParty(list []DirEntry) bool { for _, entry := range list { - if entry.FsRootType == string(vfs.RootTypeGoPath) { + if entry.RootType == vfs.RootTypeGoPath { return true } } @@ -375,7 +375,7 @@ func (root *Directory) listing(skipRoot bool, filter func(string) bool) *DirList p.Name = d.Name p.HasPkg = d.HasPkg p.Synopsis = d.Synopsis - p.FsRootType = d.FsRootType + p.RootType = d.RootType list = append(list, p) } diff --git a/godoc/static/packageroot.html b/godoc/static/packageroot.html index 497f39a23a..e8b2feee7a 100644 --- a/godoc/static/packageroot.html +++ b/godoc/static/packageroot.html @@ -48,7 +48,7 @@ {{range .List}} - {{if eq .FsRootType "GOROOT"}} + {{if eq .RootType "GOROOT"}} {{if $.DirFlat}} {{if .HasPkg}} @@ -87,7 +87,7 @@ {{range .List}} - {{if eq .FsRootType "GOPATH"}} + {{if eq .RootType "GOPATH"}} {{if $.DirFlat}} {{if .HasPkg}} diff --git a/godoc/static/static.go b/godoc/static/static.go index ce43e24b93..3c3d85b21e 100644 --- a/godoc/static/static.go +++ b/godoc/static/static.go @@ -1951,7 +1951,7 @@ function cgAddChild(tree, ul, cgn) { {{range .List}} - {{if eq .FsRootType "GOROOT"}} + {{if eq .RootType "GOROOT"}} {{if $.DirFlat}} {{if .HasPkg}} @@ -1990,7 +1990,7 @@ function cgAddChild(tree, ul, cgn) { {{range .List}} - {{if eq .FsRootType "GOPATH"}} + {{if eq .RootType "GOPATH"}} {{if $.DirFlat}} {{if .HasPkg}} diff --git a/godoc/vfs/emptyvfs.go b/godoc/vfs/emptyvfs.go index be9a87c589..0803206c51 100644 --- a/godoc/vfs/emptyvfs.go +++ b/godoc/vfs/emptyvfs.go @@ -58,7 +58,7 @@ func (e *emptyVFS) String() string { } func (e *emptyVFS) RootType(path string) RootType { - return RootTypeStandAlone + return "" } // These functions below implement os.FileInfo for the single diff --git a/godoc/vfs/gatefs/gatefs_test.go b/godoc/vfs/gatefs/gatefs_test.go index 45f8f32408..a0156b4504 100644 --- a/godoc/vfs/gatefs/gatefs_test.go +++ b/godoc/vfs/gatefs/gatefs_test.go @@ -17,7 +17,7 @@ func TestRootType(t *testing.T) { goPath := os.Getenv("GOPATH") var expectedType vfs.RootType if goPath == "" { - expectedType = vfs.RootTypeStandAlone + expectedType = "" } else { expectedType = vfs.RootTypeGoPath } @@ -27,7 +27,7 @@ func TestRootType(t *testing.T) { }{ {runtime.GOROOT(), vfs.RootTypeGoRoot}, {goPath, expectedType}, - {"/tmp/", vfs.RootTypeStandAlone}, + {"/tmp/", ""}, } for _, item := range tests { diff --git a/godoc/vfs/mapfs/mapfs.go b/godoc/vfs/mapfs/mapfs.go index 4911a2d4ff..5de7ce72e4 100644 --- a/godoc/vfs/mapfs/mapfs.go +++ b/godoc/vfs/mapfs/mapfs.go @@ -29,11 +29,8 @@ type mapFS map[string]string func (fs mapFS) String() string { return "mapfs" } -// RootType directly returns vfs.RootTypeAsset because -// mapFs is only used to return static assets and not for -// resolving Go files. func (fs mapFS) RootType(p string) vfs.RootType { - return vfs.RootTypeAsset + return "" } func (fs mapFS) Close() error { return nil } diff --git a/godoc/vfs/namespace.go b/godoc/vfs/namespace.go index f3212ba69b..b8a1122d0c 100644 --- a/godoc/vfs/namespace.go +++ b/godoc/vfs/namespace.go @@ -392,7 +392,7 @@ func (ns NameSpace) RootType(path string) RootType { return m.fs.RootType(path) } } - return RootTypeStandAlone + return "" } // byName implements sort.Interface. diff --git a/godoc/vfs/os.go b/godoc/vfs/os.go index 9001871d94..8cc9ed1990 100644 --- a/godoc/vfs/os.go +++ b/godoc/vfs/os.go @@ -26,8 +26,6 @@ func OS(root string) FileSystem { t = RootTypeGoRoot case isGoPath(root): t = RootTypeGoPath - default: - t = RootTypeStandAlone } return osFS{rootPath: root, rootType: t} } diff --git a/godoc/vfs/os_test.go b/godoc/vfs/os_test.go index 9307424ae2..98631e0516 100644 --- a/godoc/vfs/os_test.go +++ b/godoc/vfs/os_test.go @@ -16,7 +16,7 @@ func TestRootType(t *testing.T) { goPath := os.Getenv("GOPATH") var expectedType vfs.RootType if goPath == "" { - expectedType = vfs.RootTypeStandAlone + expectedType = "" } else { expectedType = vfs.RootTypeGoPath } @@ -26,7 +26,7 @@ func TestRootType(t *testing.T) { }{ {runtime.GOROOT(), vfs.RootTypeGoRoot}, {goPath, expectedType}, - {"/tmp/", vfs.RootTypeStandAlone}, + {"/tmp/", ""}, } for _, item := range tests { diff --git a/godoc/vfs/vfs.go b/godoc/vfs/vfs.go index 72265d8961..d70526d5ac 100644 --- a/godoc/vfs/vfs.go +++ b/godoc/vfs/vfs.go @@ -14,14 +14,14 @@ import ( // RootType indicates the type of files contained within a directory. // -// The two main types are a GOROOT or a GOPATH directory. +// It is used to indicate whether a directory is the root +// of a GOROOT, a GOPATH, or neither. +// An empty string represents the case when a directory is neither. type RootType string const ( - RootTypeGoRoot RootType = "GOROOT" - RootTypeGoPath RootType = "GOPATH" - RootTypeStandAlone RootType = "StandAlone" // used by emptyvfs - RootTypeAsset RootType = "Assets" // serves static assets using mapfs + RootTypeGoRoot RootType = "GOROOT" + RootTypeGoPath RootType = "GOPATH" ) // The FileSystem interface specifies the methods godoc is using diff --git a/godoc/vfs/zipfs/zipfs.go b/godoc/vfs/zipfs/zipfs.go index 894a83770c..6d27f63624 100644 --- a/godoc/vfs/zipfs/zipfs.go +++ b/godoc/vfs/zipfs/zipfs.go @@ -91,8 +91,6 @@ func (fs *zipFS) RootType(abspath string) vfs.RootType { t = vfs.RootTypeGoRoot case isGoPath(abspath): t = vfs.RootTypeGoPath - default: - t = vfs.RootTypeStandAlone } return t }