Skip to content

Commit 606deaf

Browse files
mislavmislav
committed
Allow setting empty body via editor in issue/pr create 
1 parent f570deb commit 606deaf

File tree

7 files changed

+71
-23
lines changed

7 files changed

+71
-23
lines changed

‎pkg/cmd/issue/create/create.go‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -234,10 +234,6 @@ func createRun(opts *CreateOptions) (err error) {
234234
if err != nil {
235235
return
236236
}
237-
238-
if tb.Body == "" {
239-
tb.Body = templateContent
240-
}
241237
}
242238

243239
openURL, err = generatePreviewURL(apiClient, baseRepo, tb)

‎pkg/cmd/issue/create/create_test.go‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,8 @@ func TestNewCmdCreate(t *testing.T) {
114114
args, err := shlex.Split(tt.cli)
115115
require.NoError(t, err)
116116
cmd.SetArgs(args)
117+
cmd.SetOut(ioutil.Discard)
118+
cmd.SetErr(ioutil.Discard)
117119
_, err = cmd.ExecuteC()
118120
if tt.wantsErr {
119121
assert.Error(t, err)
@@ -168,7 +170,7 @@ func Test_createRun(t *testing.T) {
168170
WebMode: true,
169171
Assignees: []string{"monalisa"},
170172
},
171-
wantsBrowse: "https://github.com/OWNER/REPO/issues/new?assignees=monalisa",
173+
wantsBrowse: "https://github.com/OWNER/REPO/issues/new?assignees=monalisa&body=",
172174
wantsStderr: "Opening github.com/OWNER/REPO/issues/new in your browser.\n",
173175
},
174176
{
@@ -185,7 +187,7 @@ func Test_createRun(t *testing.T) {
185187
"viewer": { "login": "MonaLisa" }
186188
} }`))
187189
},
188-
wantsBrowse: "https://github.com/OWNER/REPO/issues/new?assignees=MonaLisa",
190+
wantsBrowse: "https://github.com/OWNER/REPO/issues/new?assignees=MonaLisa&body=",
189191
wantsStderr: "Opening github.com/OWNER/REPO/issues/new in your browser.\n",
190192
},
191193
{
@@ -214,7 +216,7 @@ func Test_createRun(t *testing.T) {
214216
"pageInfo": { "hasNextPage": false }
215217
} } } }`))
216218
},
217-
wantsBrowse: "https://github.com/OWNER/REPO/issues/new?projects=OWNER%2FREPO%2F1",
219+
wantsBrowse: "https://github.com/OWNER/REPO/issues/new?body=&projects=OWNER%2FREPO%2F1",
218220
wantsStderr: "Opening github.com/OWNER/REPO/issues/new in your browser.\n",
219221
},
220222
{

‎pkg/cmd/pr/create/create.go‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -302,10 +302,6 @@ func createRun(opts *CreateOptions) (err error) {
302302
if err != nil {
303303
return
304304
}
305-
306-
if state.Body == "" {
307-
state.Body = templateContent
308-
}
309305
}
310306

311307
openURL, err = generateCompareURL(*ctx, *state)

‎pkg/cmd/pr/create/create_test.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,7 @@ func TestPRCreate_nontty_web(t *testing.T) {
239239

240240
assert.Equal(t, "", output.String())
241241
assert.Equal(t, "", output.Stderr())
242-
assert.Equal(t, "https://github.com/OWNER/REPO/compare/master...feature?expand=1", output.BrowsedURL)
242+
assert.Equal(t, "https://github.com/OWNER/REPO/compare/master...feature?body=&expand=1", output.BrowsedURL)
243243
}
244244

245245
func TestPRCreate_recover(t *testing.T) {
@@ -780,7 +780,7 @@ func TestPRCreate_web(t *testing.T) {
780780

781781
assert.Equal(t, "", output.String())
782782
assert.Equal(t, "Opening github.com/OWNER/REPO/compare/master...feature in your browser.\n", output.Stderr())
783-
assert.Equal(t, "https://github.com/OWNER/REPO/compare/master...feature?expand=1", output.BrowsedURL)
783+
assert.Equal(t, "https://github.com/OWNER/REPO/compare/master...feature?body=&expand=1", output.BrowsedURL)
784784
}
785785

786786
func TestPRCreate_webLongURL(t *testing.T) {
@@ -851,7 +851,7 @@ func TestPRCreate_webProject(t *testing.T) {
851851

852852
assert.Equal(t, "", output.String())
853853
assert.Equal(t, "Opening github.com/OWNER/REPO/compare/master...feature in your browser.\n", output.Stderr())
854-
assert.Equal(t, "https://github.com/OWNER/REPO/compare/master...feature?expand=1&projects=ORG%2F1", output.BrowsedURL)
854+
assert.Equal(t, "https://github.com/OWNER/REPO/compare/master...feature?body=&expand=1&projects=ORG%2F1", output.BrowsedURL)
855855
}
856856

857857
func Test_determineTrackingBranch_empty(t *testing.T) {
@@ -965,7 +965,7 @@ func Test_generateCompareURL(t *testing.T) {
965965
BaseBranch: "main",
966966
HeadBranchLabel: "feature",
967967
},
968-
want: "https://github.com/OWNER/REPO/compare/main...feature?expand=1",
968+
want: "https://github.com/OWNER/REPO/compare/main...feature?body=&expand=1",
969969
wantErr: false,
970970
},
971971
{
@@ -978,7 +978,7 @@ func Test_generateCompareURL(t *testing.T) {
978978
state: prShared.IssueMetadataState{
979979
Labels: []string{"one", "two three"},
980980
},
981-
want: "https://github.com/OWNER/REPO/compare/a...b?expand=1&labels=one%2Ctwo+three",
981+
want: "https://github.com/OWNER/REPO/compare/a...b?body=&expand=1&labels=one%2Ctwo+three",
982982
wantErr: false,
983983
},
984984
{
@@ -988,7 +988,7 @@ func Test_generateCompareURL(t *testing.T) {
988988
BaseBranch: "main/trunk",
989989
HeadBranchLabel: "owner:feature",
990990
},
991-
want: "https://github.com/OWNER/REPO/compare/main%2Ftrunk...owner%3Afeature?expand=1",
991+
want: "https://github.com/OWNER/REPO/compare/main%2Ftrunk...owner%3Afeature?body=&expand=1",
992992
wantErr: false,
993993
},
994994
}

‎pkg/cmd/pr/shared/params.go‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,13 @@ package shared
22

33
import (
44
"fmt"
5-
"github.com/google/shlex"
65
"net/url"
76
"strings"
87

98
"github.com/cli/cli/api"
109
"github.com/cli/cli/internal/ghrepo"
1110
"github.com/cli/cli/pkg/githubsearch"
11+
"github.com/google/shlex"
1212
)
1313

1414
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
2020
if state.Title != "" {
2121
q.Set("title", state.Title)
2222
}
23-
if state.Body != "" {
24-
q.Set("body", state.Body)
25-
}
23+
// We always want to set body, even if it's empty, to prevent the web interface to apply the default
24+
// template. Since the user has the option to select a template in the terminal, assume that empty body
25+
// here means that the user either skipped it or erased its contents.
26+
q.Set("body", state.Body)
2627
if len(state.Assignees) > 0 {
2728
q.Set("assignees", strings.Join(state.Assignees, ","))
2829
}

‎pkg/cmd/pr/shared/params_test.go‎

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
package shared
22

33
import (
4-
"github.com/stretchr/testify/assert"
54
"net/http"
65
"reflect"
76
"testing"
87

98
"github.com/cli/cli/api"
109
"github.com/cli/cli/internal/ghrepo"
1110
"github.com/cli/cli/pkg/httpmock"
11+
"github.com/stretchr/testify/assert"
1212
)
1313

1414
func Test_listURLWithQuery(t *testing.T) {
@@ -192,3 +192,56 @@ func Test_QueryHasStateClause(t *testing.T) {
192192
assert.Equal(t, tt.hasState, gotState)
193193
}
194194
}
195+
196+
func Test_WithPrAndIssueQueryParams(t *testing.T) {
197+
type args struct {
198+
baseURL string
199+
state IssueMetadataState
200+
}
201+
tests := []struct {
202+
name string
203+
args args
204+
want string
205+
wantErr bool
206+
}{
207+
{
208+
name: "blank",
209+
args: args{
210+
baseURL: "",
211+
state: IssueMetadataState{},
212+
},
213+
want: "?body=",
214+
},
215+
{
216+
name: "no values",
217+
args: args{
218+
baseURL: "http://example.com/hey",
219+
state: IssueMetadataState{},
220+
},
221+
want: "http://example.com/hey?body=",
222+
},
223+
{
224+
name: "title and body",
225+
args: args{
226+
baseURL: "http://example.com/hey",
227+
state: IssueMetadataState{
228+
Title: "my title",
229+
Body: "my bodeh",
230+
},
231+
},
232+
want: "http://example.com/hey?body=my+bodeh&title=my+title",
233+
},
234+
}
235+
for _, tt := range tests {
236+
t.Run(tt.name, func(t *testing.T) {
237+
got, err := WithPrAndIssueQueryParams(nil, nil, tt.args.baseURL, tt.args.state)
238+
if (err != nil) != tt.wantErr {
239+
t.Errorf("WithPrAndIssueQueryParams() error = %v, wantErr %v", err, tt.wantErr)
240+
return
241+
}
242+
if got != tt.want {
243+
t.Errorf("WithPrAndIssueQueryParams() = %v, want %v", got, tt.want)
244+
}
245+
})
246+
}
247+
}

‎pkg/cmd/pr/shared/survey.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ func BodySurvey(state *IssueMetadataState, templateContent, editorCommand string
110110
return err
111111
}
112112

113-
if state.Body != "" && preBody != state.Body {
113+
if preBody != state.Body {
114114
state.MarkDirty()
115115
}
116116

0 commit comments

Comments
 (0)