From 13a606dca2f587fd0b3a012e5640a13ea1839f3e Mon Sep 17 00:00:00 2001
From: Travis DePrato <773453+travigd@users.noreply.github.com>
Date: Wed, 18 Oct 2023 13:58:20 -0700
Subject: [PATCH] fix: Don't erase PR message if editor exits with a non-zero
 code (#204)

This has bit me a few times when flailing with vim. The file is saved successfully but Vim exits with a non-zero exit code and I've managed to lose big walls of text this way.

Going to test by causing vim to fail when creating this very PR.

It works!

```
Creating pull request for branch travis/mer-3040-av-cli-dont-erase-pr-message-if-editor-exits-non-zero:
  - pushing to origin/travis/mer-3040-av-cli-dont-erase-pr-message-if-editor-exits-non-zero
  - saved pull request description to /Users/travis/Aviator/av/.git/av/tmp/av-pr-travis-mer-3040-av-cli-dont-erase-pr-message-if-editor-exits-non-zero.md (it will be automatically re-used if you try again)
error: text editor failed: command "vi" failed: exit status 2
```
---
 cmd/av/helpers.go            | 16 +++++++++++-----
 internal/actions/pr.go       | 28 ++++++++++++++++++----------
 internal/editor/editor.go    | 23 ++++++++++++++++++-----
 internal/git/git.go          |  4 ++--
 internal/git/gittest/repo.go |  2 +-
 5 files changed, 50 insertions(+), 23 deletions(-)

diff --git a/cmd/av/helpers.go b/cmd/av/helpers.go
index cbfefc46..a13a2908 100644
--- a/cmd/av/helpers.go
+++ b/cmd/av/helpers.go
@@ -18,7 +18,13 @@ var cachedRepo *git.Repo
 
 func getRepo() (*git.Repo, error) {
 	if cachedRepo == nil {
-		cmd := exec.Command("git", "rev-parse", "--path-format=absolute", "--show-toplevel", "--git-common-dir")
+		cmd := exec.Command(
+			"git",
+			"rev-parse",
+			"--path-format=absolute",
+			"--show-toplevel",
+			"--git-common-dir",
+		)
 
 		if rootFlags.Directory != "" {
 			cmd.Dir = rootFlags.Directory
@@ -31,10 +37,10 @@ func getRepo() (*git.Repo, error) {
 			)
 		}
 
-        dir, gitDir, found := strings.Cut(strings.TrimSpace(string(paths)), "\n" )
-        if !found {
-            return nil, errors.New("Unexpected format, not able to parse toplevel and common dir.")
-        }
+		dir, gitDir, found := strings.Cut(strings.TrimSpace(string(paths)), "\n")
+		if !found {
+			return nil, errors.New("Unexpected format, not able to parse toplevel and common dir.")
+		}
 
 		cachedRepo, err = git.OpenRepo(dir, gitDir)
 		if err != nil {
diff --git a/internal/actions/pr.go b/internal/actions/pr.go
index 39bb3e60..4caea552 100644
--- a/internal/actions/pr.go
+++ b/internal/actions/pr.go
@@ -288,7 +288,11 @@ func CreatePullRequest(
 			CommentPrefix:  "%%",
 		})
 		if err != nil {
-			return nil, errors.WrapIf(err, "failed to launch text editor")
+			if res != "" {
+				savePRDescriptionToTemporaryFile(saveFile, res)
+			}
+			return nil, errors.WrapIf(err, "text editor failed")
+
 		}
 		opts.Title, opts.Body = stringutils.ParseSubjectBody(res)
 		if opts.Title == "" {
@@ -305,15 +309,7 @@ func CreatePullRequest(
 
 			// Otherwise, save what the user entered to a file so that it's not
 			// lost forever (and we can re-use it if they try again).
-			if err := os.WriteFile(saveFile, []byte(res), 0644); err != nil {
-				logrus.WithError(err).
-					Error("failed to write pull request description to temporary file")
-				return
-			}
-			_, _ = fmt.Fprint(os.Stderr,
-				"  - saved pull request description to ", colors.UserInput(saveFile),
-				" (it will be automatically re-used if you try again)\n",
-			)
+			savePRDescriptionToTemporaryFile(saveFile, res)
 		}()
 	}
 
@@ -376,6 +372,18 @@ func CreatePullRequest(
 	return &CreatePullRequestResult{didCreatePR, branchMeta, pull}, nil
 }
 
+func savePRDescriptionToTemporaryFile(saveFile string, contents string) {
+	if err := os.WriteFile(saveFile, []byte(contents), 0644); err != nil {
+		logrus.WithError(err).
+			Error("failed to write pull request description to temporary file")
+		return
+	}
+	_, _ = fmt.Fprint(os.Stderr,
+		"  - saved pull request description to ", colors.UserInput(saveFile),
+		" (it will be automatically re-used if you try again)\n",
+	)
+}
+
 type prBodyTemplateData struct {
 	Branch  string
 	Title   string
diff --git a/internal/editor/editor.go b/internal/editor/editor.go
index 2c2df097..da21e2ff 100644
--- a/internal/editor/editor.go
+++ b/internal/editor/editor.go
@@ -6,6 +6,7 @@ import (
 	"os"
 	"os/exec"
 	"strings"
+	"time"
 
 	"emperror.dev/errors"
 	"github.com/aviator-co/av/internal/git"
@@ -35,6 +36,10 @@ type Config struct {
 // https://github.com/git/git/blob/5699ec1b0aec51b9e9ba5a2785f65970c5a95d84/editor.c#L57
 const CommandNoOp = ":"
 
+// Launch launches the user's editor and allows them to edit the text.
+// The text is returned after the editor is closed. If an error occurs, the
+// (possibly edited) text is returned in addition to the error. If the file
+// could not be read, an empty string is returned.
 func Launch(repo *git.Repo, config Config) (string, error) {
 	switch {
 	case config.Command == "":
@@ -80,11 +85,11 @@ func Launch(repo *git.Repo, config Config) (string, error) {
 	cmd.Stderr = stderr
 	logrus.WithField("cmd", cmd.String()).Debug("launching editor")
 	if err := cmd.Run(); err != nil {
-		logrus.WithError(err).WithFields(logrus.Fields{
-			"cmd": cmd.String(),
-			"out": stderr.String(),
-		}).Warn("editor exited with error")
-		return "", err
+		// Try to return the contents of the file even if the editor exited with
+		// an error. We ignore any errors from parsing here since we'll just end
+		// up returning the error from above anyway.
+		res, _ := parseResult(tmp.Name(), config)
+		return res, errors.WrapIff(err, "command %q failed", config.Command)
 	}
 
 	return parseResult(tmp.Name(), config)
@@ -123,3 +128,11 @@ func parseResult(path string, config Config) (string, error) {
 	}
 	return res.String(), nil
 }
+
+func mtime(f *os.File) (time.Time, error) {
+	stat, err := f.Stat()
+	if err != nil {
+		return time.Time{}, err
+	}
+	return stat.ModTime(), nil
+}
diff --git a/internal/git/git.go b/internal/git/git.go
index 5e04094a..aec405e1 100644
--- a/internal/git/git.go
+++ b/internal/git/git.go
@@ -20,14 +20,14 @@ var ErrRemoteNotFound = errors.Sentinel("this repository doesn't have a remote o
 
 type Repo struct {
 	repoDir string
-	gitDir string
+	gitDir  string
 	log     logrus.FieldLogger
 }
 
 func OpenRepo(repoDir string, gitDir string) (*Repo, error) {
 	r := &Repo{
 		repoDir,
-        gitDir,
+		gitDir,
 		logrus.WithFields(logrus.Fields{"repo": path.Base(repoDir)}),
 	}
 
diff --git a/internal/git/gittest/repo.go b/internal/git/gittest/repo.go
index d7db5e90..0abb07c4 100644
--- a/internal/git/gittest/repo.go
+++ b/internal/git/gittest/repo.go
@@ -3,7 +3,7 @@ package gittest
 import (
 	"os"
 	"os/exec"
-    "path"
+	"path"
 	"path/filepath"
 	"testing"