diff --git a/git/git.go b/git/git.go index 9e3fe86fe..97dafa1c7 100644 --- a/git/git.go +++ b/git/git.go @@ -366,6 +366,21 @@ func ToplevelDir() (string, error) { } +func PathFromRepoRoot() string { + showCmd, err := GitCommand("rev-parse", "--show-prefix") + if err != nil { + return "" + } + output, err := run.PrepareCmd(showCmd).Output() + if err != nil { + return "" + } + if path := firstLine(output); path != "" { + return path[:len(path)-1] + } + return "" +} + func outputLines(output []byte) []string { lines := strings.TrimSuffix(string(output), "\n") return strings.Split(lines, "\n") diff --git a/pkg/cmd/browse/browse.go b/pkg/cmd/browse/browse.go index 16d793979..8bd64940d 100644 --- a/pkg/cmd/browse/browse.go +++ b/pkg/cmd/browse/browse.go @@ -3,6 +3,7 @@ package browse import ( "fmt" "net/http" + "path/filepath" "strconv" "strings" @@ -21,10 +22,11 @@ type browser interface { } type BrowseOptions struct { - BaseRepo func() (ghrepo.Interface, error) - Browser browser - HttpClient func() (*http.Client, error) - IO *iostreams.IOStreams + BaseRepo func() (ghrepo.Interface, error) + Browser browser + HttpClient func() (*http.Client, error) + IO *iostreams.IOStreams + PathFromRepoRoot func() string SelectorArg string @@ -38,9 +40,10 @@ type BrowseOptions struct { func NewCmdBrowse(f *cmdutil.Factory, runF func(*BrowseOptions) error) *cobra.Command { opts := &BrowseOptions{ - Browser: f.Browser, - HttpClient: f.HttpClient, - IO: f.IOStreams, + Browser: f.Browser, + HttpClient: f.HttpClient, + IO: f.IOStreams, + PathFromRepoRoot: git.PathFromRepoRoot, } cmd := &cobra.Command{ @@ -158,7 +161,7 @@ func parseSection(baseRepo ghrepo.Interface, opts *BrowseOptions) (string, error return fmt.Sprintf("issues/%s", opts.SelectorArg), nil } - filePath, rangeStart, rangeEnd, err := parseFile(opts.SelectorArg) + filePath, rangeStart, rangeEnd, err := parseFile(*opts, opts.SelectorArg) if err != nil { return "", err } @@ -188,7 +191,7 @@ func parseSection(baseRepo ghrepo.Interface, opts *BrowseOptions) (string, error return fmt.Sprintf("tree/%s/%s", branchName, filePath), nil } -func parseFile(f string) (p string, start int, end int, err error) { +func parseFile(opts BrowseOptions, f string) (p string, start int, end int, err error) { parts := strings.SplitN(f, ":", 3) if len(parts) > 2 { err = fmt.Errorf("invalid file argument: %q", f) @@ -196,6 +199,14 @@ func parseFile(f string) (p string, start int, end int, err error) { } p = parts[0] + if !filepath.IsAbs(p) { + p = filepath.Clean(filepath.Join(opts.PathFromRepoRoot(), p)) + // Ensure that a path using \ can be used in a URL + p = strings.ReplaceAll(p, "\\", "/") + if p == "." || strings.HasPrefix(p, "..") { + p = "" + } + } if len(parts) < 2 { return } diff --git a/pkg/cmd/browse/browse_test.go b/pkg/cmd/browse/browse_test.go index e342cde68..489ad4e09 100644 --- a/pkg/cmd/browse/browse_test.go +++ b/pkg/cmd/browse/browse_test.go @@ -4,8 +4,10 @@ import ( "fmt" "net/http" "os" + "path/filepath" "testing" + "github.com/cli/cli/v2/git" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/httpmock" @@ -152,6 +154,7 @@ func setGitDir(t *testing.T, dir string) { } func Test_runBrowse(t *testing.T) { + s := string(os.PathSeparator) setGitDir(t, "../../../git/fixtures/simple.git") tests := []struct { name string @@ -334,6 +337,32 @@ func Test_runBrowse(t *testing.T) { wantsErr: false, expectedURL: "https://github.com/vilmibm/gh-user-status/tree/6f1a2405cace1633d89a79c74c65f22fe78f9659/main.go", }, + { + name: "relative path from browse_test.go", + opts: BrowseOptions{ + SelectorArg: filepath.Join(".", "browse_test.go"), + PathFromRepoRoot: func() string { + return "pkg/cmd/browse/" + }, + }, + baseRepo: ghrepo.New("bchadwic", "gh-graph"), + defaultBranch: "trunk", + expectedURL: "https://github.com/bchadwic/gh-graph/tree/trunk/pkg/cmd/browse/browse_test.go", + wantsErr: false, + }, + { + name: "relative path to file in parent folder from browse_test.go", + opts: BrowseOptions{ + SelectorArg: ".." + s + "pr", + PathFromRepoRoot: func() string { + return "pkg/cmd/browse/" + }, + }, + baseRepo: ghrepo.New("bchadwic", "gh-graph"), + defaultBranch: "trunk", + expectedURL: "https://github.com/bchadwic/gh-graph/tree/trunk/pkg/cmd/pr", + wantsErr: false, + }, } for _, tt := range tests { @@ -356,6 +385,9 @@ func Test_runBrowse(t *testing.T) { return &http.Client{Transport: ®}, nil } opts.Browser = &browser + if opts.PathFromRepoRoot == nil { + opts.PathFromRepoRoot = git.PathFromRepoRoot + } err := runBrowse(&opts) if tt.wantsErr { @@ -376,3 +408,70 @@ func Test_runBrowse(t *testing.T) { }) } } + +func Test_parsePathFromFileArg(t *testing.T) { + s := string(os.PathSeparator) + tests := []struct { + name string + fileArg string + expectedPath string + }{ + { + name: "go to parent folder", + fileArg: ".." + s, + expectedPath: "pkg/cmd", + }, + { + name: "current folder", + fileArg: ".", + expectedPath: "pkg/cmd/browse", + }, + { + name: "current folder (alternative)", + fileArg: "." + s, + expectedPath: "pkg/cmd/browse", + }, + { + name: "file that starts with '.'", + fileArg: ".gitignore", + expectedPath: "pkg/cmd/browse/.gitignore", + }, + { + name: "file in current folder", + fileArg: filepath.Join(".", "browse.go"), + expectedPath: "pkg/cmd/browse/browse.go", + }, + { + name: "file within parent folder", + fileArg: filepath.Join("..", "browse.go"), + expectedPath: "pkg/cmd/browse.go", + }, + { + name: "file within parent folder uncleaned", + fileArg: filepath.Join("..", ".") + s + s + s + "browse.go", + expectedPath: "pkg/cmd/browse.go", + }, + { + name: "different path from root directory", + fileArg: filepath.Join("..", "..", "..", "internal/build/build.go"), + expectedPath: "internal/build/build.go", + }, + { + name: "go out of repository", + fileArg: filepath.Join("..", "..", "..", "..", "..", "..") + s + "", + expectedPath: "", + }, + { + name: "go to root of repository", + fileArg: filepath.Join("..", "..", "..") + s + "", + expectedPath: "", + }, + } + for _, tt := range tests { + path, _, _, _ := parseFile(BrowseOptions{ + PathFromRepoRoot: func() string { + return "pkg/cmd/browse/" + }}, tt.fileArg) + assert.Equal(t, tt.expectedPath, path, tt.name) + } +}