X Tutup
Skip to content

Commit 641de86

Browse files
mislavmislav
committed
Eliminate package-level state in git remote parsing
1 parent 0732234 commit 641de86

File tree

11 files changed

+200
-132
lines changed

11 files changed

+200
-132
lines changed

‎command/root.go‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import (
55
"os"
66

77
"github.com/github/gh-cli/context"
8-
"github.com/github/gh-cli/git"
98
"github.com/spf13/cobra"
109
)
1110

@@ -27,8 +26,6 @@ func initContext() {
2726
repo = os.Getenv("GH_REPO")
2827
}
2928
ctx.SetBaseRepo(repo)
30-
31-
git.InitSSHAliasMap(nil)
3229
}
3330

3431
// RootCmd is the entry point of command-line execution

‎context/config_file_test.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
)
99

1010
func eq(t *testing.T, got interface{}, expected interface{}) {
11+
t.Helper()
1112
if !reflect.DeepEqual(got, expected) {
1213
t.Errorf("expected: %v, got: %v", expected, got)
1314
}

‎context/context.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,8 @@ func (c *fsContext) Remotes() (Remotes, error) {
109109
if err != nil {
110110
return nil, err
111111
}
112-
c.remotes = parseRemotes(gitRemotes)
112+
sshTranslate := git.ParseSSHConfig().Translator()
113+
c.remotes = translateRemotes(gitRemotes, sshTranslate)
113114
}
114115
return c.remotes, nil
115116
}

‎context/remote.go‎

Lines changed: 21 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ package context
22

33
import (
44
"fmt"
5-
"regexp"
5+
"net/url"
66
"strings"
77

88
"github.com/github/gh-cli/git"
@@ -27,74 +27,44 @@ func (r Remotes) FindByName(names ...string) (*Remote, error) {
2727

2828
// Remote represents a git remote mapped to a GitHub repository
2929
type Remote struct {
30-
Name string
30+
*git.Remote
3131
Owner string
3232
Repo string
3333
}
3434

35-
func (r *Remote) String() string {
36-
return r.Name
37-
}
38-
3935
// GitHubRepository represents a GitHub respository
4036
type GitHubRepository struct {
4137
Name string
4238
Owner string
4339
}
4440

45-
func parseRemotes(gitRemotes []string) (remotes Remotes) {
46-
re := regexp.MustCompile(`(.+)\s+(.+)\s+\((push|fetch)\)`)
47-
48-
names := []string{}
49-
remotesMap := make(map[string]map[string]string)
41+
// TODO: accept an interface instead of git.RemoteSet
42+
func translateRemotes(gitRemotes git.RemoteSet, urlTranslate func(*url.URL) *url.URL) (remotes Remotes) {
5043
for _, r := range gitRemotes {
51-
if re.MatchString(r) {
52-
match := re.FindStringSubmatch(r)
53-
name := strings.TrimSpace(match[1])
54-
url := strings.TrimSpace(match[2])
55-
urlType := strings.TrimSpace(match[3])
56-
utm, ok := remotesMap[name]
57-
if !ok {
58-
utm = make(map[string]string)
59-
remotesMap[name] = utm
60-
names = append(names, name)
61-
}
62-
utm[urlType] = url
44+
var owner string
45+
var repo string
46+
if r.FetchURL != nil {
47+
owner, repo, _ = repoFromURL(urlTranslate(r.FetchURL))
6348
}
64-
}
65-
66-
for _, name := range names {
67-
urlMap := remotesMap[name]
68-
repo, err := repoFromURL(urlMap["fetch"])
69-
if err != nil {
70-
repo, err = repoFromURL(urlMap["push"])
71-
}
72-
if err == nil {
73-
remotes = append(remotes, &Remote{
74-
Name: name,
75-
Owner: repo.Owner,
76-
Repo: repo.Name,
77-
})
49+
if r.PushURL != nil && owner == "" {
50+
owner, repo, _ = repoFromURL(urlTranslate(r.PushURL))
7851
}
52+
remotes = append(remotes, &Remote{
53+
Remote: r,
54+
Owner: owner,
55+
Repo: repo,
56+
})
7957
}
80-
8158
return
8259
}
8360

84-
func repoFromURL(u string) (*GitHubRepository, error) {
85-
url, err := git.ParseURL(u)
86-
if err != nil {
87-
return nil, err
88-
}
89-
if url.Hostname() != defaultHostname {
90-
return nil, fmt.Errorf("invalid hostname: %s", url.Hostname())
61+
func repoFromURL(u *url.URL) (string, string, error) {
62+
if !strings.EqualFold(u.Hostname(), defaultHostname) {
63+
return "", "", fmt.Errorf("unsupported hostname: %s", u.Hostname())
9164
}
92-
parts := strings.SplitN(strings.TrimPrefix(url.Path, "/"), "/", 3)
65+
parts := strings.SplitN(strings.TrimPrefix(u.Path, "/"), "/", 3)
9366
if len(parts) < 2 {
94-
return nil, fmt.Errorf("invalid path: %s", url.Path)
67+
return "", "", fmt.Errorf("invalid path: %s", u.Path)
9568
}
96-
return &GitHubRepository{
97-
Owner: parts[0],
98-
Name: strings.TrimSuffix(parts[1], ".git"),
99-
}, nil
69+
return parts[0], strings.TrimSuffix(parts[1], ".git"), nil
10070
}

‎context/remote_test.go‎

Lines changed: 22 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -2,67 +2,43 @@ package context
22

33
import (
44
"errors"
5+
"net/url"
56
"testing"
67

78
"github.com/github/gh-cli/git"
89
)
910

1011
func Test_repoFromURL(t *testing.T) {
11-
git.InitSSHAliasMap(nil)
12-
13-
r, err := repoFromURL("http://github.com/monalisa/octo-cat.git")
12+
u, _ := url.Parse("http://github.com/monalisa/octo-cat.git")
13+
owner, repo, err := repoFromURL(u)
1414
eq(t, err, nil)
15-
eq(t, r, &GitHubRepository{Owner: "monalisa", Name: "octo-cat"})
15+
eq(t, owner, "monalisa")
16+
eq(t, repo, "octo-cat")
1617
}
1718

1819
func Test_repoFromURL_invalid(t *testing.T) {
19-
git.InitSSHAliasMap(nil)
20-
21-
_, err := repoFromURL("https://example.com/one/two")
22-
eq(t, err, errors.New(`invalid hostname: example.com`))
23-
24-
_, err = repoFromURL("/path/to/disk")
25-
eq(t, err, errors.New(`invalid hostname: `))
26-
}
27-
28-
func Test_repoFromURL_SSH(t *testing.T) {
29-
git.InitSSHAliasMap(map[string]string{
30-
"gh": "github.com",
31-
"github.com": "ssh.github.com",
32-
})
33-
34-
r, err := repoFromURL("git@gh:monalisa/octo-cat")
35-
eq(t, err, nil)
36-
eq(t, r, &GitHubRepository{Owner: "monalisa", Name: "octo-cat"})
37-
38-
r, err = repoFromURL("git@github.com:monalisa/octo-cat")
39-
eq(t, err, nil)
40-
eq(t, r, &GitHubRepository{Owner: "monalisa", Name: "octo-cat"})
41-
}
42-
43-
func Test_parseRemotes(t *testing.T) {
44-
git.InitSSHAliasMap(nil)
45-
46-
remoteList := []string{
47-
"mona\tgit@github.com:monalisa/myfork.git (fetch)",
48-
"origin\thttps://github.com/monalisa/octo-cat.git (fetch)",
49-
"origin\thttps://github.com/monalisa/octo-cat-push.git (push)",
50-
"upstream\thttps://example.com/nowhere.git (fetch)",
51-
"upstream\thttps://github.com/hubot/tools (push)",
20+
cases := [][]string{
21+
[]string{
22+
"https://example.com/one/two",
23+
"unsupported hostname: example.com",
24+
},
25+
[]string{
26+
"/path/to/disk",
27+
"unsupported hostname: ",
28+
},
29+
}
30+
for _, c := range cases {
31+
u, _ := url.Parse(c[0])
32+
_, _, err := repoFromURL(u)
33+
eq(t, err, errors.New(c[1]))
5234
}
53-
r := parseRemotes(remoteList)
54-
eq(t, len(r), 3)
55-
56-
eq(t, r[0], &Remote{Name: "mona", Owner: "monalisa", Repo: "myfork"})
57-
eq(t, r[1], &Remote{Name: "origin", Owner: "monalisa", Repo: "octo-cat"})
58-
eq(t, r[2], &Remote{Name: "upstream", Owner: "hubot", Repo: "tools"})
5935
}
6036

6137
func Test_Remotes_FindByName(t *testing.T) {
6238
list := Remotes{
63-
&Remote{Name: "mona", Owner: "monalisa", Repo: "myfork"},
64-
&Remote{Name: "origin", Owner: "monalisa", Repo: "octo-cat"},
65-
&Remote{Name: "upstream", Owner: "hubot", Repo: "tools"},
39+
&Remote{Remote: &git.Remote{Name: "mona"}, Owner: "monalisa", Repo: "myfork"},
40+
&Remote{Remote: &git.Remote{Name: "origin"}, Owner: "monalisa", Repo: "octo-cat"},
41+
&Remote{Remote: &git.Remote{Name: "upstream"}, Owner: "hubot", Repo: "tools"},
6642
}
6743

6844
r, err := list.FindByName("upstream", "origin")

‎git/git.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ func Log(sha1, sha2 string) (string, error) {
165165
return string(outputs), nil
166166
}
167167

168-
func Remotes() ([]string, error) {
168+
func listRemotes() ([]string, error) {
169169
remoteCmd := exec.Command("git", "remote", "-v")
170170
remoteCmd.Stderr = nil
171171
output, err := remoteCmd.Output()

‎git/remote.go‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
package git
2+
3+
import (
4+
"net/url"
5+
"regexp"
6+
"strings"
7+
)
8+
9+
var remoteRE = regexp.MustCompile(`(.+)\s+(.+)\s+\((push|fetch)\)`)
10+
11+
// RemoteSet is a slice of git remotes
12+
type RemoteSet []*Remote
13+
14+
// Remote is a parsed git remote
15+
type Remote struct {
16+
Name string
17+
FetchURL *url.URL
18+
PushURL *url.URL
19+
}
20+
21+
func (r *Remote) String() string {
22+
return r.Name
23+
}
24+
25+
// Remotes gets the git remotes set for the current repo
26+
func Remotes() (RemoteSet, error) {
27+
list, err := listRemotes()
28+
if err != nil {
29+
return nil, err
30+
}
31+
return parseRemotes(list), nil
32+
}
33+
34+
func parseRemotes(gitRemotes []string) (remotes RemoteSet) {
35+
for _, r := range gitRemotes {
36+
match := remoteRE.FindStringSubmatch(r)
37+
if match == nil {
38+
continue
39+
}
40+
name := strings.TrimSpace(match[1])
41+
urlStr := strings.TrimSpace(match[2])
42+
urlType := strings.TrimSpace(match[3])
43+
44+
var rem *Remote
45+
if len(remotes) > 0 {
46+
rem = remotes[len(remotes)-1]
47+
if name != rem.Name {
48+
rem = nil
49+
}
50+
}
51+
if rem == nil {
52+
rem = &Remote{Name: name}
53+
remotes = append(remotes, rem)
54+
}
55+
56+
u, err := ParseURL(urlStr)
57+
if err != nil {
58+
continue
59+
}
60+
61+
switch urlType {
62+
case "fetch":
63+
rem.FetchURL = u
64+
case "push":
65+
rem.PushURL = u
66+
}
67+
}
68+
return
69+
}

‎git/remote_test.go‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
package git
2+
3+
import "testing"
4+
5+
func Test_parseRemotes(t *testing.T) {
6+
remoteList := []string{
7+
"mona\tgit@github.com:monalisa/myfork.git (fetch)",
8+
"origin\thttps://github.com/monalisa/octo-cat.git (fetch)",
9+
"origin\thttps://github.com/monalisa/octo-cat-push.git (push)",
10+
"upstream\thttps://example.com/nowhere.git (fetch)",
11+
"upstream\thttps://github.com/hubot/tools (push)",
12+
"zardoz\thttps://example.com/zed.git (push)",
13+
}
14+
r := parseRemotes(remoteList)
15+
eq(t, len(r), 4)
16+
17+
eq(t, r[0].Name, "mona")
18+
eq(t, r[0].FetchURL.String(), "ssh://git@github.com/monalisa/myfork.git")
19+
if r[0].PushURL != nil {
20+
t.Errorf("expected no PushURL, got %q", r[0].PushURL)
21+
}
22+
eq(t, r[1].Name, "origin")
23+
eq(t, r[1].FetchURL.Path, "/monalisa/octo-cat.git")
24+
eq(t, r[1].PushURL.Path, "/monalisa/octo-cat-push.git")
25+
26+
eq(t, r[2].Name, "upstream")
27+
eq(t, r[2].FetchURL.Host, "example.com")
28+
eq(t, r[2].PushURL.Host, "github.com")
29+
30+
eq(t, r[3].Name, "zardoz")
31+
}

0 commit comments

Comments
 (0)
X Tutup