Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ endif
## Install/uninstall tasks are here for use on *nix platform. On Windows, there is no equivalent.

DESTDIR :=
prefix := /usr/local
prefix ?= /usr/local
bindir := ${prefix}/bin
datadir := ${prefix}/share
mandir := ${datadir}/man
Expand Down
7 changes: 4 additions & 3 deletions api/queries_pr.go
Original file line number Diff line number Diff line change
Expand Up @@ -296,9 +296,10 @@ type PullRequestCommitCommit struct {
}

type PullRequestFile struct {
Path string `json:"path"`
Additions int `json:"additions"`
Deletions int `json:"deletions"`
Path string `json:"path"`
Additions int `json:"additions"`
Deletions int `json:"deletions"`
ChangeType string `json:"changeType"`
}

type ReviewRequests struct {
Expand Down
3 changes: 2 additions & 1 deletion api/query_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,8 @@ var prFiles = shortenQuery(`
nodes {
additions,
deletions,
path
path,
changeType
}
}
`)
Expand Down
4 changes: 2 additions & 2 deletions api/query_builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func TestPullRequestGraphQL(t *testing.T) {
{
name: "compressed query",
fields: []string{"files"},
want: "files(first: 100) {nodes {additions,deletions,path}}",
want: "files(first: 100) {nodes {additions,deletions,path,changeType}}",
},
{
name: "invalid fields",
Expand Down Expand Up @@ -72,7 +72,7 @@ func TestIssueGraphQL(t *testing.T) {
{
name: "compressed query",
fields: []string{"files"},
want: "files(first: 100) {nodes {additions,deletions,path}}",
want: "files(first: 100) {nodes {additions,deletions,path,changeType}}",
},
{
name: "projectItems",
Expand Down
29 changes: 23 additions & 6 deletions internal/featuredetection/feature_detection.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,15 @@ type Detector interface {
}

type IssueFeatures struct {
StateReason bool
ActorIsAssignable bool
StateReason bool
StateReasonDuplicate bool
ActorIsAssignable bool
}

var allIssueFeatures = IssueFeatures{
StateReason: true,
ActorIsAssignable: true,
StateReason: true,
StateReasonDuplicate: true,
ActorIsAssignable: true,
}

type PullRequestFeatures struct {
Expand Down Expand Up @@ -138,8 +140,9 @@ func (d *detector) IssueFeatures() (IssueFeatures, error) {
}

features := IssueFeatures{
StateReason: false,
ActorIsAssignable: false, // replaceActorsForAssignable GraphQL mutation unavailable on GHES
StateReason: false,
StateReasonDuplicate: false,
ActorIsAssignable: false, // replaceActorsForAssignable GraphQL mutation unavailable on GHES
}

var featureDetection struct {
Expand All @@ -148,6 +151,11 @@ func (d *detector) IssueFeatures() (IssueFeatures, error) {
Name string
} `graphql:"fields(includeDeprecated: true)"`
} `graphql:"Issue: __type(name: \"Issue\")"`
IssueClosedStateReason struct {
EnumValues []struct {
Name string
} `graphql:"enumValues(includeDeprecated: true)"`
} `graphql:"IssueClosedStateReason: __type(name: \"IssueClosedStateReason\")"`
}

gql := api.NewClientFromHTTP(d.httpClient)
Expand All @@ -162,6 +170,15 @@ func (d *detector) IssueFeatures() (IssueFeatures, error) {
}
}

if features.StateReason {
for _, enumValue := range featureDetection.IssueClosedStateReason.EnumValues {
if enumValue.Name == "DUPLICATE" {
features.StateReasonDuplicate = true
break
}
}
}

return features, nil
}

Expand Down
45 changes: 37 additions & 8 deletions internal/featuredetection/feature_detection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,19 @@ func TestIssueFeatures(t *testing.T) {
name: "github.com",
hostname: "github.com",
wantFeatures: IssueFeatures{
StateReason: true,
ActorIsAssignable: true,
StateReason: true,
StateReasonDuplicate: true,
ActorIsAssignable: true,
},
wantErr: false,
},
{
name: "ghec data residency (ghe.com)",
hostname: "stampname.ghe.com",
wantFeatures: IssueFeatures{
StateReason: true,
ActorIsAssignable: true,
StateReason: true,
StateReasonDuplicate: true,
ActorIsAssignable: true,
},
wantErr: false,
},
Expand All @@ -44,23 +46,50 @@ func TestIssueFeatures(t *testing.T) {
`query Issue_fields\b`: `{"data": {}}`,
},
wantFeatures: IssueFeatures{
StateReason: false,
ActorIsAssignable: false,
StateReason: false,
StateReasonDuplicate: false,
ActorIsAssignable: false,
},
wantErr: false,
},
{
name: "GHE has state reason field",
name: "GHE has state reason field without duplicate enum",
hostname: "git.my.org",
queryResponse: map[string]string{
`query Issue_fields\b`: heredoc.Doc(`
{ "data": { "Issue": { "fields": [
{"name": "stateReason"}
] }, "IssueClosedStateReason": { "enumValues": [
{"name": "COMPLETED"},
{"name": "NOT_PLANNED"}
] } } }
`),
},
wantFeatures: IssueFeatures{
StateReason: true,
StateReason: true,
StateReasonDuplicate: false,
ActorIsAssignable: false,
},
wantErr: false,
},
{
name: "GHE has duplicate state reason enum value",
hostname: "git.my.org",
queryResponse: map[string]string{
`query Issue_fields\b`: heredoc.Doc(`
{ "data": { "Issue": { "fields": [
{"name": "stateReason"}
] }, "IssueClosedStateReason": { "enumValues": [
{"name": "COMPLETED"},
{"name": "NOT_PLANNED"},
{"name": "DUPLICATE"}
] } } }
`),
},
wantFeatures: IssueFeatures{
StateReason: true,
StateReasonDuplicate: true,
ActorIsAssignable: false,
},
wantErr: false,
},
Expand Down
16 changes: 16 additions & 0 deletions pkg/cmd/browse/browse.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ type BrowseOptions struct {
SettingsFlag bool
WikiFlag bool
ActionsFlag bool
BlameFlag bool
NoBrowserFlag bool
HasRepoOverride bool
}
Expand Down Expand Up @@ -91,6 +92,9 @@ func NewCmdBrowse(f *cmdutil.Factory, runF func(*BrowseOptions) error) *cobra.Co
# Open main.go at line 312
$ gh browse main.go:312

# Open blame view for main.go at line 312
$ gh browse main.go:312 --blame

# Open main.go with the repository at head of bug-fix branch
$ gh browse main.go --branch bug-fix

Expand Down Expand Up @@ -141,6 +145,10 @@ func NewCmdBrowse(f *cmdutil.Factory, runF func(*BrowseOptions) error) *cobra.Co
return err
}

if opts.BlameFlag && opts.SelectorArg == "" {
return cmdutil.FlagErrorf("`--blame` requires a file path argument")
}

if (isNumber(opts.SelectorArg) || isCommit(opts.SelectorArg)) && (opts.Branch != "" || opts.Commit != "") {
return cmdutil.FlagErrorf("%q is an invalid argument when using `--branch` or `--commit`", opts.SelectorArg)
}
Expand All @@ -163,6 +171,7 @@ func NewCmdBrowse(f *cmdutil.Factory, runF func(*BrowseOptions) error) *cobra.Co
cmd.Flags().BoolVarP(&opts.WikiFlag, "wiki", "w", false, "Open repository wiki")
cmd.Flags().BoolVarP(&opts.ActionsFlag, "actions", "a", false, "Open repository actions")
cmd.Flags().BoolVarP(&opts.SettingsFlag, "settings", "s", false, "Open repository settings")
cmd.Flags().BoolVar(&opts.BlameFlag, "blame", false, "Open blame view for a file")
cmd.Flags().BoolVarP(&opts.NoBrowserFlag, "no-browser", "n", false, "Print destination URL instead of opening the browser")
cmd.Flags().StringVarP(&opts.Commit, "commit", "c", "", "Select another commit by passing in the commit SHA, default is the last commit")
cmd.Flags().StringVarP(&opts.Branch, "branch", "b", "", "Select another branch by passing in the branch name")
Expand Down Expand Up @@ -272,9 +281,16 @@ func parseSection(baseRepo ghrepo.Interface, opts *BrowseOptions) (string, error
} else {
rangeFragment = fmt.Sprintf("L%d", rangeStart)
}
if opts.BlameFlag {
return fmt.Sprintf("blame/%s/%s#%s", escapePath(ref), escapePath(filePath), rangeFragment), nil
}
return fmt.Sprintf("blob/%s/%s?plain=1#%s", escapePath(ref), escapePath(filePath), rangeFragment), nil
}

if opts.BlameFlag {
return fmt.Sprintf("blame/%s/%s", escapePath(ref), escapePath(filePath)), nil
}

return strings.TrimSuffix(fmt.Sprintf("tree/%s/%s", escapePath(ref), escapePath(filePath)), "/"), nil
}

Expand Down
79 changes: 79 additions & 0 deletions pkg/cmd/browse/browse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,29 @@ func TestNewCmdBrowse(t *testing.T) {
cli: "de07febc26e19000f8c9e821207f3bc34a3c8038 --commit=12a4",
wantsErr: true,
},
{
name: "blame flag",
cli: "main.go --blame",
wants: BrowseOptions{
BlameFlag: true,
SelectorArg: "main.go",
},
wantsErr: false,
},
{
name: "blame flag without file argument",
cli: "--blame",
wantsErr: true,
},
{
name: "blame flag with line number",
cli: "main.go:312 --blame",
wants: BrowseOptions{
BlameFlag: true,
SelectorArg: "main.go:312",
},
wantsErr: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down Expand Up @@ -239,6 +262,7 @@ func TestNewCmdBrowse(t *testing.T) {
assert.Equal(t, tt.wants.SettingsFlag, opts.SettingsFlag)
assert.Equal(t, tt.wants.ActionsFlag, opts.ActionsFlag)
assert.Equal(t, tt.wants.Commit, opts.Commit)
assert.Equal(t, tt.wants.BlameFlag, opts.BlameFlag)
})
}
}
Expand Down Expand Up @@ -595,6 +619,61 @@ func Test_runBrowse(t *testing.T) {
expectedURL: "https://github.com/bchadwic/test/tree/trunk/77507cd94ccafcf568f8560cfecde965fcfa63e7.txt",
wantsErr: false,
},
{
name: "file with blame flag",
opts: BrowseOptions{
SelectorArg: "path/to/file.txt",
BlameFlag: true,
},
baseRepo: ghrepo.New("owner", "repo"),
defaultBranch: "main",
expectedURL: "https://github.com/owner/repo/blame/main/path/to/file.txt",
wantsErr: false,
},
{
name: "file with blame flag and line number",
opts: BrowseOptions{
SelectorArg: "path/to/file.txt:42",
BlameFlag: true,
},
baseRepo: ghrepo.New("owner", "repo"),
defaultBranch: "main",
expectedURL: "https://github.com/owner/repo/blame/main/path/to/file.txt#L42",
wantsErr: false,
},
{
name: "file with blame flag and line range",
opts: BrowseOptions{
SelectorArg: "path/to/file.txt:10-20",
BlameFlag: true,
},
baseRepo: ghrepo.New("owner", "repo"),
defaultBranch: "main",
expectedURL: "https://github.com/owner/repo/blame/main/path/to/file.txt#L10-L20",
wantsErr: false,
},
{
name: "file with blame flag and branch",
opts: BrowseOptions{
SelectorArg: "main.go:100",
BlameFlag: true,
Branch: "feature-branch",
},
baseRepo: ghrepo.New("owner", "repo"),
expectedURL: "https://github.com/owner/repo/blame/feature-branch/main.go#L100",
wantsErr: false,
},
{
name: "file with blame flag and commit",
opts: BrowseOptions{
SelectorArg: "src/app.js:50",
BlameFlag: true,
Commit: "abc123",
},
baseRepo: ghrepo.New("owner", "repo"),
expectedURL: "https://github.com/owner/repo/blame/abc123/src/app.js#L50",
wantsErr: false,
},
}

for _, tt := range tests {
Expand Down
Loading
Loading