Merge remote-tracking branch 'origin' into extensions-remove-notice

This commit is contained in:
Mislav Marohnić 2021-07-16 15:12:56 +02:00
commit 882f6d33cb
17 changed files with 424 additions and 54 deletions

View file

@ -23,7 +23,7 @@ jobs:
- name: Run GoReleaser
uses: goreleaser/goreleaser-action@v2
with:
version: v0.169.0 # pinning bc our config breaks on latest
version: v0.172.1 # pinning to prevent breaking on latest
args: release --release-notes=CHANGELOG.md
env:
GITHUB_TOKEN: ${{secrets.GITHUB_TOKEN}}

View file

@ -64,5 +64,6 @@ nfpms:
formats:
- deb
- rpm
files:
"./share/man/man1/gh*.1": "/usr/share/man/man1"
contents:
- src: "/share/man/man1/gh*.1"
dst: "/usr/share/man/man1"

View file

@ -68,13 +68,18 @@ func (cm *ConfigMap) FindEntry(key string) (ce *ConfigEntry, err error) {
ce = &ConfigEntry{}
topLevelKeys := cm.Root.Content
for i, v := range topLevelKeys {
// Content slice goes [key1, value1, key2, value2, ...]
topLevelPairs := cm.Root.Content
for i, v := range topLevelPairs {
// Skip every other slice item since we only want to check against keys
if i%2 != 0 {
continue
}
if v.Value == key {
ce.KeyNode = v
ce.Index = i
if i+1 < len(topLevelKeys) {
ce.ValueNode = topLevelKeys[i+1]
if i+1 < len(topLevelPairs) {
ce.ValueNode = topLevelPairs[i+1]
}
return
}

View file

@ -0,0 +1,65 @@
package config
import (
"fmt"
"testing"
"github.com/stretchr/testify/assert"
"gopkg.in/yaml.v3"
)
func TestFindEntry(t *testing.T) {
tests := []struct {
name string
key string
output string
wantErr bool
}{
{
name: "find key",
key: "valid",
output: "present",
},
{
name: "find key that is not present",
key: "invalid",
wantErr: true,
},
{
name: "find key with blank value",
key: "blank",
output: "",
},
{
name: "find key that has same content as a value",
key: "same",
output: "logical",
},
}
for _, tt := range tests {
cm := ConfigMap{Root: testYaml()}
t.Run(tt.name, func(t *testing.T) {
out, err := cm.FindEntry(tt.key)
if tt.wantErr {
assert.EqualError(t, err, "not found")
return
}
assert.NoError(t, err)
fmt.Println(out)
assert.Equal(t, tt.output, out.ValueNode.Value)
})
}
}
func testYaml() *yaml.Node {
var root yaml.Node
var data = `
valid: present
erroneous: same
blank:
same: logical
`
_ = yaml.Unmarshal([]byte(data), &root)
return root.Content[0]
}

View file

@ -26,10 +26,11 @@ type BrowseOptions struct {
SelectorArg string
Branch string
ProjectsFlag bool
SettingsFlag bool
WikiFlag bool
Branch string
ProjectsFlag bool
SettingsFlag bool
WikiFlag bool
NoBrowserFlag bool
}
func NewCmdBrowse(f *cmdutil.Factory, runF func(*BrowseOptions) error) *cobra.Command {
@ -99,6 +100,7 @@ func NewCmdBrowse(f *cmdutil.Factory, runF func(*BrowseOptions) error) *cobra.Co
cmd.Flags().BoolVarP(&opts.ProjectsFlag, "projects", "p", false, "Open repository projects")
cmd.Flags().BoolVarP(&opts.WikiFlag, "wiki", "w", false, "Open repository wiki")
cmd.Flags().BoolVarP(&opts.SettingsFlag, "settings", "s", false, "Open repository settings")
cmd.Flags().BoolVarP(&opts.NoBrowserFlag, "no-browser", "n", false, "Print destination URL instead of opening the browser")
cmd.Flags().StringVarP(&opts.Branch, "branch", "b", "", "Select another branch by passing in the branch name")
return cmd
@ -151,10 +153,15 @@ func runBrowse(opts *BrowseOptions) error {
}
}
if opts.IO.IsStdoutTTY() {
fmt.Fprintf(opts.IO.Out, "now opening %s in browser\n", url)
if opts.NoBrowserFlag {
fmt.Fprintf(opts.IO.Out, "%s\n", url)
return nil
} else {
if opts.IO.IsStdoutTTY() {
fmt.Fprintf(opts.IO.Out, "now opening %s in browser\n", url)
}
return opts.Browser.Browse(url)
}
return opts.Browser.Browse(url)
}
func parseFileArg(fileArg string) (string, error) {

View file

@ -1,6 +1,7 @@
package browse
import (
"fmt"
"net/http"
"testing"
@ -49,6 +50,14 @@ func TestNewCmdBrowse(t *testing.T) {
},
wantsErr: false,
},
{
name: "no browser flag",
cli: "--no-browser",
wants: BrowseOptions{
NoBrowserFlag: true,
},
wantsErr: false,
},
{
name: "branch flag",
cli: "--branch main",
@ -118,6 +127,7 @@ func TestNewCmdBrowse(t *testing.T) {
assert.Equal(t, tt.wants.SelectorArg, opts.SelectorArg)
assert.Equal(t, tt.wants.ProjectsFlag, opts.ProjectsFlag)
assert.Equal(t, tt.wants.WikiFlag, opts.WikiFlag)
assert.Equal(t, tt.wants.NoBrowserFlag, opts.NoBrowserFlag)
assert.Equal(t, tt.wants.SettingsFlag, opts.SettingsFlag)
})
}
@ -233,6 +243,17 @@ func Test_runBrowse(t *testing.T) {
wantsErr: false,
expectedURL: "https://github.com/github/ThankYouGitHub/tree/first-browse-pull/browse.go#L32",
},
{
name: "no browser with branch file and line number",
opts: BrowseOptions{
Branch: "3-0-stable",
SelectorArg: "init.rb:6",
NoBrowserFlag: true,
},
baseRepo: ghrepo.New("mislav", "will_paginate"),
wantsErr: false,
expectedURL: "https://github.com/mislav/will_paginate/tree/3-0-stable/init.rb#L6",
},
}
for _, tt := range tests {
@ -263,9 +284,15 @@ func Test_runBrowse(t *testing.T) {
assert.NoError(t, err)
}
assert.Equal(t, "", stdout.String())
assert.Equal(t, "", stderr.String())
browser.Verify(t, tt.expectedURL)
if opts.NoBrowserFlag {
assert.Equal(t, fmt.Sprintf("%s\n", tt.expectedURL), stdout.String())
assert.Equal(t, "", stderr.String())
browser.Verify(t, "")
} else {
assert.Equal(t, "", stdout.String())
assert.Equal(t, "", stderr.String())
browser.Verify(t, tt.expectedURL)
}
})
}
}

