Allow setting empty body via editor in issue/pr create

This commit is contained in:
Mislav Marohnić 2021-06-04 21:32:54 +02:00
parent f570deb118
commit 606deaf134
7 changed files with 71 additions and 23 deletions

View file

@ -234,10 +234,6 @@ func createRun(opts *CreateOptions) (err error) {
if err != nil { if err != nil {
return return
} }
if tb.Body == "" {
tb.Body = templateContent
}
} }
openURL, err = generatePreviewURL(apiClient, baseRepo, tb) openURL, err = generatePreviewURL(apiClient, baseRepo, tb)

View file

@ -114,6 +114,8 @@ func TestNewCmdCreate(t *testing.T) {
args, err := shlex.Split(tt.cli) args, err := shlex.Split(tt.cli)
require.NoError(t, err) require.NoError(t, err)
cmd.SetArgs(args) cmd.SetArgs(args)
cmd.SetOut(ioutil.Discard)
cmd.SetErr(ioutil.Discard)
_, err = cmd.ExecuteC() _, err = cmd.ExecuteC()
if tt.wantsErr { if tt.wantsErr {
assert.Error(t, err) assert.Error(t, err)
@ -168,7 +170,7 @@ func Test_createRun(t *testing.T) {
WebMode: true, WebMode: true,
Assignees: []string{"monalisa"}, Assignees: []string{"monalisa"},
}, },
wantsBrowse: "https://github.com/OWNER/REPO/issues/new?assignees=monalisa", wantsBrowse: "https://github.com/OWNER/REPO/issues/new?assignees=monalisa&body=",
wantsStderr: "Opening github.com/OWNER/REPO/issues/new in your browser.\n", wantsStderr: "Opening github.com/OWNER/REPO/issues/new in your browser.\n",
}, },
{ {
@ -185,7 +187,7 @@ func Test_createRun(t *testing.T) {
"viewer": { "login": "MonaLisa" } "viewer": { "login": "MonaLisa" }
} }`)) } }`))
}, },
wantsBrowse: "https://github.com/OWNER/REPO/issues/new?assignees=MonaLisa", wantsBrowse: "https://github.com/OWNER/REPO/issues/new?assignees=MonaLisa&body=",
wantsStderr: "Opening github.com/OWNER/REPO/issues/new in your browser.\n", wantsStderr: "Opening github.com/OWNER/REPO/issues/new in your browser.\n",
}, },
{ {
@ -214,7 +216,7 @@ func Test_createRun(t *testing.T) {
"pageInfo": { "hasNextPage": false } "pageInfo": { "hasNextPage": false }
} } } }`)) } } } }`))
}, },
wantsBrowse: "https://github.com/OWNER/REPO/issues/new?projects=OWNER%2FREPO%2F1", wantsBrowse: "https://github.com/OWNER/REPO/issues/new?body=&projects=OWNER%2FREPO%2F1",
wantsStderr: "Opening github.com/OWNER/REPO/issues/new in your browser.\n", wantsStderr: "Opening github.com/OWNER/REPO/issues/new in your browser.\n",
}, },
{ {

View file

@ -302,10 +302,6 @@ func createRun(opts *CreateOptions) (err error) {
if err != nil { if err != nil {
return return
} }
if state.Body == "" {
state.Body = templateContent
}
} }
openURL, err = generateCompareURL(*ctx, *state) openURL, err = generateCompareURL(*ctx, *state)

View file

@ -239,7 +239,7 @@ func TestPRCreate_nontty_web(t *testing.T) {
assert.Equal(t, "", output.String()) assert.Equal(t, "", output.String())
assert.Equal(t, "", output.Stderr()) assert.Equal(t, "", output.Stderr())
assert.Equal(t, "https://github.com/OWNER/REPO/compare/master...feature?expand=1", output.BrowsedURL) assert.Equal(t, "https://github.com/OWNER/REPO/compare/master...feature?body=&expand=1", output.BrowsedURL)
} }
func TestPRCreate_recover(t *testing.T) { func TestPRCreate_recover(t *testing.T) {
@ -780,7 +780,7 @@ func TestPRCreate_web(t *testing.T) {
assert.Equal(t, "", output.String()) assert.Equal(t, "", output.String())
assert.Equal(t, "Opening github.com/OWNER/REPO/compare/master...feature in your browser.\n", output.Stderr()) assert.Equal(t, "Opening github.com/OWNER/REPO/compare/master...feature in your browser.\n", output.Stderr())
assert.Equal(t, "https://github.com/OWNER/REPO/compare/master...feature?expand=1", output.BrowsedURL) assert.Equal(t, "https://github.com/OWNER/REPO/compare/master...feature?body=&expand=1", output.BrowsedURL)
} }
func TestPRCreate_webLongURL(t *testing.T) { func TestPRCreate_webLongURL(t *testing.T) {
@ -851,7 +851,7 @@ func TestPRCreate_webProject(t *testing.T) {
assert.Equal(t, "", output.String()) assert.Equal(t, "", output.String())
assert.Equal(t, "Opening github.com/OWNER/REPO/compare/master...feature in your browser.\n", output.Stderr()) assert.Equal(t, "Opening github.com/OWNER/REPO/compare/master...feature in your browser.\n", output.Stderr())
assert.Equal(t, "https://github.com/OWNER/REPO/compare/master...feature?expand=1&projects=ORG%2F1", output.BrowsedURL) assert.Equal(t, "https://github.com/OWNER/REPO/compare/master...feature?body=&expand=1&projects=ORG%2F1", output.BrowsedURL)
} }
func Test_determineTrackingBranch_empty(t *testing.T) { func Test_determineTrackingBranch_empty(t *testing.T) {
@ -965,7 +965,7 @@ func Test_generateCompareURL(t *testing.T) {
BaseBranch: "main", BaseBranch: "main",
HeadBranchLabel: "feature", HeadBranchLabel: "feature",
}, },
want: "https://github.com/OWNER/REPO/compare/main...feature?expand=1", want: "https://github.com/OWNER/REPO/compare/main...feature?body=&expand=1",
wantErr: false, wantErr: false,
}, },
{ {
@ -978,7 +978,7 @@ func Test_generateCompareURL(t *testing.T) {
state: prShared.IssueMetadataState{ state: prShared.IssueMetadataState{
Labels: []string{"one", "two three"}, Labels: []string{"one", "two three"},
}, },
want: "https://github.com/OWNER/REPO/compare/a...b?expand=1&labels=one%2Ctwo+three", want: "https://github.com/OWNER/REPO/compare/a...b?body=&expand=1&labels=one%2Ctwo+three",
wantErr: false, wantErr: false,
}, },
{ {
@ -988,7 +988,7 @@ func Test_generateCompareURL(t *testing.T) {
BaseBranch: "main/trunk", BaseBranch: "main/trunk",
HeadBranchLabel: "owner:feature", HeadBranchLabel: "owner:feature",
}, },
want: "https://github.com/OWNER/REPO/compare/main%2Ftrunk...owner%3Afeature?expand=1", want: "https://github.com/OWNER/REPO/compare/main%2Ftrunk...owner%3Afeature?body=&expand=1",
wantErr: false, wantErr: false,
}, },
} }

