cmd/edit: unify description edition functions

Instead of having two different functions for editing MR and issue
descriptions based on the sources (command line or filename), use the
same function but different args. The actions are simple, so keeping
them together is simpler to maintain and understand. In case the
function require additional logic for new things we may split them
again later.

Signed-off-by: Bruno Meneguele <[email protected]>

cmd/edit_common.go

@@ -4,6 +4,8 @@ import (
 	"bytes"
 	"fmt"
 	"io/ioutil"
+	"runtime"
+	"strconv"
 	"strings"
 	"text/template"
 
@@ -38,43 +40,45 @@ func getUpdateUsers(currentUsers []string, users []string, remove []string) ([]i
 	return userIDs, usersChanged, nil
 }
 
-// editGetTitleDescription returns a title and description based on the
+// editDescription returns a title and description based on the
 // current issue title and description and various flags from the command
 // line
-func editGetTitleDescription(title string, body string, msgs []string) (string, string, error) {
+func editDescription(title string, body string, msgs []string, filename string) (string, string, error) {
 	if len(msgs) > 0 {
 		title = msgs[0]
 
 		if len(msgs) > 1 {
 			body = strings.Join(msgs[1:], "\n\n")
 		}
 
-		// we have everything we need
+		return title, body, nil
+	}
+
+	if filename != "" {
+		var lines []string
+
+		content, err := ioutil.ReadFile(filename)
+		if err != nil {
+			return "", "", nil
+		}
+		lines = strings.Split(string(content), "\n")
+
+		title = lines[0]
+		body = strings.Join(lines[1:], "\n")
+
 		return title, body, nil
 	}
 
 	text, err := editText(title, body)
 	if err != nil {
 		return "", "", err
 	}
-	return git.Edit("EDIT", text)
-}
 
-// editGetTitleDescFromFile returns the new title and description based on
-// the content of a file. The first line is considered the title, the
-// remaining is the description.
-func editGetTitleDescFromFile(filename string) (string, string, error) {
-	var title, body string
-	var lines []string
-
-	content, err := ioutil.ReadFile(filename)
+	title, body, err = git.Edit("EDIT", text)
 	if err != nil {
-		return "", "", nil
+		_, f, l, _ := runtime.Caller(0)
+		log.Fatal(f+":"+strconv.Itoa(l)+" ", err)
 	}
-	lines = strings.Split(string(content), "\n")
-
-	title = lines[0]
-	body = strings.Join(lines[1:], "\n")
 
 	return title, body, nil
 }

cmd/edit_common_test.go

@@ -1,57 +1,93 @@
 package cmd
 
 import (
+	"path/filepath"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	gitlab "github.com/xanzy/go-gitlab"
 )
 
-func Test_editGetTitleAndDescription(t *testing.T) {
+func Test_editDescription(t *testing.T) {
+	repo := copyTestRepo(t)
+
+	type fakeGLObj struct {
+		Title       string
+		Description string
+	}
+
+	type funcArgs struct {
+		Msgs     []string
+		Filename string
+	}
+
 	tests := []struct {
 		Name                string
-		Issue               *gitlab.Issue
-		Args                []string
+		GLObj               fakeGLObj
+		Args                funcArgs
 		ExpectedTitle       string
 		ExpectedDescription string
 	}{
 		{
 			Name: "Using messages",
-			Issue: &gitlab.Issue{
+			GLObj: fakeGLObj{
 				Title:       "old title",
 				Description: "old body",
 			},
-			Args:                []string{"new title", "new body 1", "new body 2"},
+			Args: funcArgs{
+				Msgs:     []string{"new title", "new body 1", "new body 2"},
+				Filename: "",
+			},
 			ExpectedTitle:       "new title",
 			ExpectedDescription: "new body 1\n\nnew body 2",
 		},
 		{
 			Name: "Using a single message",
-			Issue: &gitlab.Issue{
+			GLObj: fakeGLObj{
 				Title:       "old title",
 				Description: "old body",
 			},
-			Args:                []string{"new title"},
+			Args: funcArgs{
+				Msgs:     []string{"new title"},
+				Filename: "",
+			},
 			ExpectedTitle:       "new title",
 			ExpectedDescription: "old body",
 		},
 		{
 			Name: "From Editor",
-			Issue: &gitlab.Issue{
+			GLObj: fakeGLObj{
 				Title:       "old title",
 				Description: "old body",
 			},
-			Args:                nil,
+			Args: funcArgs{
+				Msgs:     nil,
+				Filename: "",
+			},
 			ExpectedTitle:       "old title",
 			ExpectedDescription: "old body",
 		},
+		{
+			Name: "From file",
+			GLObj: fakeGLObj{
+				Title:       "old title",
+				Description: "old body",
+			},
+			Args: funcArgs{
+				Msgs:     nil,
+				Filename: filepath.Join(repo, "testedit"),
+			},
+			ExpectedTitle:       "new title",
+			ExpectedDescription: "\nnew body\n",
+		},
 	}
+
 	for _, test := range tests {
 		t.Run(test.Name, func(t *testing.T) {
 			test := test
 			t.Parallel()
-			title, body, err := editGetTitleDescription(test.Issue.Title, test.Issue.Description, test.Args, len(test.Args))
+			title, body, err := editDescription(test.GLObj.Title,
+				test.GLObj.Description, test.Args.Msgs, test.Args.Filename)
 			if err != nil {
 				t.Fatal(err)
 			}

cmd/issue_edit.go

@@ -2,7 +2,6 @@ package cmd
 
 import (
 	"fmt"
-	"runtime"
 	"strconv"
 	"strings"
 
@@ -137,10 +136,9 @@ var issueEditCmd = &cobra.Command{
 		// We only consider editing an issue with -m or when no other flag is
 		// passed, but --linebreak.
 		if len(msgs) > 0 || cmd.Flags().NFlag() == 0 || (cmd.Flags().NFlag() == 1 && linebreak) {
-			title, body, err = editGetTitleDescription(issue.Title, issue.Description, msgs)
+			title, body, err = editDescription(issue.Title, issue.Description, msgs, "")
 			if err != nil {
-				_, f, l, _ := runtime.Caller(0)
-				log.Fatal(f+":"+strconv.Itoa(l)+" ", err)
+				log.Fatal(err)
 			}
 			if title == "" {
 				log.Fatal("aborting: empty issue title")

cmd/mr_create.go

@@ -243,7 +243,7 @@ func runMRCreate(cmd *cobra.Command, args []string) {
 			log.Fatal("option -F cannot be combined with -m/-c")
 		}
 
-		title, body, err = editGetTitleDescFromFile(filename)
+		title, body, err = editDescription("", "", nil, filename)
 		if err != nil {
 			log.Fatal(err)
 		}

cmd/mr_edit.go

@@ -3,8 +3,6 @@ package cmd
 import (
 	"fmt"
 	"os"
-	"runtime"
-	"strconv"
 	"strings"
 
 	"github.com/MakeNowJust/heredoc/v2"
@@ -192,29 +190,25 @@ var mrEditCmd = &cobra.Command{
 		title := mr.Title
 		body := mr.Description
 
-		if filename != "" {
-			if len(msgs) > 0 {
-				log.Fatal("option -F cannot be combined with -m")
-			}
+		if len(msgs) > 0 && filename != "" {
+			log.Fatal("option -F cannot be combined with -m")
+		}
 
-			title, body, err = editGetTitleDescFromFile(filename)
+		// We only consider editing title and body on -m, -F or when no other
+		// flag is passed, but --linebreak.
+		if len(msgs) > 0 || filename != "" ||
+			cmd.Flags().NFlag() == 0 || (cmd.Flags().NFlag() == 1 && linebreak) {
+			title, body, err = editDescription(mr.Title, mr.Description, msgs, filename)
 			if err != nil {
 				log.Fatal(err)
 			}
-		} else {
-			// We only consider editing an mr with -m, -F or when no other flag
-			// is passed, but --linebreak.
-			if len(msgs) > 0 || cmd.Flags().NFlag() == 0 || (cmd.Flags().NFlag() == 1 && linebreak) {
-				title, body, err = editGetTitleDescription(mr.Title, mr.Description, msgs)
-				if err != nil {
-					_, f, l, _ := runtime.Caller(0)
-					log.Fatal(f+":"+strconv.Itoa(l)+" ", err)
-				}
-			}
-
 			if title == "" {
 				log.Fatal("aborting: empty mr title")
 			}
+
+			if linebreak {
+				body = textToMarkdown(body)
+			}
 		}
 
 		isWIP := hasPrefix(title, "wip:") ||
@@ -256,10 +250,6 @@ var mrEditCmd = &cobra.Command{
 			log.Fatal("aborting: no changes")
 		}
 
-		if linebreak {
-			body = textToMarkdown(body)
-		}
-
 		opts := &gitlab.UpdateMergeRequestOptions{
 			Title:       &title,
 			Description: &body,

testdata/testedit

@@ -0,0 +1,3 @@
+new title
+
+new body