View file

@ -10,6 +10,7 @@ import (
"github.com/cli/cli/git"
"github.com/cli/cli/internal/ghrepo"
"github.com/cli/cli/pkg/cmdutil"
"github.com/cli/cli/pkg/extensions"
"github.com/cli/cli/utils"
"github.com/spf13/cobra"
)
@ -73,13 +74,15 @@ func NewCmdExtensions(f *cmdutil.Factory) *cobra.Command {
}
return m.InstallLocal(wd)
}
repo, err := ghrepo.FromFullName(args[0])
if err != nil {
return err
}
if !strings.HasPrefix(repo.RepoName(), "gh-") {
return errors.New("the repository name must start with `gh-`")
if err := checkValidExtension(cmd.Root(), m, repo.RepoName()); err != nil {
return err
}
cfg, err := f.Config()
if err != nil {
return err
@ -137,3 +140,24 @@ func NewCmdExtensions(f *cmdutil.Factory) *cobra.Command {
extCmd.Hidden = true
return &extCmd
}
func checkValidExtension(rootCmd *cobra.Command, m extensions.ExtensionManager, extName string) error {
if !strings.HasPrefix(extName, "gh-") {
return errors.New("extension repository name must start with `gh-`")
}
commandName := strings.TrimPrefix(extName, "gh-")
if c, _, err := rootCmd.Traverse([]string{commandName}); err != nil {
return err
} else if c != rootCmd {
return fmt.Errorf("%q matches the name of a built-in command", commandName)
}
for _, ext := range m.List() {
if ext.Name() == commandName {
return fmt.Errorf("there is already an installed extension that provides the %q command", commandName)
}
}
return nil
}

View file

@ -11,6 +11,7 @@ import (
"github.com/cli/cli/pkg/cmdutil"
"github.com/cli/cli/pkg/extensions"
"github.com/cli/cli/pkg/iostreams"
"github.com/spf13/cobra"
"github.com/stretchr/testify/assert"
)
@ -26,6 +27,7 @@ func TestNewCmdExtensions(t *testing.T) {
managerStubs func(em *extensions.ExtensionManagerMock) func(*testing.T)
isTTY bool
wantErr bool
errMsg string
wantStdout string
wantStderr string
}{
@ -33,16 +35,37 @@ func TestNewCmdExtensions(t *testing.T) {
name: "install an extension",
args: []string{"install", "owner/gh-some-ext"},
managerStubs: func(em *extensions.ExtensionManagerMock) func(*testing.T) {
em.ListFunc = func() []extensions.Extension {
return []extensions.Extension{}
}
em.InstallFunc = func(s string, out, errOut io.Writer) error {
return nil
}
return func(t *testing.T) {
calls := em.InstallCalls()
assert.Equal(t, 1, len(calls))
assert.Equal(t, "https://github.com/owner/gh-some-ext.git", calls[0].URL)
installCalls := em.InstallCalls()
assert.Equal(t, 1, len(installCalls))
assert.Equal(t, "https://github.com/owner/gh-some-ext.git", installCalls[0].URL)
listCalls := em.ListCalls()
assert.Equal(t, 1, len(listCalls))
}
},
},
{
name: "install an extension with same name as existing extension",
args: []string{"install", "owner/gh-existing-ext"},
managerStubs: func(em *extensions.ExtensionManagerMock) func(*testing.T) {
em.ListFunc = func() []extensions.Extension {
e := &Extension{path: "owner2/gh-existing-ext"}
return []extensions.Extension{e}
}
return func(t *testing.T) {
calls := em.ListCalls()
assert.Equal(t, 1, len(calls))
}
},
wantErr: true,
errMsg: "there is already an installed extension that provides the \"existing-ext\" command",
},
{
name: "install local extension",
args: []string{"install", "."},
@ -61,6 +84,7 @@ func TestNewCmdExtensions(t *testing.T) {
name: "upgrade error",
args: []string{"upgrade"},
wantErr: true,
errMsg: "must specify an extension to upgrade",
},
{
name: "upgrade an extension",
@ -151,7 +175,7 @@ func TestNewCmdExtensions(t *testing.T) {
_, err := cmd.ExecuteC()
if tt.wantErr {
assert.Error(t, err)
assert.EqualError(t, err, tt.errMsg)
} else {
assert.NoError(t, err)
}
@ -169,3 +193,79 @@ func TestNewCmdExtensions(t *testing.T) {
func normalizeDir(d string) string {
return strings.TrimPrefix(d, "/private")
}
func Test_checkValidExtension(t *testing.T) {
rootCmd := &cobra.Command{}
rootCmd.AddCommand(&cobra.Command{Use: "help"})
rootCmd.AddCommand(&cobra.Command{Use: "auth"})
m := &extensions.ExtensionManagerMock{
ListFunc: func() []extensions.Extension {
return []extensions.Extension{
&extensions.ExtensionMock{
NameFunc: func() string { return "screensaver" },
},
&extensions.ExtensionMock{
NameFunc: func() string { return "triage" },
},
}
},
}
type args struct {
rootCmd *cobra.Command
manager extensions.ExtensionManager
extName string
}
tests := []struct {
name string
args args
wantError string
}{
{
name: "valid extension",
args: args{
rootCmd: rootCmd,
manager: m,
extName: "gh-hello",
},
},
{
name: "invalid extension name",
args: args{
rootCmd: rootCmd,
manager: m,
extName: "gherkins",
},
wantError: "extension repository name must start with `gh-`",
},
{
name: "clashes with built-in command",
args: args{
rootCmd: rootCmd,
manager: m,
extName: "gh-auth",
},
wantError: "\"auth\" matches the name of a built-in command",
},
{
name: "clashes with an installed extension",
args: args{
rootCmd: rootCmd,
manager: m,
extName: "gh-triage",
},
wantError: "there is already an installed extension that provides the \"triage\" command",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := checkValidExtension(tt.args.rootCmd, tt.args.manager, tt.args.extName)
if tt.wantError == "" {
assert.NoError(t, err)
} else {
assert.EqualError(t, err, tt.wantError)
}
})
}
}

View file

@ -22,7 +22,7 @@ func PrintIssues(io *iostreams.IOStreams, prefix string, totalCount int, issues
issueNum = "#" + issueNum
}
issueNum = prefix + issueNum
labels := IssueLabelList(issue)
labels := IssueLabelList(issue, cs)
if labels != "" && table.IsTTY() {
labels = fmt.Sprintf("(%s)", labels)
}
@ -56,14 +56,14 @@ func truncateLabels(w int, t string) string {
return fmt.Sprintf("(%s)", truncated)
}
func IssueLabelList(issue api.Issue) string {
func IssueLabelList(issue api.Issue, cs *iostreams.ColorScheme) string {
if len(issue.Labels.Nodes) == 0 {
return ""
}
labelNames := make([]string, 0, len(issue.Labels.Nodes))
for _, label := range issue.Labels.Nodes {
labelNames = append(labelNames, label.Name)
labelNames = append(labelNames, cs.HexToRGB(label.Color, label.Name))
}
return strings.Join(labelNames, ", ")

View file

@ -125,7 +125,7 @@ func viewRun(opts *ViewOptions) error {
return nil
}
return printRawIssuePreview(opts.IO.Out, issue)
return printRawIssuePreview(opts.IO.Out, issue, opts.IO.ColorScheme())
}
func findIssue(client *http.Client, baseRepoFn func() (ghrepo.Interface, error), selector string, loadComments bool) (*api.Issue, error) {
@ -141,9 +141,9 @@ func findIssue(client *http.Client, baseRepoFn func() (ghrepo.Interface, error),
return issue, err
}
func printRawIssuePreview(out io.Writer, issue *api.Issue) error {
func printRawIssuePreview(out io.Writer, issue *api.Issue, cs *iostreams.ColorScheme) error {
assignees := issueAssigneeList(*issue)
labels := shared.IssueLabelList(*issue)
labels := shared.IssueLabelList(*issue, cs)
projects := issueProjectList(*issue)
// Print empty strings for empty values so the number of metadata lines is consistent when
@ -193,7 +193,7 @@ func printHumanIssuePreview(opts *ViewOptions, issue *api.Issue) error {
fmt.Fprint(out, cs.Bold("Assignees: "))
fmt.Fprintln(out, assignees)
}
if labels := shared.IssueLabelList(*issue); labels != "" {
if labels := shared.IssueLabelList(*issue, cs); labels != "" {
fmt.Fprint(out, cs.Bold("Labels: "))
fmt.Fprintln(out, labels)
}

View file

@ -139,7 +139,7 @@ func printRawPrPreview(io *iostreams.IOStreams, pr *api.PullRequest) error {
reviewers := prReviewerList(*pr, cs)
assignees := prAssigneeList(*pr)
labels := prLabelList(*pr)
labels := prLabelList(*pr, cs)
projects := prProjectList(*pr)
fmt.Fprintf(out, "title:\t%s\n", pr.Title)
@ -197,7 +197,7 @@ func printHumanPrPreview(opts *ViewOptions, pr *api.PullRequest) error {
fmt.Fprint(out, cs.Bold("Assignees: "))
fmt.Fprintln(out, assignees)
}
if labels := prLabelList(*pr); labels != "" {
if labels := prLabelList(*pr, cs); labels != "" {
fmt.Fprint(out, cs.Bold("Labels: "))
fmt.Fprintln(out, labels)
}
@ -367,14 +367,14 @@ func prAssigneeList(pr api.PullRequest) string {
return list
}
func prLabelList(pr api.PullRequest) string {
func prLabelList(pr api.PullRequest, cs *iostreams.ColorScheme) string {
if len(pr.Labels.Nodes) == 0 {
return ""
}
labelNames := make([]string, 0, len(pr.Labels.Nodes))
for _, label := range pr.Labels.Nodes {
labelNames = append(labelNames, label.Name)
labelNames = append(labelNames, cs.HexToRGB(label.Color, label.Name))
}
list := strings.Join(labelNames, ", ")

View file

@ -221,21 +221,23 @@ func createRun(opts *CreateOptions) error {
return err
}
// GitIgnore and License templates not added when a template repository is passed.
if gitIgnoreTemplate == "" && opts.Template == "" && opts.IO.CanPrompt() {
gt, err := interactiveGitIgnore(api.NewClientFromHTTP(httpClient), host)
if err != nil {
return err
// GitIgnore and License templates not added when a template repository
// is passed, or when the confirm flag is set.
if opts.Template == "" && opts.IO.CanPrompt() && !opts.ConfirmSubmit {
if gitIgnoreTemplate == "" {
gt, err := interactiveGitIgnore(api.NewClientFromHTTP(httpClient), host)
if err != nil {
return err
}
gitIgnoreTemplate = gt
}
gitIgnoreTemplate = gt
}
if repoLicenseTemplate == "" && opts.Template == "" && opts.IO.CanPrompt() {
lt, err := interactiveLicense(api.NewClientFromHTTP(httpClient), host)
if err != nil {
return err
if repoLicenseTemplate == "" {
lt, err := interactiveLicense(api.NewClientFromHTTP(httpClient), host)
if err != nil {
return err
}
repoLicenseTemplate = lt
}
repoLicenseTemplate = lt
}
}

View file

@ -817,3 +817,64 @@ func TestRepoCreate_WithGitIgnore_Org(t *testing.T) {
t.Errorf("expected %q, got %q", "OWNERID", ownerId)
}
}
func TestRepoCreate_WithConfirmFlag(t *testing.T) {
cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
cs.Register(`git remote add -f origin https://github\.com/OWNER/REPO\.git`, 0, "")
cs.Register(`git rev-parse --show-toplevel`, 0, "")
reg := &httpmock.Registry{}
reg.Register(
httpmock.GraphQL(`mutation RepositoryCreate\b`),
httpmock.StringResponse(`
{ "data": { "createRepository": {
"repository": {
"id": "REPOID",
"url": "https://github.com/OWNER/REPO",
"name": "REPO",
"owner": {
"login": "OWNER"
}
}
} } }`),
)
reg.Register(
httpmock.REST("GET", "users/OWNER"),
httpmock.StringResponse(`{ "node_id": "OWNERID" }`),
)
httpClient := &http.Client{Transport: reg}
in := "OWNER/REPO --confirm --private"
output, err := runCommand(httpClient, in, true)
if err != nil {
t.Errorf("error running command `repo create %v`: %v", in, err)
}
assert.Equal(t, "", output.String())
assert.Equal(t, "✓ Created repository OWNER/REPO on GitHub\n✓ Added remote https://github.com/OWNER/REPO.git\n", output.Stderr())
var reqBody struct {
Query string
Variables struct {
Input map[string]interface{}
}
}
if len(reg.Requests) != 2 {
t.Fatalf("expected 2 HTTP request, got %d", len(reg.Requests))
}
bodyBytes, _ := ioutil.ReadAll(reg.Requests[1].Body)
_ = json.Unmarshal(bodyBytes, &reqBody)
if repoName := reqBody.Variables.Input["name"].(string); repoName != "REPO" {
t.Errorf("expected %q, got %q", "REPO", repoName)
}
if repoVisibility := reqBody.Variables.Input["visibility"].(string); repoVisibility != "PRIVATE" {
t.Errorf("expected %q, got %q", "PRIVATE", repoVisibility)
}
}

View file

@ -123,6 +123,9 @@ func watchRun(opts *WatchOptions) error {
if run.Status == shared.Completed {
fmt.Fprintf(out, "Run %s (%s) has already completed with '%s'\n", cs.Bold(run.Name), cs.Cyanf("%d", run.ID), run.Conclusion)
if opts.ExitStatus && run.Conclusion != shared.Success {
return cmdutil.SilentError
}
return nil
}

View file

@ -190,6 +190,21 @@ func TestWatchRun(t *testing.T) {
},
wantOut: "Run failed (1234) has already completed with 'failure'\n",
},
{
name: "already completed, exit status",
opts: &WatchOptions{
RunID: "1234",
ExitStatus: true,
},
httpStubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/1234"),
httpmock.JSONResponse(shared.FailedRun))
},
wantOut: "Run failed (1234) has already completed with 'failure'\n",
wantErr: true,
errMsg: "SilentError",
},
{
name: "prompt, no in progress runs",
tty: true,
@ -307,13 +322,8 @@ func TestWatchRun(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
err := watchRun(tt.opts)
if tt.wantErr {
assert.Error(t, err)
assert.Equal(t, tt.errMsg, err.Error())
if !tt.opts.ExitStatus {
return
}
}
if !tt.opts.ExitStatus {
assert.EqualError(t, err, tt.errMsg)
} else {
assert.NoError(t, err)
}
assert.Equal(t, tt.wantOut, stdout.String())

View file

@ -3,6 +3,7 @@ package iostreams
import (
"fmt"
"os"
"strconv"
"strings"
"github.com/mgutz/ansi"
@ -202,3 +203,14 @@ func (c *ColorScheme) ColorFromString(s string) func(string) string {
return fn
}
func (c *ColorScheme) HexToRGB(hex string, x string) string {
if !c.enabled || !c.is256enabled {
return x
}
r, _ := strconv.ParseInt(hex[0:2], 16, 64)
g, _ := strconv.ParseInt(hex[2:4], 16, 64)
b, _ := strconv.ParseInt(hex[4:6], 16, 64)
return fmt.Sprintf("\033[38;2;%d;%d;%dm%s\033[0m", r, g, b, x)
}

View file

@ -3,6 +3,8 @@ package iostreams
import (
"os"
"testing"
"github.com/stretchr/testify/assert"
)
func TestEnvColorDisabled(t *testing.T) {
@ -143,3 +145,54 @@ func TestEnvColorForced(t *testing.T) {
})
}
}
func Test_HextoRGB(t *testing.T) {
tests := []struct {
name string
hex string
arg string
expectedOutput string
expectedError bool
cs *ColorScheme
}{
{
name: "Colored red enabled color",
hex: "fc0303",
arg: "red",
expectedOutput: "\033[38;2;252;3;3mred\033[0m",
cs: NewColorScheme(true, true),
},
{
name: "Failed colored red enabled color",
hex: "fc0303",
arg: "red",
expectedOutput: "\033[38;2;252;2;3mred\033[0m",
expectedError: true,
cs: NewColorScheme(true, true),
},
{
name: "Colored red disabled color",
hex: "fc0303",
arg: "red",
expectedOutput: "red",
cs: NewColorScheme(false, false),
},
{
name: "Failed colored red disabled color",
hex: "fc0303",
arg: "red",
expectedOutput: "\033[38;2;252;3;3mred\033[0m",
expectedError: true,
cs: NewColorScheme(false, false),
},
}
for _, tt := range tests {
output := tt.cs.HexToRGB(tt.hex, tt.arg)
if tt.expectedError {
assert.NotEqual(t, tt.expectedOutput, output)
} else {
assert.Equal(t, tt.expectedOutput, output)
}
}
}