Refactor CopilotReplacer logic based on PR feedback
Thanks to @bagtoad, this commit refactors the argument to `NewCopilotReplacer(bool)` from being where this is used to what it effect is has. Because there is already precedence for display name being `<login> (<name>)`, I worried there would be confusion for `Copilot (AI)` being display name for assignees and reviewers but `Copilot` when going to GitHub.com UI. Instead, I renamed the argument based on whether the login is returned / replaced.
This commit is contained in:
parent
e2bed653df
commit
4f0a5807bf
4 changed files with 23 additions and 20 deletions
|
|
@ -177,9 +177,10 @@ func createRun(opts *CreateOptions) (err error) {
|
|||
}
|
||||
|
||||
// Replace special values in assignees
|
||||
// For web mode, @copilot should be replaced by name; otherwise, login.
|
||||
assigneeSet := set.NewStringSet()
|
||||
meReplacer := prShared.NewMeReplacer(apiClient, baseRepo.RepoHost())
|
||||
copilotReplacer := prShared.NewCopilotReplacer(opts.WebMode)
|
||||
copilotReplacer := prShared.NewCopilotReplacer(!opts.WebMode)
|
||||
assignees, err := meReplacer.ReplaceSlice(opts.Assignees)
|
||||
if err != nil {
|
||||
return err
|
||||
|
|
|
|||
|
|
@ -118,7 +118,7 @@ func (e Editable) AssigneeIds(client *api.Client, repo ghrepo.Interface) (*[]str
|
|||
// curate the final list of assignees from the default list.
|
||||
if len(e.Assignees.Add) != 0 || len(e.Assignees.Remove) != 0 {
|
||||
meReplacer := NewMeReplacer(client, repo.RepoHost())
|
||||
copilotReplacer := NewCopilotReplacer(false)
|
||||
copilotReplacer := NewCopilotReplacer(true)
|
||||
|
||||
replaceSpecialAssigneeNames := func(value []string) ([]string, error) {
|
||||
replaced, err := meReplacer.ReplaceSlice(value)
|
||||
|
|
|
|||
|
|
@ -285,14 +285,15 @@ func (r *MeReplacer) ReplaceSlice(handles []string) ([]string, error) {
|
|||
return res, nil
|
||||
}
|
||||
|
||||
// CopilotReplacer resolves usages of `@copilot` to Copilot's login.
|
||||
// CopilotReplacer resolves usages of `@copilot` to either Copilot's login or name.
|
||||
// Login is generally needed for API calls; name is used when launching web browser.
|
||||
type CopilotReplacer struct {
|
||||
webMode bool
|
||||
returnLogin bool
|
||||
}
|
||||
|
||||
func NewCopilotReplacer(webMode bool) *CopilotReplacer {
|
||||
func NewCopilotReplacer(returnLogin bool) *CopilotReplacer {
|
||||
return &CopilotReplacer{
|
||||
webMode: webMode,
|
||||
returnLogin: returnLogin,
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -300,11 +301,10 @@ func (r *CopilotReplacer) replace(handle string) string {
|
|||
if !strings.EqualFold(handle, "@copilot") {
|
||||
return handle
|
||||
}
|
||||
// When Copilot is used as an assignee in web mode, the login is not used.
|
||||
if r.webMode {
|
||||
return "copilot"
|
||||
if r.returnLogin {
|
||||
return api.CopilotActorLogin
|
||||
}
|
||||
return api.CopilotActorLogin
|
||||
return "copilot"
|
||||
}
|
||||
|
||||
// ReplaceSlice replaces usages of `@copilot` in a slice with Copilot's login.
|
||||
|
|
|
|||
|
|
@ -192,21 +192,21 @@ func TestCopilotReplacer_ReplaceSlice(t *testing.T) {
|
|||
handles []string
|
||||
}
|
||||
tests := []struct {
|
||||
name string
|
||||
webMode bool
|
||||
args args
|
||||
want []string
|
||||
name string
|
||||
returnLogin bool
|
||||
args args
|
||||
want []string
|
||||
}{
|
||||
{
|
||||
name: "replaces @copilot with copilot-swe-agent for non-web mode",
|
||||
name: "replaces @copilot with copilot-swe-agent for non-web mode",
|
||||
returnLogin: true,
|
||||
args: args{
|
||||
handles: []string{"monalisa", "@copilot", "hubot"},
|
||||
},
|
||||
want: []string{"monalisa", "copilot-swe-agent", "hubot"},
|
||||
},
|
||||
{
|
||||
name: "replaces @copilot with copilot for web mode",
|
||||
webMode: true,
|
||||
name: "replaces @copilot with copilot for web mode",
|
||||
args: args{
|
||||
handles: []string{"monalisa", "@copilot", "hubot"},
|
||||
},
|
||||
|
|
@ -220,14 +220,16 @@ func TestCopilotReplacer_ReplaceSlice(t *testing.T) {
|
|||
want: []string{"monalisa", "user", "hubot"},
|
||||
},
|
||||
{
|
||||
name: "replaces multiple @copilot mentions",
|
||||
name: "replaces multiple @copilot mentions",
|
||||
returnLogin: true,
|
||||
args: args{
|
||||
handles: []string{"@copilot", "user", "@copilot"},
|
||||
},
|
||||
want: []string{"copilot-swe-agent", "user", "copilot-swe-agent"},
|
||||
},
|
||||
{
|
||||
name: "handles @copilot case-insensitively",
|
||||
name: "handles @copilot case-insensitively",
|
||||
returnLogin: true,
|
||||
args: args{
|
||||
handles: []string{"@Copilot", "user", "@CoPiLoT"},
|
||||
},
|
||||
|
|
@ -250,7 +252,7 @@ func TestCopilotReplacer_ReplaceSlice(t *testing.T) {
|
|||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
r := NewCopilotReplacer(tt.webMode)
|
||||
r := NewCopilotReplacer(tt.returnLogin)
|
||||
got := r.ReplaceSlice(tt.args.handles)
|
||||
require.Equal(t, tt.want, got)
|
||||
})
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue