Refactor how commented sections are added to contents of text editor

Before, the code outside of `editor.go` was responsible for prefixing
lines of the commented section with the comment char. The code to do
this was scattered all over the codebase and was difficult to refactor
re: adding support for `commentchar=auto`.

Now, Editor has a `AddCommentedSection()` method that allows for
declarative adding of a commented section. Furthermore, the caller
doesn't have to query git to determine the comment char.

commands/issue.go

@@ -367,25 +367,24 @@ func createIssue(cmd *Command, args *Args) {
 		title, body, editor, err = readMsgFromFile(flagIssueFile, flagIssueEdit, "ISSUE", "issue")
 		utils.Check(err)
 	} else {
-		cs := git.CommentChar()
-		message := strings.Replace(fmt.Sprintf(`
-# Creating an issue for %s
-#
-# Write a message for this issue. The first block of
-# text is the title and the rest is the description.
-`, project), "#", cs, -1)
+		message := ""
+		helpMessage := fmt.Sprintf(`Creating an issue for %s
+
+Write a message for this issue. The first block of
+text is the title and the rest is the description.`, project)
 
 		workdir, _ := git.WorkdirName()
 		if workdir != "" {
 			template, err := github.ReadTemplate(github.IssueTemplate, workdir)
 			utils.Check(err)
 			if template != "" {
-				message = template + "\n" + message
+				message = template
 			}
 		}
 
 		editor, err := github.NewEditor("ISSUE", "issue", message)
 		utils.Check(err)
+		editor.AddCommentedSection(helpMessage)
 
 		title, body, err = editor.EditTitleAndBody()
 		utils.Check(err)

commands/pull_request.go

@@ -234,11 +234,45 @@ func pullRequest(cmd *Command, args *Args) {
 			headForMessage = head
 		}
 
-		message, err := createPullRequestMessage(baseTracking, headForMessage, fullBase, fullHead)
-		utils.Check(err)
+		message := ""
+		commitLogs := ""
+
+		commits, _ := git.RefList(baseTracking, headForMessage)
+		if len(commits) == 1 {
+			message, err = git.Show(commits[0])
+			utils.Check(err)
+		} else if len(commits) > 1 {
+			commitLogs, err = git.Log(baseTracking, headForMessage)
+			utils.Check(err)
+		}
+
+		workdir, _ := git.WorkdirName()
+		if workdir != "" {
+			template, _ := github.ReadTemplate(github.PullRequestTemplate, workdir)
+			if template != "" {
+				if message == "" {
+					message = "\n\n" + template
+				} else {
+					parts := strings.SplitN(message, "\n\n", 2)
+					message = parts[0] + "\n\n" + template
+					if len(parts) > 1 && parts[1] != "" {
+						message = message + "\n\n" + parts[1]
+					}
+				}
+			}
+		}
+
+		helpMessage := fmt.Sprintf(`Requesting a pull to %s from %s
+
+Write a message for this pull request. The first block
+of text is the title and the rest is the description.`, fullBase, fullHead)
+		if commitLogs != "" {
+			helpMessage = helpMessage + "\n\nChanges:\n\n" + strings.TrimSpace(commitLogs)
+		}
 
 		editor, err = github.NewEditor("PULLREQ", "pull request", message)
 		utils.Check(err)
+		editor.AddCommentedSection(helpMessage)
 
 		title, body, err = editor.EditTitleAndBody()
 		utils.Check(err)
@@ -349,49 +383,6 @@ func pullRequest(cmd *Command, args *Args) {
 	printBrowseOrCopy(args, pullRequestURL, flagPullRequestBrowse, flagPullRequestCopy)
 }
 
-func createPullRequestMessage(base, head, fullBase, fullHead string) (string, error) {
-	var (
-		defaultMsg string
-		commitLogs string
-		err        error
-	)
-
-	commits, _ := git.RefList(base, head)
-	if len(commits) == 1 {
-		defaultMsg, err = git.Show(commits[0])
-		if err != nil {
-			return "", err
-		}
-	} else if len(commits) > 1 {
-		commitLogs, err = git.Log(base, head)
-		if err != nil {
-			return "", err
-		}
-	}
-
-	workdir, _ := git.WorkdirName()
-	if workdir != "" {
-		template, err := github.ReadTemplate(github.PullRequestTemplate, workdir)
-		if err != nil {
-			return "", err
-		} else if template != "" {
-			if defaultMsg == "" {
-				defaultMsg = "\n\n" + template
-			} else {
-				parts := strings.SplitN(defaultMsg, "\n\n", 2)
-				defaultMsg = parts[0] + "\n\n" + template
-				if len(parts) > 1 && parts[1] != "" {
-					defaultMsg = defaultMsg + "\n\n" + parts[1]
-				}
-			}
-		}
-	}
-
-	cs := git.CommentChar()
-
-	return renderPullRequestTpl(defaultMsg, cs, fullBase, fullHead, commitLogs)
-}
-
 func parsePullRequestProject(context *github.Project, s string) (p *github.Project, ref string) {
 	p = context
 	ref = s

commands/pull_request_tpl.go

-package commands
-
-import (
-	"bytes"
-	"fmt"
-	"regexp"
-	"strings"
-	"text/template"
-)
-
-const pullRequestTmpl = `{{if .InitMsg}}{{.InitMsg}}
-{{end}}
-{{.CS}} Requesting a pull to {{.Base}} from {{.Head}}
-{{.CS}}
-{{.CS}} Write a message for this pull request. The first block
-{{.CS}} of text is the title and the rest is the description.{{if .HasCommitLogs}}
-{{.CS}}
-{{.CS}} Changes:
-{{.CS}}
-{{.FormattedCommitLogs}}{{end}}`
-
-type pullRequestMsg struct {
-	InitMsg    string
-	CS         string
-	Base       string
-	Head       string
-	CommitLogs string
-}
-
-func (p *pullRequestMsg) HasCommitLogs() bool {
-	return len(p.CommitLogs) > 0
-}
-
-func (p *pullRequestMsg) FormattedCommitLogs() string {
-	startRegexp := regexp.MustCompilePOSIX("^")
-	endRegexp := regexp.MustCompilePOSIX(" +$")
-
-	commitLogs := strings.TrimSpace(p.CommitLogs)
-	commitLogs = startRegexp.ReplaceAllString(commitLogs, fmt.Sprintf("%s ", p.CS))
-	commitLogs = endRegexp.ReplaceAllString(commitLogs, "")
-
-	return commitLogs
-}
-
-func renderPullRequestTpl(initMsg, cs, base, head string, commitLogs string) (string, error) {
-	t, err := template.New("pullRequestTmpl").Parse(pullRequestTmpl)
-	if err != nil {
-		return "", err
-	}
-
-	msg := &pullRequestMsg{
-		InitMsg:    initMsg,
-		CS:         cs,
-		Base:       base,
-		Head:       head,
-		CommitLogs: commitLogs,
-	}
-
-	var b bytes.Buffer
-	err = t.Execute(&b, msg)
-
-	return b.String(), err
-}

commands/pull_request_tpl_test.go

-package commands
-
-import (
-	"testing"
-
-	"github.com/bmizerany/assert"
-)
-
-func TestRenderPullRequestTpl(t *testing.T) {
-	msg, err := renderPullRequestTpl("init", "#", "base", "head", "one\ntwo")
-	assert.Equal(t, nil, err)
-
-	expMsg := `init
-
-# Requesting a pull to base from head
-#
-# Write a message for this pull request. The first block
-# of text is the title and the rest is the description.
-#
-# Changes:
-#
-# one
-# two`
-	assert.Equal(t, expMsg, msg)
-}

commands/release.go

@@ -7,7 +7,6 @@ import (
 	"path/filepath"
 	"strings"
 
-	"github.com/github/hub/git"
 	"github.com/github/hub/github"
 	"github.com/github/hub/ui"
 	"github.com/github/hub/utils"
@@ -307,12 +306,15 @@ func createRelease(cmd *Command, args *Args) {
 		title, body, editor, err = readMsgFromFile(flagReleaseFile, flagReleaseEdit, "RELEASE", "release")
 		utils.Check(err)
 	} else {
-		cs := git.CommentChar()
-		message, err := renderReleaseTpl("Creating", cs, tagName, project.String(), flagReleaseCommitish)
-		utils.Check(err)
+		message := ""
+		helpMessage := fmt.Sprintf(`Creating release %s for %s
+
+Write a message for this release. The first block of
+text is the title and the rest is the description.`, tagName, project.String())
 
 		editor, err := github.NewEditor("RELEASE", "release", message)
 		utils.Check(err)
+		editor.AddCommentedSection(helpMessage)
 
 		title, body, err = editor.EditTitleAndBody()
 		utils.Check(err)
@@ -369,11 +371,9 @@ func editRelease(cmd *Command, args *Args) {
 	utils.Check(err)
 
 	params := map[string]interface{}{}
-	commitish := release.TargetCommitish
 
 	if cmd.FlagPassed("commitish") {
 		params["target_commitish"] = flagReleaseCommitish
-		commitish = flagReleaseCommitish
 	}
 
 	if cmd.FlagPassed("draft") {
@@ -398,13 +398,15 @@ func editRelease(cmd *Command, args *Args) {
 			utils.Check(fmt.Errorf("Aborting editing due to empty release title"))
 		}
 	} else {
-		cs := git.CommentChar()
-		message, err := renderReleaseTpl("Editing", cs, tagName, project.String(), commitish)
-		utils.Check(err)
+		message := fmt.Sprintf("%s\n\n%s", release.Name, release.Body)
+		helpMessage := fmt.Sprintf(`Editing release %s for %s
+
+Write a message for this release. The first block of
+text is the title and the rest is the description.`, tagName, project.String())
 
-		message = fmt.Sprintf("%s\n\n%s\n%s", release.Name, release.Body, message)
 		editor, err := github.NewEditor("RELEASE", "release", message)
 		utils.Check(err)
+		editor.AddCommentedSection(helpMessage)
 
 		title, body, err = editor.EditTitleAndBody()
 		utils.Check(err)

commands/release_tpl.go

-package commands
-
-import (
-	"bytes"
-	"text/template"
-)
-
-const releaseTmpl = `
-{{.CS}} {{.Operation}} release {{.TagName}} for {{.ProjectName}}{{if .BranchName}} from {{.BranchName}}{{end}}
-{{.CS}}
-{{.CS}} Write a message for this release. The first block of
-{{.CS}} text is the title and the rest is the description.`
-
-type releaseMsg struct {
-	Operation   string
-	CS          string
-	TagName     string
-	ProjectName string
-	BranchName  string
-}
-
-func renderReleaseTpl(operation, cs, tagName, projectName, branchName string) (string, error) {
-	t, err := template.New("releaseTmpl").Parse(releaseTmpl)
-	if err != nil {
-		return "", err
-	}
-
-	msg := &releaseMsg{
-		Operation:   operation,
-		CS:          cs,
-		TagName:     tagName,
-		ProjectName: projectName,
-		BranchName:  branchName,
-	}
-
-	var b bytes.Buffer
-	err = t.Execute(&b, msg)
-
-	return b.String(), err
-}

commands/release_tpl_test.go

-package commands
-
-import (
-	"testing"
-
-	"github.com/bmizerany/assert"
-)
-
-func TestRenderReleaseTpl(t *testing.T) {
-	msg, err := renderReleaseTpl("Creating", "#", "1.0", "github/hub", "master")
-	assert.Equal(t, nil, err)
-
-	expMsg := `
-# Creating release 1.0 for github/hub from master
-#
-# Write a message for this release. The first block of
-# text is the title and the rest is the description.`
-	assert.Equal(t, expMsg, msg)
-}

git/git.go

@@ -192,13 +192,13 @@ func (r *Range) IsAncestor() bool {
 	return cmd.Success()
 }
 
-func CommentChar() string {
+func CommentChar(text string) (string, error) {
 	char, err := Config("core.commentchar")
 	if err != nil {
 		char = "#"
 	}
 
-	return char
+	return char, nil
 }
 
 func Show(sha string) (string, error) {

github/editor.go

@@ -26,7 +26,10 @@ func NewEditor(filePrefix, topic, message string) (editor *Editor, err error) {
 		return
 	}
 
-	cs := git.CommentChar()
+	cs, err := git.CommentChar(message)
+	if err != nil {
+		return
+	}
 
 	editor = &Editor{
 		Program:    program,
@@ -49,6 +52,14 @@ type Editor struct {
 	openEditor func(program, file string) error
 }
 
+func (e *Editor) AddCommentedSection(text string) {
+	startRegexp := regexp.MustCompilePOSIX("^")
+	endRegexp := regexp.MustCompilePOSIX(" +$")
+	commentedText := startRegexp.ReplaceAllString(text, e.CS+" ")
+	commentedText = endRegexp.ReplaceAllString(commentedText, "")
+	e.Message = e.Message + "\n" + commentedText
+}
+
 func (e *Editor) DeleteFile() error {
 	return os.Remove(e.File)
 }