Skip to content

Commit 2b42ab9

Browse files
bchadwicbchadwic
committed
refactored browse.go
1 parent 5f64243 commit 2b42ab9

File tree

2 files changed

+58
-89
lines changed

2 files changed

+58
-89
lines changed

‎pkg/cmd/browse/browse.go‎

Lines changed: 57 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111
"github.com/cli/cli/internal/ghrepo"
1212
"github.com/cli/cli/pkg/cmdutil"
1313
"github.com/cli/cli/pkg/iostreams"
14-
"github.com/cli/cli/utils"
1514
"github.com/spf13/cobra"
1615
)
1716

@@ -25,7 +24,6 @@ type BrowseOptions struct {
2524
HttpClient func() (*http.Client, error)
2625
IO *iostreams.IOStreams
2726

28-
FlagAmount int
2927
SelectorArg string
3028

3129
Branch string
@@ -71,25 +69,33 @@ func NewCmdBrowse(f *cmdutil.Factory, runF func(*BrowseOptions) error) *cobra.Co
7169
- by path for opening folders and files, e.g. "cmd/gh/main.go"
7270
`),
7371
"help:environment": heredoc.Doc(`
74-
To configure a web browser other than the default, use the BROWSER environment variable.
72+
To configure a web browser other than the default, use the BROWSER environment variable.
7573
`),
7674
},
7775
RunE: func(cmd *cobra.Command, args []string) error {
7876
opts.BaseRepo = f.BaseRepo
79-
opts.FlagAmount = cmd.Flags().NFlag()
8077

81-
if opts.FlagAmount > 2 {
82-
return &cmdutil.FlagError{Err: fmt.Errorf("cannot have more than two flags, %d were recieved", opts.FlagAmount)}
78+
if len(args) > 0 {
79+
opts.SelectorArg = args[0]
8380
}
84-
if repoOverride, _ := cmd.Flags().GetString("repo"); repoOverride != "" {
85-
opts.RepoFlag = true
81+
82+
if err := cmdutil.MutuallyExclusive("", opts.ProjectsFlag, opts.SettingsFlag); err != nil {
83+
return err
8684
}
87-
if opts.FlagAmount > 1 && !opts.RepoFlag {
88-
return &cmdutil.FlagError{Err: fmt.Errorf("these two flags are incompatible, see below for instructions")}
85+
if err := cmdutil.MutuallyExclusive("", opts.ProjectsFlag, opts.WikiFlag); err != nil {
86+
return err
8987
}
90-
91-
if len(args) > 0 {
92-
opts.SelectorArg = args[0]
88+
if err := cmdutil.MutuallyExclusive("", opts.ProjectsFlag, opts.Branch != ""); err != nil {
89+
return err
90+
}
91+
if err := cmdutil.MutuallyExclusive("", opts.SettingsFlag, opts.WikiFlag); err != nil {
92+
return err
93+
}
94+
if err := cmdutil.MutuallyExclusive("", opts.SettingsFlag, opts.Branch != ""); err != nil {
95+
return err
96+
}
97+
if err := cmdutil.MutuallyExclusive("", opts.WikiFlag, opts.Branch != ""); err != nil {
98+
return err
9399
}
94100

95101
if runF != nil {
@@ -98,6 +104,7 @@ func NewCmdBrowse(f *cmdutil.Factory, runF func(*BrowseOptions) error) *cobra.Co
98104
return runBrowse(opts)
99105
},
100106
}
107+
101108
cmdutil.EnableRepoOverride(cmd, f)
102109
cmd.Flags().BoolVarP(&opts.ProjectsFlag, "projects", "p", false, "Open repository projects")
103110
cmd.Flags().BoolVarP(&opts.WikiFlag, "wiki", "w", false, "Open repository wiki")
@@ -119,96 +126,61 @@ func runBrowse(opts *BrowseOptions) error {
119126
return fmt.Errorf("unable to create an http client: %w\nUse 'gh browse --help' for more information about browse\n", err)
120127
}
121128

122-
repoUrl := ghrepo.GenerateRepoURL(baseRepo, "")
129+
url := ghrepo.GenerateRepoURL(baseRepo, "")
123130

124-
hasArg := opts.SelectorArg != ""
125-
hasFlag := opts.FlagAmount > 0
126-
127-
if opts.Branch != "" {
128-
repoUrl, err = addBranch(opts, repoUrl)
129-
if err != nil {
130-
return err
131-
}
132-
} else if hasArg && (!hasFlag || opts.RepoFlag) {
133-
apiClient := api.NewClientFromHTTP(httpClient)
134-
branchName, err := api.RepoDefaultBranch(apiClient, baseRepo)
135-
if err != nil {
136-
return fmt.Errorf("unable to determine default branch for %s: %w", ghrepo.FullName(baseRepo), err)
137-
}
138-
repoUrl, err = addArg(opts, repoUrl, branchName)
139-
if err != nil {
140-
return err
141-
}
142-
} else if !hasArg && hasFlag {
143-
repoUrl = addFlag(opts, repoUrl)
144-
} else {
145-
if opts.IO.IsStdoutTTY() {
146-
fmt.Fprintf(opts.IO.Out, "Opening %s in your browser.\n", utils.DisplayURL(repoUrl))
147-
}
131+
if opts.ProjectsFlag {
132+
url += "/projects"
133+
err := opts.Browser.Browse(url)
134+
return err
148135
}
149-
opts.Browser.Browse(repoUrl)
150-
return nil
151-
}
152136

153-
func addBranch(opts *BrowseOptions, url string) (string, error) {
154-
if opts.SelectorArg == "" {
155-
url += "/tree/" + opts.Branch
156-
} else {
157-
arr, err := parseFileArg(opts.SelectorArg)
158-
if err != nil {
159-
return url, err
160-
}
161-
url += parsedUrl(arr, opts.Branch)
162-
}
163-
if opts.IO.IsStdoutTTY() {
164-
fmt.Fprintf(opts.IO.Out, "now opening %s in browser . . .\n", url)
137+
if opts.SettingsFlag {
138+
url += "/settings"
139+
err := opts.Browser.Browse(url)
140+
return err
165141
}
166-
return url, nil
167-
}
168142

169-
func addFlag(opts *BrowseOptions, url string) string {
170-
if opts.ProjectsFlag {
171-
url += "/projects"
172-
} else if opts.SettingsFlag {
173-
url += "/settings"
174-
} else if opts.WikiFlag {
143+
if opts.WikiFlag {
175144
url += "/wiki"
145+
err := opts.Browser.Browse(url)
146+
return err
176147
}
177-
if opts.IO.IsStdoutTTY() {
178-
fmt.Fprintf(opts.IO.Out, "now opening %s in browser . . .\n", url)
179-
}
180-
return url
181-
}
182148

183-
func addArg(opts *BrowseOptions, url string, branchName string) (string, error) {
184149
if isNumber(opts.SelectorArg) {
185150
url += "/issues/" + opts.SelectorArg
186-
if opts.IO.IsStdoutTTY() {
187-
fmt.Fprintf(opts.IO.Out, "now opening issue/pr in browser . . .\n")
188-
}
151+
err := opts.Browser.Browse(url)
152+
return err
153+
}
154+
155+
if opts.Branch != "" {
156+
url += "/tree/" + opts.Branch + "/"
189157
} else {
190-
arr, err := parseFileArg(opts.SelectorArg)
158+
apiClient := api.NewClientFromHTTP(httpClient)
159+
branchName, err := api.RepoDefaultBranch(apiClient, baseRepo)
191160
if err != nil {
192-
return url, err
161+
return err
193162
}
163+
url += "/tree/" + branchName + "/"
164+
}
194165

195-
url += parsedUrl(arr, branchName)
196-
197-
if opts.IO.IsStdoutTTY() {
198-
fmt.Fprintf(opts.IO.Out, "now opening %s in browser . . .\n", url)
166+
if opts.SelectorArg != "" {
167+
arr, err := parseFileArg(opts.SelectorArg)
168+
if err != nil {
169+
return err
170+
}
171+
if len(arr) > 1 {
172+
url += arr[0] + "#L" + arr[1]
173+
} else {
174+
url += arr[0]
199175
}
200176
}
201-
return url, nil
202-
}
203177

204-
func parsedUrl(arr []string, branchName string) string {
205-
if len(arr) > 1 {
206-
return "/tree/" + branchName + "/" + arr[0] + "#L" + arr[1]
207-
} else {
208-
return "/tree/" + branchName + "/" + arr[0]
178+
err = opts.Browser.Browse(url)
179+
if opts.IO.IsStdoutTTY() && err == nil {
180+
fmt.Fprintf(opts.IO.Out, "now opening %s in browser . . .\n", url)
209181
}
182+
return err
210183
}
211-
212184
func parseFileArg(fileArg string) ([]string, error) {
213185
arr := strings.Split(fileArg, ":")
214186
if len(arr) > 2 {
@@ -217,6 +189,7 @@ func parseFileArg(fileArg string) ([]string, error) {
217189
if len(arr) > 1 && !isNumber(arr[1]) {
218190
return arr, fmt.Errorf("invalid line number after colon\nUse 'gh browse --help' for more information about browse\n")
219191
}
192+
220193
return arr, nil
221194
}
222195

‎pkg/cmd/browse/browse_test.go‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ func TestNewCmdBrowse(t *testing.T) {
2929
assert.Equal(t, false, opts.ProjectsFlag)
3030
assert.Equal(t, false, opts.WikiFlag)
3131
assert.Equal(t, false, opts.SettingsFlag)
32-
assert.Equal(t, 1, opts.FlagAmount)
3332
}
3433

3534
func Test_runBrowse(t *testing.T) {
@@ -45,7 +44,6 @@ func Test_runBrowse(t *testing.T) {
4544
name: "no arguments",
4645
opts: BrowseOptions{
4746
SelectorArg: "",
48-
FlagAmount: 0,
4947
},
5048
baseRepo: ghrepo.New("jessica", "cli"),
5149
expectedURL: "https://github.com/jessica/cli",
@@ -60,8 +58,7 @@ func Test_runBrowse(t *testing.T) {
6058
{
6159
name: "branch flag",
6260
opts: BrowseOptions{
63-
Branch: "trunk",
64-
FlagAmount: 1,
61+
Branch: "trunk",
6562
},
6663
baseRepo: ghrepo.New("bchadwic", "cli"),
6764
expectedURL: "https://github.com/bchadwic/cli/tree/trunk",
@@ -70,7 +67,6 @@ func Test_runBrowse(t *testing.T) {
7067
name: "settings flag",
7168
opts: BrowseOptions{
7269
SettingsFlag: true,
73-
FlagAmount: 1,
7470
},
7571
baseRepo: ghrepo.New("bchadwic", "cli"),
7672
expectedURL: "https://github.com/bchadwic/cli/settings",

0 commit comments

Comments
 (0)