Better error messages when you don't supply an arg#144
Merged
probablycorey merged 9 commits intomasterfrom Dec 12, 2019
Merged
Conversation
probablycorey
commented
Dec 11, 2019
mislav
suggested changes
Dec 11, 2019
command/pr.go
Outdated
| if err != nil { | ||
| return err | ||
| } | ||
| return fmt.Errorf("no open pull requests found for branch %q. To open a specific PR use a PR number as an argument", branch) |
Contributor
There was a problem hiding this comment.
You could keep the old behavior and detect a particular error from PullRequestForBranch and add extra error text:
if err != nil {
if strings.Contains(err.Error(), "no open pull requests found") {
err = fmt.Errorf("%s\nTo open a specific pull request, add the pull request number as argument", err)
}
return err
}
probablycorey
commented
Dec 11, 2019
| func (e *NotFoundError) Error() string { | ||
| return e.message | ||
| } | ||
|
|
Contributor
Author
There was a problem hiding this comment.
We can probably use an error like this in situations where no issue or pr is found, like gh pr view 239874987. But I left that for another PR.
Contributor
Author
|
I gave this a go with the custom Error struct idea, let me know what you think. |
mislav
approved these changes
Dec 12, 2019
command/pr.go
Outdated
| if err != nil { | ||
| return err | ||
| switch err.(type) { | ||
| case *api.NotFoundError: |
Contributor
There was a problem hiding this comment.
Nit: a more Go 1.13-way of matching a specific error type, one that plays well with errors.Wrap https://golang.org/pkg/errors/#As
var notFoundErr *api.NotFoundError
if errors.As(err, ¬FoundErr) {
return fmt.Errorf(...)
}
return err
api/queries_pr.go
Outdated
| } | ||
|
|
||
| type NotFoundError struct { | ||
| message string |
Contributor
There was a problem hiding this comment.
Nit: you can simplify this implementation via embedding
type NotFoundError struct {
error
}
// usage:
return NotFoundError{fmt.Errorf(...)}Then you don't need to define the Error() function.
mislav
pushed a commit
that referenced
this pull request
Sep 28, 2021
ghcs ssh: remove terminal, bash_profile setup
Stonre
pushed a commit
to Stonre/strands-fork
that referenced
this pull request
Jul 21, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #75
gh pr checkout
From
requires at least 1 arg(s), only received 0torequires a PR number as an argumentgh pr view
From
no open pull requests found for branch “empty-pr-list”tono open pull requests found for branch “empty-pr-list”. To open a specific PR use a PR number as an argumentgh issue view
From
requires at least 1 arg(s), only received 0torequires an issue number as an argument