View file

@ -2,13 +2,13 @@ package shared
import ( import (
"fmt" "fmt"
"github.com/google/shlex"
"net/url" "net/url"
"strings" "strings"
"github.com/cli/cli/api" "github.com/cli/cli/api"
"github.com/cli/cli/internal/ghrepo" "github.com/cli/cli/internal/ghrepo"
"github.com/cli/cli/pkg/githubsearch" "github.com/cli/cli/pkg/githubsearch"
"github.com/google/shlex"
) )
func WithPrAndIssueQueryParams(client *api.Client, baseRepo ghrepo.Interface, baseURL string, state IssueMetadataState) (string, error) { func WithPrAndIssueQueryParams(client *api.Client, baseRepo ghrepo.Interface, baseURL string, state IssueMetadataState) (string, error) {
@ -20,9 +20,10 @@ func WithPrAndIssueQueryParams(client *api.Client, baseRepo ghrepo.Interface, ba
if state.Title != "" { if state.Title != "" {
q.Set("title", state.Title) q.Set("title", state.Title)
} }
if state.Body != "" { // We always want to set body, even if it's empty, to prevent the web interface to apply the default
q.Set("body", state.Body) // template. Since the user has the option to select a template in the terminal, assume that empty body
} // here means that the user either skipped it or erased its contents.
q.Set("body", state.Body)
if len(state.Assignees) > 0 { if len(state.Assignees) > 0 {
q.Set("assignees", strings.Join(state.Assignees, ",")) q.Set("assignees", strings.Join(state.Assignees, ","))
} }

View file

@ -1,7 +1,6 @@
package shared package shared
import ( import (
"github.com/stretchr/testify/assert"
"net/http" "net/http"
"reflect" "reflect"
"testing" "testing"
@ -9,6 +8,7 @@ import (
"github.com/cli/cli/api" "github.com/cli/cli/api"
"github.com/cli/cli/internal/ghrepo" "github.com/cli/cli/internal/ghrepo"
"github.com/cli/cli/pkg/httpmock" "github.com/cli/cli/pkg/httpmock"
"github.com/stretchr/testify/assert"
) )
func Test_listURLWithQuery(t *testing.T) { func Test_listURLWithQuery(t *testing.T) {
@ -192,3 +192,56 @@ func Test_QueryHasStateClause(t *testing.T) {
assert.Equal(t, tt.hasState, gotState) assert.Equal(t, tt.hasState, gotState)
} }
} }
func Test_WithPrAndIssueQueryParams(t *testing.T) {
type args struct {
baseURL string
state IssueMetadataState
}
tests := []struct {
name string
args args
want string
wantErr bool
}{
{
name: "blank",
args: args{
baseURL: "",
state: IssueMetadataState{},
},
want: "?body=",
},
{
name: "no values",
args: args{
baseURL: "http://example.com/hey",
state: IssueMetadataState{},
},
want: "http://example.com/hey?body=",
},
{
name: "title and body",
args: args{
baseURL: "http://example.com/hey",
state: IssueMetadataState{
Title: "my title",
Body: "my bodeh",
},
},
want: "http://example.com/hey?body=my+bodeh&title=my+title",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := WithPrAndIssueQueryParams(nil, nil, tt.args.baseURL, tt.args.state)
if (err != nil) != tt.wantErr {
t.Errorf("WithPrAndIssueQueryParams() error = %v, wantErr %v", err, tt.wantErr)
return
}
if got != tt.want {
t.Errorf("WithPrAndIssueQueryParams() = %v, want %v", got, tt.want)
}
})
}
}

View file

@ -110,7 +110,7 @@ func BodySurvey(state *IssueMetadataState, templateContent, editorCommand string
return err return err
} }
if state.Body != "" && preBody != state.Body { if preBody != state.Body {
state.MarkDirty() state.MarkDirty()
} }