Skip to content

Commit

Permalink
fix: Don't erase PR message if editor exits with a non-zero code (#204)
Browse files Browse the repository at this point in the history
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
```
  • Loading branch information
twavv authored Oct 18, 2023
1 parent 7923ddf commit 13a606d
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 23 deletions.
16 changes: 11 additions & 5 deletions cmd/av/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down
28 changes: 18 additions & 10 deletions internal/actions/pr.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 == "" {
Expand All @@ -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)
}()
}

Expand Down Expand Up @@ -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
Expand Down
23 changes: 18 additions & 5 deletions internal/editor/editor.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"os"
"os/exec"
"strings"
"time"

"emperror.dev/errors"
"github.com/aviator-co/av/internal/git"
Expand Down Expand Up @@ -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 == "":
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
}
4 changes: 2 additions & 2 deletions internal/git/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)}),
}

Expand Down
2 changes: 1 addition & 1 deletion internal/git/gittest/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package gittest
import (
"os"
"os/exec"
"path"
"path"
"path/filepath"
"testing"

Expand Down

0 comments on commit 13a606d

Please sign in to comment.