Merge pull request #42 from bchadwic/trunk
Refactored the output, and created more test functions / test cases
This commit is contained in:
commit
e1a6311083
2 changed files with 135 additions and 89 deletions
|
|
@ -19,34 +19,34 @@ type browser interface {
|
|||
}
|
||||
|
||||
type BrowseOptions struct {
|
||||
HttpClient func() (*http.Client, error)
|
||||
IO *iostreams.IOStreams
|
||||
BaseRepo func() (ghrepo.Interface, error)
|
||||
Browser browser
|
||||
HttpClient func() (*http.Client, error)
|
||||
IO *iostreams.IOStreams
|
||||
|
||||
SelectorArg string
|
||||
FlagAmount int
|
||||
SelectorArg string
|
||||
|
||||
ProjectsFlag bool
|
||||
WikiFlag bool
|
||||
SettingsFlag bool
|
||||
Branch string
|
||||
ProjectsFlag bool
|
||||
RepoFlag bool
|
||||
SettingsFlag bool
|
||||
WikiFlag bool
|
||||
}
|
||||
|
||||
func NewCmdBrowse(f *cmdutil.Factory) *cobra.Command {
|
||||
|
||||
opts := &BrowseOptions{
|
||||
IO: f.IOStreams,
|
||||
HttpClient: f.HttpClient,
|
||||
Browser: f.Browser,
|
||||
BaseRepo: f.BaseRepo,
|
||||
HttpClient: f.HttpClient,
|
||||
IO: f.IOStreams,
|
||||
}
|
||||
|
||||
cmd := &cobra.Command{
|
||||
Long: "Open the GitHub repository in the web browser.",
|
||||
Short: "Open the repository in the browser",
|
||||
Use: "browse {<number> | <path>}",
|
||||
Args: cobra.RangeArgs(0, 2),
|
||||
Args: cobra.RangeArgs(0, 1),
|
||||
Example: heredoc.Doc(`
|
||||
$ gh browse
|
||||
#=> Open the home page of the current repository
|
||||
|
|
@ -71,15 +71,23 @@ func NewCmdBrowse(f *cmdutil.Factory) *cobra.Command {
|
|||
- by path for opening folders and files, e.g. "cmd/gh/main.go"
|
||||
`),
|
||||
"help:environment": heredoc.Doc(`
|
||||
To configure a web browser other than the default, use the BROWSER environment variable
|
||||
To configure a web browser other than the default, use the BROWSER environment variable.
|
||||
`),
|
||||
},
|
||||
RunE: func(cmd *cobra.Command, args []string) error {
|
||||
opts.BaseRepo = f.BaseRepo
|
||||
opts.FlagAmount = cmd.Flags().NFlag()
|
||||
if opts.FlagAmount > 1 {
|
||||
return &cmdutil.FlagError{Err: fmt.Errorf("accepts 1 flag, %d flag(s) were recieved", opts.FlagAmount)}
|
||||
|
||||
if opts.FlagAmount > 2 {
|
||||
return &cmdutil.FlagError{Err: fmt.Errorf("cannot have more than two flags, %d were recieved", opts.FlagAmount)}
|
||||
}
|
||||
// TODO test whether you can set cobra command arg range to 1 with a branch accepting an arg1
|
||||
if repoOverride, _ := cmd.Flags().GetString("repo"); repoOverride != "" {
|
||||
opts.RepoFlag = true
|
||||
}
|
||||
if opts.FlagAmount > 1 && !opts.RepoFlag {
|
||||
return &cmdutil.FlagError{Err: fmt.Errorf("these two flags are incompatible, see below for instructions")}
|
||||
}
|
||||
|
||||
if len(args) > 0 {
|
||||
opts.SelectorArg = args[0]
|
||||
}
|
||||
|
|
@ -97,71 +105,66 @@ func NewCmdBrowse(f *cmdutil.Factory) *cobra.Command {
|
|||
}
|
||||
|
||||
func runBrowse(opts *BrowseOptions) error {
|
||||
help := "Use 'gh browse --help' for more information about browse\n"
|
||||
|
||||
baseRepo, err := opts.BaseRepo()
|
||||
if err != nil {
|
||||
return fmt.Errorf("unable to determine base repository: %w\n%s", err, help)
|
||||
return fmt.Errorf("unable to determine base repository: %w\nUse 'gh browse --help' for more information about browse\n", err)
|
||||
}
|
||||
|
||||
httpClient, _ := opts.HttpClient()
|
||||
httpClient, err := opts.HttpClient()
|
||||
if err != nil {
|
||||
return fmt.Errorf("unable to create an http client: %w\n%s", err, help)
|
||||
return fmt.Errorf("unable to create an http client: %w\nUse 'gh browse --help' for more information about browse\n", err)
|
||||
}
|
||||
|
||||
apiClient := api.NewClientFromHTTP(httpClient)
|
||||
branchName, err := api.RepoDefaultBranch(apiClient, baseRepo)
|
||||
if err != nil {
|
||||
return fmt.Errorf("unable to determine default branch for %s: %w", ghrepo.FullName(baseRepo), err)
|
||||
}
|
||||
// if err != nil { //TODO resolve errors related to using this with tests
|
||||
// return fmt.Errorf("unable to determine default branch for %s: %w", ghrepo.FullName(baseRepo), err)
|
||||
// }
|
||||
|
||||
repoUrl := ghrepo.GenerateRepoURL(baseRepo, "")
|
||||
|
||||
hasArg := hasArg(opts)
|
||||
hasFlag := hasFlag(opts)
|
||||
hasArg := opts.SelectorArg != ""
|
||||
hasFlag := opts.FlagAmount > 0
|
||||
|
||||
if hasArg && !hasFlag {
|
||||
repoUrl = addArg(opts, repoUrl, branchName)
|
||||
} else if !hasArg && hasFlag {
|
||||
err, repoUrl = addFlag(opts, repoUrl)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
} else if hasArg && hasFlag {
|
||||
if opts.Branch != "" {
|
||||
err, repoUrl = addBranch(opts, repoUrl)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
} else if hasArg && (!hasFlag || opts.RepoFlag) {
|
||||
err, repoUrl = addArg(opts, repoUrl, branchName)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
} else if !hasArg && hasFlag {
|
||||
repoUrl = addFlag(opts, repoUrl)
|
||||
} else {
|
||||
fmt.Fprintf(opts.IO.Out, "now opening %s in browser . . .\n", repoUrl)
|
||||
}
|
||||
|
||||
opts.Browser.Browse(repoUrl)
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
func addBranch(opts *BrowseOptions, url string) (error, string) {
|
||||
help := "Use 'gh browse --help' for more information about browse\n"
|
||||
|
||||
if opts.Branch == "" {
|
||||
return fmt.Errorf("invalid use of flag and argument\n%s", help), ""
|
||||
}
|
||||
|
||||
if opts.SelectorArg == "" {
|
||||
url += "/tree/" + opts.Branch
|
||||
fmt.Fprintf(opts.IO.Out, "now opening %s in browser . . .\n", url)
|
||||
return nil, url
|
||||
} else {
|
||||
err, arr := parseFileArg(opts.SelectorArg)
|
||||
if err != nil {
|
||||
return err, url
|
||||
}
|
||||
if len(arr) > 1 {
|
||||
url += "/tree/" + opts.Branch + "/" + arr[0] + "#L" + arr[1]
|
||||
} else {
|
||||
url += "/tree/" + opts.Branch + "/" + arr[0]
|
||||
}
|
||||
}
|
||||
|
||||
arr := parseFileArg(opts)
|
||||
if len(arr) > 1 {
|
||||
return nil, url + "/tree/" + opts.Branch + "/" + arr[0] + "#L" + arr[1]
|
||||
}
|
||||
return nil, url + "/tree/" + opts.Branch + "/" + arr[0]
|
||||
fmt.Fprintf(opts.IO.Out, "now opening %s in browser . . .\n", url)
|
||||
return nil, url
|
||||
}
|
||||
|
||||
func addFlag(opts *BrowseOptions, url string) (error, string) {
|
||||
func addFlag(opts *BrowseOptions, url string) string {
|
||||
if opts.ProjectsFlag {
|
||||
url += "/projects"
|
||||
} else if opts.SettingsFlag {
|
||||
|
|
@ -170,37 +173,37 @@ func addFlag(opts *BrowseOptions, url string) (error, string) {
|
|||
url += "/wiki"
|
||||
}
|
||||
fmt.Fprintf(opts.IO.Out, "now opening %s in browser . . .\n", url)
|
||||
return nil, url
|
||||
return url
|
||||
}
|
||||
|
||||
func addArg(opts *BrowseOptions, url string, branchName string) string {
|
||||
func addArg(opts *BrowseOptions, url string, branchName string) (error, string) {
|
||||
if isNumber(opts.SelectorArg) {
|
||||
url += "/issues/" + opts.SelectorArg
|
||||
fmt.Fprintf(opts.IO.Out, "now opening issue/pr in browser . . .\n")
|
||||
return url
|
||||
}
|
||||
|
||||
arr := parseFileArg(opts)
|
||||
if len(arr) > 1 {
|
||||
} else {
|
||||
err, arr := parseFileArg(opts.SelectorArg)
|
||||
if err != nil {
|
||||
return err, url
|
||||
}
|
||||
if len(arr) > 1 {
|
||||
url += "/tree/" + branchName + "/" + arr[0] + "#L" + arr[1]
|
||||
} else {
|
||||
url += "/tree/" + branchName + "/" + arr[0]
|
||||
}
|
||||
fmt.Fprintf(opts.IO.Out, "now opening %s in browser . . .\n", url)
|
||||
return url + "/tree/" + branchName + "/" + arr[0] + "#L" + arr[1]
|
||||
}
|
||||
|
||||
fmt.Fprintf(opts.IO.Out, "now opening %s in browser . . .\n", url)
|
||||
return url + "/tree/" + branchName + "/" + arr[0]
|
||||
return nil, url
|
||||
}
|
||||
|
||||
func parseFileArg(opts *BrowseOptions) []string {
|
||||
arr := strings.Split(opts.SelectorArg, ":")
|
||||
return arr
|
||||
}
|
||||
|
||||
func hasFlag(opts *BrowseOptions) bool {
|
||||
return opts.FlagAmount > 0
|
||||
}
|
||||
|
||||
func hasArg(opts *BrowseOptions) bool {
|
||||
return opts.SelectorArg != ""
|
||||
func parseFileArg(fileArg string) (error, []string) {
|
||||
arr := strings.Split(fileArg, ":")
|
||||
if len(arr) > 2 {
|
||||
return fmt.Errorf("invalid use of colon\nUse 'gh browse --help' for more information about browse\n"), arr
|
||||
}
|
||||
if len(arr) > 1 && !isNumber(arr[1]) {
|
||||
return fmt.Errorf("invalid line number after colon\nUse 'gh browse --help' for more information about browse\n"), arr
|
||||
}
|
||||
return nil, arr
|
||||
}
|
||||
|
||||
func isNumber(arg string) bool {
|
||||
|
|
|
|||
|
|
@ -16,20 +16,7 @@ import (
|
|||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
type args struct {
|
||||
repo ghrepo.Interface
|
||||
cli string
|
||||
}
|
||||
type testCase struct {
|
||||
name string
|
||||
args args
|
||||
errorExpected bool
|
||||
stdoutExpected string
|
||||
stderrExpected string
|
||||
urlExpected string
|
||||
}
|
||||
|
||||
func runCommand(rt http.RoundTripper, t testCase) (*test.CmdOut, error) {
|
||||
func runCommand(rt http.RoundTripper, repo ghrepo.Interface, cli string) (*test.CmdOut, error) {
|
||||
io, _, stdout, stderr := iostreams.Test()
|
||||
|
||||
browser := &cmdutil.TestBrowser{}
|
||||
|
|
@ -41,13 +28,13 @@ func runCommand(rt http.RoundTripper, t testCase) (*test.CmdOut, error) {
|
|||
return &http.Client{Transport: rt}, nil
|
||||
},
|
||||
BaseRepo: func() (ghrepo.Interface, error) {
|
||||
return t.args.repo, nil
|
||||
return repo, nil
|
||||
},
|
||||
}
|
||||
|
||||
cmd := NewCmdBrowse(factory)
|
||||
|
||||
argv, err := shlex.Split(t.args.cli)
|
||||
argv, err := shlex.Split(cli)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
|
@ -65,7 +52,20 @@ func runCommand(rt http.RoundTripper, t testCase) (*test.CmdOut, error) {
|
|||
}, err
|
||||
}
|
||||
func TestNewCmdBrowse(t *testing.T) {
|
||||
var tests = []testCase{
|
||||
|
||||
type args struct {
|
||||
repo ghrepo.Interface
|
||||
cli string
|
||||
}
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
args args
|
||||
errorExpected bool
|
||||
stdoutExpected string
|
||||
stderrExpected string
|
||||
urlExpected string
|
||||
}{
|
||||
{
|
||||
name: "multiple flag",
|
||||
args: args{
|
||||
|
|
@ -74,7 +74,7 @@ func TestNewCmdBrowse(t *testing.T) {
|
|||
},
|
||||
errorExpected: true,
|
||||
stdoutExpected: "",
|
||||
stderrExpected: "accepts 1 flag, 2 flag(s) were recieved",
|
||||
stderrExpected: "these two flags are incompatible, see below for instructions",
|
||||
urlExpected: "",
|
||||
},
|
||||
{
|
||||
|
|
@ -131,7 +131,7 @@ func TestNewCmdBrowse(t *testing.T) {
|
|||
_, cmdTeardown := run.Stub()
|
||||
defer cmdTeardown(t)
|
||||
|
||||
output, err := runCommand(http, tt)
|
||||
output, err := runCommand(http, tt.args.repo, tt.args.cli)
|
||||
|
||||
if tt.errorExpected {
|
||||
assert.Equal(t, err.Error(), tt.stderrExpected)
|
||||
|
|
@ -143,3 +143,46 @@ func TestNewCmdBrowse(t *testing.T) {
|
|||
})
|
||||
}
|
||||
}
|
||||
func TestFileArgParsing(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
arg string
|
||||
errorExpected bool
|
||||
fileArg string
|
||||
lineArg string
|
||||
stderrExpected string
|
||||
}{
|
||||
{
|
||||
name: "non line number",
|
||||
arg: "main.go",
|
||||
errorExpected: false,
|
||||
fileArg: "main.go",
|
||||
},
|
||||
{
|
||||
name: "line number",
|
||||
arg: "main.go:32",
|
||||
errorExpected: false,
|
||||
fileArg: "main.go",
|
||||
lineArg: "32",
|
||||
},
|
||||
{
|
||||
name: "non line number error",
|
||||
arg: "ma:in.go",
|
||||
errorExpected: true,
|
||||
stderrExpected: "invalid line number after colon\nUse 'gh browse --help' for more information about browse\n",
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
err, arr := parseFileArg(tt.arg)
|
||||
if tt.errorExpected {
|
||||
assert.Equal(t, err.Error(), tt.stderrExpected)
|
||||
} else {
|
||||
assert.Equal(t, err, nil)
|
||||
if len(arr) > 1 {
|
||||
assert.Equal(t, tt.lineArg, arr[1])
|
||||
}
|
||||
assert.Equal(t, tt.fileArg, arr[0])
|
||||
}
|
||||
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue