From 23f68481e421f69a47fe9bbbb6dfbaaaa0cab30d Mon Sep 17 00:00:00 2001 From: Jingwen Owen Ou Date: Sat, 7 Dec 2013 11:40:15 -0800 Subject: [PATCH] Fix bug due to project owner and name parsing --- commands/create.go | 4 ++-- commands/fetch.go | 4 ++-- commands/pull_request.go | 23 ++++++++++++++------ github/project.go | 41 ++++++++++------------------------- github/project_test.go | 47 ++++++++++++++++------------------------ 5 files changed, 51 insertions(+), 68 deletions(-) diff --git a/commands/create.go b/commands/create.go index 5f7a0c78..6ce1549e 100644 --- a/commands/create.go +++ b/commands/create.go @@ -61,7 +61,7 @@ func create(command *Command, args *Args) { } var msg string - project := github.NewProjectFromNameAndOwner(name, "") + project := github.NewProjectFromOwnerAndName("", name) gh := github.NewWithoutProject() if gh.IsRepositoryExist(project) { fmt.Printf("%s already exists on %s\n", project, github.GitHubHost) @@ -70,7 +70,7 @@ func create(command *Command, args *Args) { if !args.Noop { repo, err := gh.CreateRepository(project, flagCreateDescription, flagCreateHomepage, flagCreatePrivate) utils.Check(err) - project = github.NewProjectFromNameAndOwner("", repo.FullName) + project = github.NewProjectFromOwnerAndName(repo.FullName, "") } msg = "created repository" } diff --git a/commands/fetch.go b/commands/fetch.go index 842f2edf..aa021a8c 100644 --- a/commands/fetch.go +++ b/commands/fetch.go @@ -46,13 +46,13 @@ func tranformFetchArgs(args *Args) error { ownerRegexp := regexp.MustCompile(OwnerRe) for _, name := range names { if ownerRegexp.MatchString(name) && !hasGitRemote(name) { - project := github.NewProjectFromNameAndOwner("", name) + project := github.NewProjectFromOwnerAndName("", name) repo, err := gh.Repository(project) if err != nil { continue } - project = github.NewProjectFromNameAndOwner("", repo.FullName) + project = github.NewProjectFromOwnerAndName(repo.FullName, "") projects = append(projects, project) } } diff --git a/commands/pull_request.go b/commands/pull_request.go index 55889ca0..ad868e86 100644 --- a/commands/pull_request.go +++ b/commands/pull_request.go @@ -7,6 +7,7 @@ import ( "github.com/jingweno/gh/git" "github.com/jingweno/gh/github" "github.com/jingweno/gh/utils" + "io" "io/ioutil" "os" "reflect" @@ -84,9 +85,6 @@ func pullRequest(cmd *Command, args *Args) { headProject, err := localRepo.CurrentProject() utils.Check(err) - gh := github.NewWithoutProject() - gh.Project = baseProject - var ( base, head string force, explicitOwner bool @@ -96,7 +94,11 @@ func pullRequest(cmd *Command, args *Args) { if strings.Contains(flagPullRequestBase, ":") { split := strings.SplitN(flagPullRequestBase, ":", 2) base = split[1] - baseProject.Owner = split[0] + var name string + if !strings.Contains(split[0], "/") { + name = baseProject.Name + } + baseProject = github.NewProjectFromOwnerAndName(split[0], name) } else { base = flagPullRequestBase } @@ -106,7 +108,7 @@ func pullRequest(cmd *Command, args *Args) { if strings.Contains(flagPullRequestHead, ":") { split := strings.SplitN(flagPullRequestHead, ":", 2) head = split[1] - headProject.Owner = split[0] + headProject = github.NewProjectFromOwnerAndName(split[0], headProject.Name) explicitOwner = true } else { head = flagPullRequestHead @@ -152,9 +154,12 @@ func pullRequest(cmd *Command, args *Args) { } } + gh := github.NewWithoutProject() + gh.Project = baseProject + // when no tracking, assume remote branch is published under active user's fork if tberr != nil && !explicitOwner && gh.Config.User != headProject.Owner { - headProject = github.NewProjectFromString(headProject.Name) + headProject = github.NewProjectFromOwnerAndName("", headProject.Name) } var title, body string @@ -333,13 +338,17 @@ func readTitleAndBodyFrom(reader *bufio.Reader) (title, body string, err error) line, err = readLine(reader) } + if err == io.EOF { + err = nil + } + title = strings.Join(titleParts, " ") title = strings.TrimSpace(title) body = strings.Join(bodyParts, "\n") body = strings.TrimSpace(body) - return title, body, nil + return } func readLine(r *bufio.Reader) (string, error) { diff --git a/github/project.go b/github/project.go index 918fbc9a..f52df243 100644 --- a/github/project.go +++ b/github/project.go @@ -95,40 +95,23 @@ func NewProjectFromURL(url *url.URL) (*Project, error) { return &Project{Name: name, Owner: parts[1]}, nil } -// NewProjectFromURL creates a new Project from a string +// NewProjectFromOwnerAndName creates a new Project from a specified owner and name // -// If the string is in the format of OWNER/NAME, it's split and used as the owner and name of the Project. -// Otherwise the string is used as the name of the Project and the current user is used as the owner. -// If the string is empty, the current dir name is used as the name of the Project. -func NewProjectFromString(nameAndOwner string) *Project { - var name, owner string - if strings.Contains(nameAndOwner, "/") { - result := strings.SplitN(nameAndOwner, "/", 2) - owner = result[0] - name = result[1] - } else { - name = nameAndOwner - } - - if owner == "" { - owner = CurrentConfig().FetchUser() - } - - if name == "" { - name, _ = utils.DirName() - } - - return &Project{Name: name, Owner: owner} -} - -func NewProjectFromNameAndOwner(name, owner string) *Project { +// If the owner or the name string is in the format of OWNER/NAME, it's split and used as the owner and name of the Project. +// If the owner string is empty, the current user is used as the name of the Project. +// If the name string is empty, the current dir name is used as the name of the Project. +func NewProjectFromOwnerAndName(owner, name string) *Project { if strings.Contains(owner, "/") { result := strings.SplitN(owner, "/", 2) owner = result[0] - name = result[1] + if name == "" { + name = result[1] + } } else if strings.Contains(name, "/") { - result := strings.SplitN(owner, "/", 2) - owner = result[0] + result := strings.SplitN(name, "/", 2) + if owner == "" { + owner = result[0] + } name = result[1] } diff --git a/github/project_test.go b/github/project_test.go index 6e3ea5a5..788f5397 100644 --- a/github/project_test.go +++ b/github/project_test.go @@ -14,42 +14,33 @@ func TestNewProjectOwnerAndName(t *testing.T) { SaveConfig(&config) defer os.RemoveAll(filepath.Dir(DefaultConfigFile)) - project := NewProjectFromNameAndOwner("", "jingweno/gh") + project := NewProjectFromOwnerAndName("", "mojombo/gh") + assert.Equal(t, "mojombo", project.Owner) + assert.Equal(t, "gh", project.Name) + project = NewProjectFromOwnerAndName("progmob", "mojombo/gh") + assert.Equal(t, "progmob", project.Owner) + assert.Equal(t, "gh", project.Name) + + project = NewProjectFromOwnerAndName("mojombo/jekyll", "gh") + assert.Equal(t, "mojombo", project.Owner) + assert.Equal(t, "gh", project.Name) + + project = NewProjectFromOwnerAndName("mojombo/gh", "") + assert.Equal(t, "mojombo", project.Owner) + assert.Equal(t, "gh", project.Name) + + project = NewProjectFromOwnerAndName("", "gh") assert.Equal(t, "jingweno", project.Owner) assert.Equal(t, "gh", project.Name) - project = NewProjectFromNameAndOwner("gh", "") - - assert.Equal(t, "jingweno", project.Owner) - assert.Equal(t, "gh", project.Name) - - project = NewProjectFromNameAndOwner("", "jingweno/gh/foo") - + project = NewProjectFromOwnerAndName("", "jingweno/gh/foo") assert.Equal(t, "jingweno", project.Owner) assert.Equal(t, "gh/foo", project.Name) -} -func TestNewProjectFromString(t *testing.T) { - DefaultConfigFile = "./test_support/clone_gh" - config := Config{User: "jingweno", Token: "123"} - SaveConfig(&config) - defer os.RemoveAll(filepath.Dir(DefaultConfigFile)) - - project := NewProjectFromString("jingweno/gh") - - assert.Equal(t, "jingweno", project.Owner) + project = NewProjectFromOwnerAndName("mojombo", "gh") + assert.Equal(t, "mojombo", project.Owner) assert.Equal(t, "gh", project.Name) - - project = NewProjectFromString("gh") - - assert.Equal(t, "jingweno", project.Owner) - assert.Equal(t, "gh", project.Name) - - project = NewProjectFromString("jingweno/gh/foo") - - assert.Equal(t, "jingweno", project.Owner) - assert.Equal(t, "gh/foo", project.Name) } func TestWebURL(t *testing.T) {